Skip to content

feat(observability): group /v1 API spans by domain and endpoint#888

Merged
thostetler merged 3 commits into
adsabs:masterfrom
thostetler:feat/sentry-api-span-grouping
Jun 22, 2026
Merged

feat(observability): group /v1 API spans by domain and endpoint#888
thostetler merged 3 commits into
adsabs:masterfrom
thostetler:feat/sentry-api-span-grouping

Conversation

@thostetler

@thostetler thostetler commented Jun 16, 2026

Copy link
Copy Markdown
Member

Sentry couldn't tell our API calls apart. Client http.client spans only carry the full URL in span.description, so everything grouped on raw descriptions with query strings. The
search.results.render / search.facets.render spans also opened from the shared result list, so they fired on non-search pages too.

  1. Tag /v1/* spans in all three runtimes via a beforeSendSpan hook: api.domain (allowlisted backend tier, else other) and api.endpoint (path only, query/fragment stripped).
  2. Redact every dynamic path segment
  3. Measure the render spans at paint time
  4. Gate those spans behind an opt-in measureRenderSpan prop so only the search page emits them, while still counting zero-result searches.

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.57522% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.0%. Comparing base (e1e3cc5) to head (ad1f40d).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/lib/normalizeApiSpan.ts 95.5% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #888     +/-   ##
========================================
+ Coverage    64.2%   66.0%   +1.9%     
========================================
  Files         350     337     -13     
  Lines       41492   40010   -1482     
  Branches     2010    2025     +15     
========================================
- Hits        26602   26376    -226     
+ Misses      14847   13592   -1255     
+ Partials       43      42      -1     
Files with missing lines Coverage Δ
src/lib/performance.ts 77.5% <100.0%> (-0.7%) ⬇️
src/lib/normalizeApiSpan.ts 95.5% <95.5%> (ø)

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Client spans only carry the full URL in span.description, so dashboards
can't group them. Derive low-cardinality api.domain and api.endpoint tags
in a beforeSendSpan hook across all three runtimes; redact dynamic
segments to {id} so no id or PII reaches a tag value.
Open results/facets spans in useLayoutEffect and close after paint via
requestAnimationFrame, instead of module-level slots. Opt-in via
measureRenderSpan so the shared result list doesn't emit search spans off
the search page.
@thostetler thostetler force-pushed the feat/sentry-api-span-grouping branch from aaccb7e to 26ce4dd Compare June 17, 2026 17:59
@thostetler thostetler marked this pull request as ready for review June 17, 2026 17:59
@thostetler thostetler requested review from Copilot and shinyichen June 17, 2026 17:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves observability by normalizing Sentry spans for /v1/* API calls (domain + redacted endpoint, no query/fragment) across client/server/edge, and by making search render spans opt-in and measured closer to paint time to avoid emitting search render spans from shared result lists on non-search pages.

Changes:

  • Add beforeSendSpan hook (beforeSendApiSpan) to tag /v1/* http.client spans with low-cardinality api.domain + api.endpoint, including dynamic-segment redaction.
  • Replace “open in Telemetry / close after paint” render-span flow with a hook-driven paint-timed approach gated behind measureRenderSpan.
  • Add Vitest coverage for API span normalization behavior.

Risk summary

Medium risk overall: touches core telemetry and Sentry initialization across runtimes; one confirmed TypeScript compile-time blocker exists.

Findings (priority order)

blocker

  • TypeScript compile error from import type { spanToJSON } used with typeof spanToJSON
    • Impact: build fails (new normalizeApiSpan.ts won’t typecheck).
    • Location: src/lib/normalizeApiSpan.ts:1-4
    • Minimal fix: import spanToJSON as a value (or use another exported JSON type).
    • Confidence: high

medium

  • useLayoutEffect will trigger SSR warning for server-rendered SimpleResultList

    • Impact: noisy SSR warnings; can hide real SSR issues during debugging.
    • Location: src/lib/useRenderSpan.ts:19-23
    • Minimal fix: use an isomorphic layout effect (layout in browser, effect on server).
    • Confidence: high
  • Spans can leak if enabled toggles from truefalse while mounted

    • Impact: long-running/never-ended spans skew render timing data.
    • Location: src/lib/useRenderSpan.ts:39-42
    • Minimal fix: when disabled, cancel pending rAF and end/null any open spans.
    • Confidence: high

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/providers.tsx Removes render-span opening from Telemetry to avoid shared-list emissions.
src/pages/search/index.tsx Enables render-span measurement only on the search results page.
src/lib/useRenderSpan.ts Implements paint-timed render spans with opt-in gating.
src/lib/performance.ts Removes module-level slots for results/facets render spans.
src/lib/normalizeApiSpan.ts Adds span normalization/tagging for /v1/* http.client spans.
src/lib/normalizeApiSpan.test.ts Adds unit tests for domain/endpoint parsing and redaction.
src/components/ResultList/SimpleResultList.tsx Adds measureRenderSpan prop to gate render-span emission.
sentry.server.config.ts Registers beforeSendSpan normalization hook on server runtime.
sentry.edge.config.ts Registers beforeSendSpan normalization hook on edge runtime.
sentry.client.config.ts Registers beforeSendSpan normalization hook on client runtime.

Comment thread src/lib/normalizeApiSpan.ts Outdated
Comment on lines +1 to +4
import type { spanToJSON } from '@sentry/nextjs';

// @sentry/nextjs doesn't re-export SpanJSON, so derive it from spanToJSON.
export type SpanJSON = ReturnType<typeof spanToJSON>;
Comment thread src/lib/useRenderSpan.ts
Comment on lines +19 to +23
// Open after DOM commit, before the browser paints.
useLayoutEffect(() => {
if (!enabled) {
return;
}
Comment thread src/lib/useRenderSpan.ts
Comment on lines 39 to 42
useEffect(() => {
if (docs.length === 0) {
if (!enabled) {
return;
}
@thostetler thostetler merged commit ca9840c into adsabs:master Jun 22, 2026
5 checks passed
@thostetler thostetler deleted the feat/sentry-api-span-grouping branch June 22, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants