-
Notifications
You must be signed in to change notification settings - Fork 991
Add option to explicitly set app version for telemetry #9388
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 1 commit
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 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -54,7 +54,13 @@ export function getTelemetry( | |||||||||||||
| TELEMETRY_TYPE | ||||||||||||||
| ); | ||||||||||||||
| const identifier = options?.endpointUrl || ''; | ||||||||||||||
| return telemetryProvider.getImmediate({ identifier }); | ||||||||||||||
| const telemetry: TelemetryService = telemetryProvider.getImmediate({ | ||||||||||||||
| identifier | ||||||||||||||
| }); | ||||||||||||||
| if (options) { | ||||||||||||||
| telemetry.options = options; | ||||||||||||||
| } | ||||||||||||||
| return telemetry; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
|
|
@@ -72,20 +78,27 @@ export function captureError( | |||||||||||||
| attributes?: AnyValueMap | ||||||||||||||
| ): void { | ||||||||||||||
| const logger = telemetry.loggerProvider.getLogger('error-logger'); | ||||||||||||||
| const customAttributes = attributes || {}; | ||||||||||||||
|
|
||||||||||||||
| // Add trace metadata | ||||||||||||||
| const activeSpanContext = trace.getActiveSpan()?.spanContext(); | ||||||||||||||
| const traceAttributes = {} as AnyValueMap; | ||||||||||||||
| if (telemetry.app.options.projectId && activeSpanContext?.traceId) { | ||||||||||||||
| traceAttributes[ | ||||||||||||||
| customAttributes[ | ||||||||||||||
| 'logging.googleapis.com/trace' | ||||||||||||||
| ] = `projects/${telemetry.app.options.projectId}/traces/${activeSpanContext.traceId}`; | ||||||||||||||
| if (activeSpanContext?.spanId) { | ||||||||||||||
| traceAttributes['logging.googleapis.com/spanId'] = | ||||||||||||||
| customAttributes['logging.googleapis.com/spanId'] = | ||||||||||||||
| activeSpanContext.spanId; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const customAttributes = attributes || {}; | ||||||||||||||
| // Add app version metadata | ||||||||||||||
| let appVersion = 'unset'; | ||||||||||||||
| // TODO: implement app version fallback logic | ||||||||||||||
| if ((telemetry as TelemetryService).options?.appVersion) { | ||||||||||||||
| appVersion = (telemetry as TelemetryService).options!.appVersion!; | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+98
to
+100
Contributor
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. The current check for
Suggested change
Author
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. An empty string should map to unset |
||||||||||||||
| customAttributes['app.version'] = appVersion; | ||||||||||||||
|
|
||||||||||||||
| if (error instanceof Error) { | ||||||||||||||
| logger.emit({ | ||||||||||||||
|
|
@@ -94,27 +107,20 @@ export function captureError( | |||||||||||||
| attributes: { | ||||||||||||||
| 'error.type': error.name || 'Error', | ||||||||||||||
| 'error.stack': error.stack || 'No stack trace available', | ||||||||||||||
| ...traceAttributes, | ||||||||||||||
| ...customAttributes | ||||||||||||||
| } | ||||||||||||||
| }); | ||||||||||||||
| } else if (typeof error === 'string') { | ||||||||||||||
| logger.emit({ | ||||||||||||||
| severityNumber: SeverityNumber.ERROR, | ||||||||||||||
| body: error, | ||||||||||||||
| attributes: { | ||||||||||||||
| ...traceAttributes, | ||||||||||||||
| ...customAttributes | ||||||||||||||
| } | ||||||||||||||
| attributes: customAttributes | ||||||||||||||
| }); | ||||||||||||||
| } else { | ||||||||||||||
| logger.emit({ | ||||||||||||||
| severityNumber: SeverityNumber.ERROR, | ||||||||||||||
| body: `Unknown error type: ${typeof error}`, | ||||||||||||||
| attributes: { | ||||||||||||||
| ...traceAttributes, | ||||||||||||||
| ...customAttributes | ||||||||||||||
| } | ||||||||||||||
| attributes: customAttributes | ||||||||||||||
| }); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -16,13 +16,23 @@ | |||||
| */ | ||||||
|
|
||||||
| import { _FirebaseService, FirebaseApp } from '@firebase/app'; | ||||||
| import { Telemetry } from './public-types'; | ||||||
| import { Telemetry, TelemetryOptions } from './public-types'; | ||||||
| import { LoggerProvider } from '@opentelemetry/sdk-logs'; | ||||||
|
|
||||||
| export class TelemetryService implements Telemetry, _FirebaseService { | ||||||
| _options?: TelemetryOptions; | ||||||
|
||||||
| _options?: TelemetryOptions; | |
| private _options?: TelemetryOptions; |
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.
This test creates a plain object that structurally mimics a
TelemetryServiceinstance. This approach is not type-safe and relies on the internal implementation detail ofcaptureErrorcasting thetelemetryobject. A more robust and type-safe approach would be to instantiateTelemetryServicedirectly, which will make the test less brittle to future refactoring.