Skip to content

ref(node): Streamline Firebase instrumentation - #21748

Merged
nicohrubec merged 7 commits into
developfrom
nh/streamline-firebase
Jun 26, 2026
Merged

ref(node): Streamline Firebase instrumentation#21748
nicohrubec merged 7 commits into
developfrom
nh/streamline-firebase

Conversation

@nicohrubec

@nicohrubecnicohrubec commented Jun 24, 2026

Copy link
Copy Markdown
Member

Streamlines the firebase instrumentation:

  • Folds attributes set via hooks into the instrumentation.
  • Use startSpan APIs from @sentry/core.
  • Remove code paths e.g. configs.
  • No tests added since this should be sufficiently covered already.

Closes#21745

Create Firebase spans via the `@sentry/core` `startSpan` API instead of the OTel
tracer, and fold the integration's firestore/functions hooks (origin, op,
captureException) directly into the instrumentation.
Ref JS-2848
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear-code

Copy link
Copy Markdown

JS-2848

Comment threadpackages/node/src/integrations/tracing/firebase/otel/patches/functions.ts Outdated
@github-actions

github-actionsBot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser27.47 kB--
@sentry/browser - with treeshaking flags25.91 kB--
@sentry/browser (incl. Tracing)45.97 kB--
@sentry/browser (incl. Tracing + Span Streaming)47.72 kB--
@sentry/browser (incl. Tracing, Profiling)50.76 kB--
@sentry/browser (incl. Tracing, Replay)85.22 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags74.81 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)89.91 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)102.57 kB--
@sentry/browser (incl. Feedback)44.66 kB--
@sentry/browser (incl. sendFeedback)32.26 kB--
@sentry/browser (incl. FeedbackAsync)37.4 kB--
@sentry/browser (incl. Metrics)28.54 kB--
@sentry/browser (incl. Logs)28.78 kB--
@sentry/browser (incl. Metrics & Logs)29.47 kB--
@sentry/react29.27 kB--
@sentry/react (incl. Tracing)48.28 kB--
@sentry/vue32.63 kB--
@sentry/vue (incl. Tracing)47.84 kB--
@sentry/svelte27.5 kB--
CDN Bundle29.89 kB--
CDN Bundle (incl. Tracing)47.89 kB--
CDN Bundle (incl. Logs, Metrics)31.44 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)49.24 kB--
CDN Bundle (incl. Replay, Logs, Metrics)70.78 kB--
CDN Bundle (incl. Tracing, Replay)85.4 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)86.68 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)91.19 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)92.45 kB--
CDN Bundle - uncompressed88.94 kB--
CDN Bundle (incl. Tracing) - uncompressed145.03 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed93.65 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed149 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed218.62 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed264.05 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed268 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed277.75 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed281.69 kB--
@sentry/nextjs (client)50.67 kB--
@sentry/sveltekit (client)46.37 kB--
@sentry/core/server77.46 kB--
@sentry/core/browser63.8 kB--
@sentry/node-core61.51 kB+0.01%+1 B 🔺
@sentry/node124.1 kB-0.4%-491 B 🔽
@sentry/node/import (ESM hook with diagnostics-channel injection)69.95 kB--
@sentry/node/light50.4 kB+0.01%+1 B 🔺
@sentry/node - without tracing73.61 kB+0.01%+1 B 🔺
@sentry/aws-serverless84.4 kB-0.01%-1 B 🔽
@sentry/cloudflare (withSentry) - minified180.31 kB--
@sentry/cloudflare (withSentry)446.24 kB--

View base workflow run

@nicohrubec
nicohrubec marked this pull request as ready for review June 24, 2026 12:17
@nicohrubec
nicohrubec requested a review from a team as a code ownerJune 24, 2026 12:17
@nicohrubec
nicohrubec requested review from JPeer264, andreiborza and mydea and removed request for a teamJune 24, 2026 12:17

@JPeer264JPeer264 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Everything looks good, except the clankers objection. Once this is resolved I'll review again

Use `startInactiveSpan` + `withActiveSpan` so the span is ended (enqueuing the
finished transaction) before `flush` drains the queue on handler failure.
Previously the error path ran `captureException` + `flush` inside the `startSpan`
callback, before the span auto-ended, so the transaction could be dropped in
short-lived Firebase Functions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d52644a. Configure here.

Comment threadpackages/node/src/integrations/tracing/firebase/otel/patches/functions.ts Outdated
@nicohrubec
nicohrubec requested a review from JPeer264June 25, 2026 08:23
nicohrubecand others added 3 commits June 25, 2026 10:27
Set kind on the Sentry span options (SERVER for functions, CLIENT for firestore)
so `otel.kind` is emitted as before instead of defaulting to INTERNAL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the magic kind numbers with the `SPAN_KIND` const from `@sentry/core`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines +87 to +108
const span = startInactiveSpan({
name: `firebase.function.${triggerType}`,
op: 'http.request',
kind: SPAN_KIND.SERVER,
attributes,
});

return withActiveSpan(span, async () => {
try {
result = await handler.apply(this, handlerArgs);
} catch (e) {
error = e as Error;
}

functionsConfig?.responseHook?.(span, error);

if (error) {
span.recordException(error);
}

span.end();

if (error) {
await functionsConfig?.errorHook?.(span, error);
const result = await handler.apply(this, handlerArgs);
span.end();
return result;
} catch (error) {
span.setStatus({ code: SPAN_STATUS_ERROR });
captureException(error, {
mechanism: {
type: 'auto.firebase.otel.functions',
handled: false,
},
});
span.end();
await flush(2000);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

l: We could rewrite that to use startSpanManual instead of startInactiveSpan + withActiveSpan

Replace startInactiveSpan + withActiveSpan with a single startSpanManual call.
Behavior is unchanged: the span stays active during the handler and is ended
manually before flushing on error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicohrubec
nicohrubec merged commit fc29e61 into developJun 26, 2026
551 of 556 checks passed
@nicohrubec
nicohrubec deleted the nh/streamline-firebase branch June 26, 2026 08:16
Sign up for freeto 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.

Streamline FirebaseInstrumentation

3 participants

@nicohrubec@JPeer264@andreiborza