-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(server-utils): Migrate FirebaseInstrumentation to orchestrion
#22141
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,22 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
|
|
||
| // When `E2E_ORCHESTRION=true`, exercise the diagnostics-channel injection path (the orchestrion-based | ||
| // `Firebase` integration) instead of the OTel one. Opting in before `init()` is enough: this file is | ||
| // imported before `app.ts` imports `firebase/firestore/lite`, so the channel-injection hooks are | ||
| // installed before firestore loads. | ||
| const useOrchestrion = process.env.E2E_ORCHESTRION === 'true'; | ||
|
|
||
| if (useOrchestrion) { | ||
| Sentry.experimentalUseDiagnosticsChannelInjection(); | ||
| } | ||
|
|
||
| Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| tracesSampleRate: 1.0, | ||
| integrations: [Sentry.firebaseIntegration()], | ||
| integrations: useOrchestrion | ||
| ? [Sentry.diagnosticsChannelInjectionIntegrations().firebaseIntegration()] | ||
| : [Sentry.firebaseIntegration()], | ||
| defaultIntegrations: false, | ||
| tunnel: `http://localhost:3031/`, // proxy server | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
|
|
||
| // Minimal structural types inlined from `firebase/app` and `firebase/firestore`, kept just wide enough | ||
| // for the attributes the subscriber reads off a Firestore reference. Inlined (rather than imported) so | ||
| // `@sentry/server-utils` needs no firebase dependency. | ||
|
|
||
| export interface FirebaseOptions { | ||
| [key: string]: any; | ||
| apiKey?: string; | ||
| projectId?: string; | ||
| appId?: string; | ||
| messagingSenderId?: string; | ||
| storageBucket?: string; | ||
| } | ||
|
|
||
| export interface FirebaseApp { | ||
| name: string; | ||
| options: FirebaseOptions; | ||
| } | ||
|
|
||
| export interface FirestoreSettings { | ||
| host?: string; | ||
| ssl?: boolean; | ||
| } | ||
|
|
||
| interface FirestoreLike { | ||
| app: FirebaseApp; | ||
| settings: FirestoreSettings; | ||
| toJSON: () => { app: FirebaseApp; settings: FirestoreSettings }; | ||
| } | ||
|
|
||
| export interface DocumentData { | ||
| [field: string]: any; | ||
| } | ||
|
|
||
| export interface DocumentReference { | ||
| id: string; | ||
| firestore: FirestoreLike; | ||
| type: string; | ||
| path: string; | ||
| parent: CollectionReference | null; | ||
| } | ||
|
|
||
| export interface CollectionReference { | ||
| id: string; | ||
| firestore: FirestoreLike; | ||
| type: string; | ||
| path: string; | ||
| parent: DocumentReference | null; | ||
| } | ||
|
|
||
| export type FirestoreReference = CollectionReference | DocumentReference; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| import * as net from 'node:net'; | ||
| import { | ||
| DB_COLLECTION_NAME, | ||
| DB_NAMESPACE, | ||
| DB_OPERATION_NAME, | ||
| DB_SYSTEM_NAME, | ||
| SERVER_ADDRESS, | ||
| SERVER_PORT, | ||
| } from '@sentry/conventions/attributes'; | ||
| import type { Span, SpanAttributes } from '@sentry/core'; | ||
| import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_KIND, startInactiveSpan } from '@sentry/core'; | ||
| import type { FirebaseApp, FirebaseOptions, FirestoreReference, FirestoreSettings } from './firestore-types'; | ||
|
|
||
| /** | ||
| * Opens the inactive `db.query` span for a Firestore operation. `bindTracingChannelToSpan` makes it the | ||
| * active span for the traced call and ends it when the call settles. Mirrors the OTel integration's span, | ||
| * with a distinct `auto.firebase.orchestrion.firestore` origin. | ||
| */ | ||
| export function startFirestoreSpan(spanName: string, reference: FirestoreReference): Span { | ||
| return startInactiveSpan({ | ||
| name: `${spanName} ${reference.path}`, | ||
| op: 'db.query', | ||
| kind: SPAN_KIND.CLIENT, | ||
| attributes: { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.firebase.orchestrion.firestore', | ||
| [DB_OPERATION_NAME]: spanName, | ||
| ...buildAttributes(reference), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the server address and port attributes from the Firestore settings. | ||
| * It's best effort to extract the address and port from the settings, especially for IPv6. | ||
| * @param settings - The Firestore settings containing host information. | ||
| */ | ||
| export function getPortAndAddress(settings: FirestoreSettings): { | ||
| address?: string; | ||
| port?: number; | ||
| } { | ||
| let address: string | undefined; | ||
| let port: string | undefined; | ||
|
|
||
| if (typeof settings.host === 'string') { | ||
| if (settings.host.startsWith('[')) { | ||
| // IPv6 addresses can be enclosed in square brackets, e.g., [2001:db8::1]:8080 | ||
| if (settings.host.endsWith(']')) { | ||
| // IPv6 with square brackets without port | ||
| address = settings.host.replace(/^\[|\]$/g, ''); | ||
| } else if (settings.host.includes(']:')) { | ||
| // IPv6 with square brackets with port | ||
| const lastColonIndex = settings.host.lastIndexOf(':'); | ||
| if (lastColonIndex !== -1) { | ||
| address = settings.host.slice(1, lastColonIndex).replace(/^\[|\]$/g, ''); | ||
| port = settings.host.slice(lastColonIndex + 1); | ||
| } | ||
| } | ||
| } else { | ||
| // IPv4 or IPv6 without square brackets | ||
| // If it's an IPv6 address without square brackets, we assume it does not have a port. | ||
| if (net.isIPv6(settings.host)) { | ||
| address = settings.host; | ||
| } | ||
| // If it's an IPv4 address, we can extract the port if it exists. | ||
| else { | ||
| const lastColonIndex = settings.host.lastIndexOf(':'); | ||
| if (lastColonIndex !== -1) { | ||
| address = settings.host.slice(0, lastColonIndex); | ||
| port = settings.host.slice(lastColonIndex + 1); | ||
| } else { | ||
| address = settings.host; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return { | ||
| address: address, | ||
| port: port ? parseInt(port, 10) : undefined, | ||
| }; | ||
| } | ||
|
|
||
| function buildAttributes(reference: FirestoreReference): SpanAttributes { | ||
| const firestoreApp: FirebaseApp = reference.firestore.app; | ||
| const firestoreOptions: FirebaseOptions = firestoreApp.options; | ||
| const json: { settings?: FirestoreSettings } = reference.firestore.toJSON() || {}; | ||
| const settings: FirestoreSettings = json.settings || {}; | ||
|
|
||
| const attributes: SpanAttributes = { | ||
| [DB_COLLECTION_NAME]: reference.path, | ||
| [DB_NAMESPACE]: firestoreApp.name, | ||
| [DB_SYSTEM_NAME]: 'firebase.firestore', | ||
| 'firebase.firestore.type': reference.type, | ||
| 'firebase.firestore.options.projectId': firestoreOptions.projectId, | ||
| 'firebase.firestore.options.appId': firestoreOptions.appId, | ||
| 'firebase.firestore.options.messagingSenderId': firestoreOptions.messagingSenderId, | ||
| 'firebase.firestore.options.storageBucket': firestoreOptions.storageBucket, | ||
| }; | ||
|
|
||
| const { address, port } = getPortAndAddress(settings); | ||
|
|
||
| if (address) { | ||
| attributes[SERVER_ADDRESS] = address; | ||
| } | ||
| if (port) { | ||
| attributes[SERVER_PORT] = port; | ||
| } | ||
|
|
||
| return attributes; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicated Firestore attribute helpersMedium Severity
Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit 538303f. Configure here. |
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absent attribute not asserted
Low Severity
The suite comment says orchestrion spans omit
otel.kind, but the expectation is built withexpect.objectContaining, which cannot fail if that attribute is still present. The negative case is therefore untested.Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 538303f. Configure here.