Skip to content

feat(node)!: Use function.gcp span op for Firebase functions #22659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
msonnb wants to merge 3 commits into from
+7 −6
Closed

feat(node)!: Use function.gcp span op for Firebase functions #22659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3711b52
feat(node)!: Use `function.gcp` span op for Firebase functions
msonnb Jul 27, 2026
83e7945
use sentry.op
msonnb Jul 28, 2026
2c8a0b1
whoops
msonnb Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions dev-packages/e2e-tests/test-applications/node-firebase/tests/functions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ test.fixme('should only call the function once without any extra calls', async (
'faas.provider': 'firebase',
'faas.trigger': 'http.request',
'sentry.kind': 'server',
'sentry.op': 'http.request',
'sentry.op': 'function.gcp',
'sentry.origin': 'auto.firebase.orchestrion.functions',
'sentry.sample_rate': expect.any(Number),
'sentry.source': 'route',
},
op: 'http.request',
op: 'function.gcp',

@cursor cursor Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Feat lacks runnable coverage

Medium Severity

This feat PR only updates test.fixme e2e cases and does not add a runnable unit, integration, or e2e test that exercises the new function.gcp op on the changed instrumentation. Per the project PR review guidelines, feat PRs need at least one integration or e2e test covering the new behavior.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit 2c8a0b1. Configure here.

origin: 'auto.firebase.orchestrion.functions',
span_id: expect.any(String),
status: 'ok',
Expand Down Expand Up @@ -101,12 +101,12 @@ test.fixme('should create a document and trigger onDocumentCreated and another w
'faas.provider': 'firebase',
'faas.trigger': 'http.request',
'sentry.kind': 'server',
'sentry.op': 'http.request',
'sentry.op': 'function.gcp',
'sentry.origin': 'auto.firebase.orchestrion.functions',
'sentry.sample_rate': expect.any(Number),
'sentry.source': 'route',
},
op: 'http.request',
op: 'function.gcp',

@cursor cursor Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incomplete op assertions in tests

Medium Severity

This feat changes the Firebase function span op to function.gcp, including for non-HTTP triggers like onDocumentCreated, but the Firestore transaction assertions still use expect.any(String) for sentry.op and op. Those are the payloads that previously wrongly reported http.request, so the new behavior is not thoroughly asserted. Flagged because the review rules require feat coverage to assert newly added payload data thoroughly and to avoid overly relaxed assertions.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit e1785e3. Configure here.

origin: 'auto.firebase.orchestrion.functions',
span_id: expect.any(String),
status: 'ok',
Expand Down
5 changes: 3 additions & 2 deletions packages/node/src/integrations/tracing/firebase/otel/patches/functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { InstrumentationBase } from '@opentelemetry/instrumentation';
import { InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation';
import { InstrumentationNodeModuleFile } from '../../../InstrumentationNodeModuleFile';
import { SENTRY_KIND } from '@sentry/conventions/attributes';
import { SENTRY_KIND, SENTRY_OP } from '@sentry/conventions/attributes';
import { FAAS_FUNCTION_GCP_SPAN_OP } from '@sentry/conventions/op';
import type { SpanAttributes } from '@sentry/core';
import {
captureException,
Expand Down Expand Up @@ -68,6 +69,7 @@ export function patchV2Functions<T extends FirebaseFunctions = FirebaseFunctions
const functionName = process.env.FUNCTION_TARGET || process.env.K_SERVICE || 'unknown';

const attributes: SpanAttributes = {
[SENTRY_OP]: FAAS_FUNCTION_GCP_SPAN_OP,
[SENTRY_KIND]: 'server',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.firebase.otel.functions',
'faas.name': functionName,
Expand All @@ -87,7 +89,6 @@ export function patchV2Functions<T extends FirebaseFunctions = FirebaseFunctions
return startSpanManual(
{
name: `firebase.function.${triggerType}`,
op: 'http.request',

@cursor cursor Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wrong Firebase path updated

High Severity

The production change sets function.gcp only on the unused OTEL Firebase patch, while the public default firebaseIntegration (orchestrion) still opens spans with function.firebase. The e2e expectations for auto.firebase.orchestrion.functions were updated to function.gcp, so the intended migration does not land on the path users and those tests actually exercise.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2c8a0b1. Configure here.

attributes,
},
async span => {
Expand Down
Loading