MetaMask / MetaMask/core

[P2] `QueryClient` distributed-tracing observability in `BaseDataService`

Open
#8,552 0 comments 0 reactions 0 assignees View on GitHub
for-migration mig-epic-42-child
Dominant language
TypeScript
Stars
413
Forks
308
Avg merge
1d 4h
Merged PRs (30d)
253

Description

**Parent Epic:** None (standalone)
**File:** `packages/base-data-service/src/BaseDataService.ts`
**Size:** S | **Hours:** 2–4

---

## Problem

Option A's proxy model (TanStack Query `QueryClient` in the background process, proxy `queryFn` in the UI) intentionally hides the cache, dedupe, retry, fetch, and sync chain from UI-perceived latency tracing. The distributed-tracing wrappers in [metamask-extension#39891](https://github.com/MetaMask/metamask-extension/pull/39891) give us the call boundaries (`useQuery → submitRequestToBackground → messenger → BaseDataService action → response`). Inside the opaque region, every interesting decision is invisible:

- **Cache hit vs. miss.** Most data service calls don't make a network request. Auto-instrumented fetch spans see zero. The query looks "slow" with no attribution.
- **In-flight dedupe.** N concurrent observers collapse to one fetch. The fetch span doesn't know N.
- **Cockatiel retry sequences.** A single logical "fetch this" can produce 0, 1, or many HTTP requests. Auto-spans see them as unrelated. Backoff intervals are invisible.
- **Circuit breaker state.** When open, no fetch happens. The query fails or returns cached data with no auto-span at all.
- **Cache sync to UI.** `setData → cacheUpdated event → Port.postMessage → UI hydrate` is the post-fetch synchronization cost. None of it surfaces as a span.
- **Trigger attribution.** Poll interval, user invalidation, observer subscribe, and WebSocket invalidation are all identical from the network's view.

Three Option A tradeoffs documented in [decisions#131](https://github.com/MetaMask/decisions/pull/131) currently depend on observability that doesn't exist:

- **Per-screen `staleTime`.** Validating that the service-side workarounds hold in production requires distinguishing fetches from cache serves.
- **`refetchOnMount` / `refetchOnWindowFocus` collapse.** Measuring whether concurrent queries amplify messenger volume or are absorbed by background `staleTime`.
- **Split-process staleness indirection.** Correlating UI proxy calls with background fetches.

Without this visibility, the tradeoffs are theoretical. With it, they're verifiable.

---

## Solution

Add a sanctioned extension point to `BaseDataService` that exposes the internal `QueryClient` for **read-only** observability subscriptions. Two variants; preference is maintainer's call:

1. **Public getter.** `get queryClient(): QueryClient`
2. **Constructor callback.** `onCreate?: (qc: QueryClient) => void`

Either is ~3 lines in core. Downstream clients attach their own instrumentation externally (e.g. `qc.getQueryCache().subscribe(emitSpan)` plus Cockatiel `onRetry` / `onBreak` handlers in extension's Sentry init, ~50 lines). The full span chain becomes stitchable end-to-end:

```
useQuery → submitRequestToBackground (span) → messenger (span) →
[BaseDataService.fetchQuery — cache miss, observerAdded → fetching →
Cockatiel attempt 1 → fetch (auto, child of rpc.handler) → 503 →
backoff 200ms → fetch (auto) → 200 → setData → success] →
cacheUpdated → port → UI hydrate
```
---

## Acceptance Criteria

- [ ] `BaseDataService` exposes either a public `queryClient` getter or an `onCreate` constructor hook (maintainer's choice).
- [ ] Downstream `queryCache.subscribe(...)` wiring composes cleanly from a client-side Sentry init file. A prototype in the extension repo demonstrates the full stack trace from `useQuery` through retry through success.
- [ ] No new UI-facing surface.
- [ ] No change to `OmitKeyof` type erasure.
- [ ] No change to messenger contract.
- [ ] No change to proxy `queryFn` semantics.

---

## Labels

`area-data-service`, `area-observability`

---

## Dependencies

**Related:** [#8530](https://github.com/MetaMask/core/issues/8530), [#8531](https://github.com/MetaMask/core/issues/8531) (orthogonal — same epic scope but different surface; not blocking).
**Depends on:** none.
**Dependents:** none.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with packages/base-data-service/src/BaseDataService.ts and trace how its internal QueryClient is created and used. Evaluate the getter and constructor-hook options, then verify that downstream queryCache.subscribe wiring can observe the cache and request lifecycle without changing the UI surface, OmitKeyof behavior, messenger contract, or proxy queryFn semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
observability
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.