feat(web): give API services an endpoint-shaped view of themselves - #476
Draft
Makisuo wants to merge 1 commit into
Draft
feat(web): give API services an endpoint-shaped view of themselves#476Makisuo wants to merge 1 commit into
Makisuo wants to merge 1 commit into
Conversation
The service detail page treated every service the same: Overview, Operations, Dependencies. For an HTTP service that buries the unit people actually reason about — the endpoint — inside a list of raw span names, with no way to open one. Detect HTTP APIs and offer them an Endpoints tab plus a per-endpoint page. Detection rides the existing service-detail overview bundle rather than taking its own round-trip, because it gates a trigger in the page header and so must resolve on first paint of *every* tab, not just Overview. It is deliberately permissive (any server traffic with routes, no ratio gate) — the tab is additive, so a false positive costs a thin extra tab while a false negative hides the feature. A probe failure degrades to "not an API" instead of taking the page's charts down with it. The endpoint list rides the existing rollups: service_operations_* already stores the normalized "GET /api/users" name, so it is an endpoint rollup in disguise. Raw edges filter accurately on SpanKind = 'Server'; the rollup interiors match the display-name shape, since those tables dropped SpanKind. The consequence is recorded in the file: a non-HTTP span literally named "GET /foo" counts inside the rollup window. A dedicated service_endpoints_hourly MV removes both that heuristic and the string split, and ch/queries/ service-endpoints.ts is the single swap point when it lands. Also fixes a latent bug this surfaced. canUseAnnualServiceOverview gated the service-overview rollup route on the singular `spanName` but not the plural `spanNames` — the spelling every modern caller uses. Those queries were routed to service_overview_minutely/_hourly, which aggregate SpanName away, so the filter was silently dropped and the caller got service-wide numbers under one endpoint's name with no error anywhere. canUseServiceOverviewMv checks both spellings; this predicate already checked excludedSpanNames, so the omission was an oversight rather than intent. It had no test coverage at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The service detail page treated every service identically — Overview, Operations, Dependencies. For an HTTP service that buries the unit people actually reason about (the endpoint: method + route + status) inside a list of raw span names, with no way to open one.
This detects HTTP APIs and gives them an Endpoints tab plus a per-endpoint page. Non-API services are unchanged.
Detection
Rides the existing
serviceDetailOverviewbundle as a fourth concurrent query rather than taking its own round-trip. It gates a trigger in the page header, so it has to resolve on first paint of every tab — a deep link to?tab=operationsstill needs to know whether to offer Endpoints. The header's environment switcher already subscribes to that same response, so this is free.isHttpApiServiceis deliberately permissive — any server traffic carrying routes, no ratio gate. The tab is additive (Operations stays), so a false positive costs a thin extra tab while a false negative hides the feature entirely; a worker that also exposes a health endpoint should still get it. The probe is clamped to a 24h window (a service that serves HTTP now served HTTP an hour ago) and wrapped so a failure degrades to "not an API" instead of failing the bundle that draws the page's charts.Deliberately not environment-scoped: whether a service is an API is a property of the service, and an unscoped answer keeps the tab from flickering away when you select a low-traffic env.
Endpoint list
v1 rides the existing rollups — no new MV, no backfill.
service_operations_minutely/_hourlyalready store the normalized"GET /api/users"name, so they're an endpoint rollup in disguise. What they dropped isSpanKind, so:SpanKind = 'Server'+ non-empty route/url.path/target);^(GET|POST|…) /).The consequence is documented in the file: a non-HTTP span literally named
"GET /foo"counts inside the rollup window. A dedicatedservice_endpoints_hourlyMV keyed(OrgId, Hour, ServiceName, DeploymentEnv, HttpMethod, HttpRoute)and filtered at write time removes both that heuristic and the runtime string split —ch/queries/service-endpoints.tsis the single swap point when it lands, and nothing downstream of it knows how the rows are sourced.Method and route are split server-side so the UI never parses a fused name, but
spanNameis kept alongside them because it — notroute— is the key the/tracesfilter and the sparkline timeseries match on.Endpoint detail
/services/:name/endpoints?method=GET&route=/v1/users. The identity lives in search params, not a path segment: routes contain slashes, and folding one into a segment means%2Fround-tripping and an unreadable URL.Reuses the four service charts (via a new optional
spanNamesonmakeAllMetricsTimeseriesRequest— the filter was already in the wire schema, only that builder omitted it), plus a status-class panel and a slowest-requests list.The status query is written in SQL rather than driven through the generic
groupBy: "attribute"path, because that path takes a single attribute key and HTTP status has two live semconv spellings (http.response.status_codeandhttp.status_code). Coalescing them — the same precedencetrace_list_mvuses — is one query; picking one key is a silent blind spot for every older SDK.Bug fix: a span-name filter was being silently dropped
Found while verifying the detail charts, which read ~2.4k/s for an endpoint doing ~973/s.
canUseAnnualServiceOverviewgated the service-overview rollup route onopts.spanName == null— the singular spelling only. The pluralspanNames, which every modern caller uses, sailed through toservice_overview_minutely/_hourly, which aggregateSpanNameaway. The filter was dropped with no error: service-wide numbers returned under one endpoint's name.This is pre-existing shared code — any
allMetricscaller passingspanNameswas affected, not just this feature.canUseServiceOverviewMvchecks both spellings, and this predicate already checkedexcludedSpanNames, so the omission reads as an oversight. It had no test coverage at all;overview-rollup-route.test.tsnow covers the predicate and asserts at the SQL level that an endpoint-scoped chart really does read rawtracesand really does carry its filter. Both new assertions fail when the one-line fix is reverted.Not in scope
Per-endpoint downstream dependency fan-out — the dependency rollups aren't span-scoped, so it needs raw trace-tree traversal or the future MV.
I also dropped the planned per-endpoint error-issues panel: the v2
errorIssueslist has no span-name filter, so it would have shown service-wide issues on an endpoint page. The status-class split covers "how is it failing" accurately instead.Testing
overview-rollup-route.test.ts(new) — routing guard + SQL-level filter assertion; both fail without the fix.service-endpoints.test.ts× 2 (query + web helpers) — SQL shape, both rollup tiers, method/route split, detection thresholds, route middle-elision round-trip.domain,query-engine,api,web.Verified against real data locally:
subscriptions-apigets the tab;kafka-consumers-sdk-metadoes not, and a stale?tab=endpointsfalls back to Overview.METHOD+ route with the distinguishing tail intact; the~sampling marker renders.One gotcha for re-testing: the first reload after the fix still showed the old numbers to ten decimal places — a cached response, not a stale build. Changing the time preset busts the key.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.