Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions common/api-review/telemetry-angular.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface Telemetry {

// @public
export interface TelemetryOptions {
appVersion?: string;
endpointUrl?: string;
}

Expand Down
1 change: 1 addition & 0 deletions common/api-review/telemetry-react.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface Telemetry {

// @public
export interface TelemetryOptions {
appVersion?: string;
endpointUrl?: string;
}

Expand Down
1 change: 1 addition & 0 deletions common/api-review/telemetry.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface Telemetry {

// @public
export interface TelemetryOptions {
appVersion?: string;
endpointUrl?: string;
}

Expand Down
11 changes: 11 additions & 0 deletions docs-devsite/telemetry_.telemetryoptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,19 @@ export interface TelemetryOptions

| Property | Type | Description |
| --- | --- | --- |
| [appVersion](./telemetry_.telemetryoptions.md#telemetryoptionsappversion) | string | The version of the application. This should be a unique string that identifies the snapshot of code to be deployed, such as "1.0.2". If not specified, other default locations will be checked for an identifier. Setting a value here takes precedent over any other values. |
| [endpointUrl](./telemetry_.telemetryoptions.md#telemetryoptionsendpointurl) | string | The URL for the endpoint to which telemetry data should be sent, in the Open Telemetry format. By default, data will be sent to Firebase. |

## TelemetryOptions.appVersion

The version of the application. This should be a unique string that identifies the snapshot of code to be deployed, such as "1.0.2". If not specified, other default locations will be checked for an identifier. Setting a value here takes precedent over any other values.

<b>Signature:</b>

```typescript
appVersion?: string;
```

## TelemetryOptions.endpointUrl

The URL for the endpoint to which telemetry data should be sent, in the Open Telemetry format. By default, data will be sent to Firebase.
Expand Down
11 changes: 11 additions & 0 deletions docs-devsite/telemetry_angular.telemetryoptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,19 @@ export interface TelemetryOptions

| Property | Type | Description |
| --- | --- | --- |
| [appVersion](./telemetry_angular.telemetryoptions.md#telemetryoptionsappversion) | string | The version of the application. This should be a unique string that identifies the snapshot of code to be deployed, such as "1.0.2". If not specified, other default locations will be checked for an identifier. Setting a value here takes precedent over any other values. |
| [endpointUrl](./telemetry_angular.telemetryoptions.md#telemetryoptionsendpointurl) | string | The URL for the endpoint to which telemetry data should be sent, in the Open Telemetry format. By default, data will be sent to Firebase. |

## TelemetryOptions.appVersion

The version of the application. This should be a unique string that identifies the snapshot of code to be deployed, such as "1.0.2". If not specified, other default locations will be checked for an identifier. Setting a value here takes precedent over any other values.

<b>Signature:</b>

```typescript
appVersion?: string;
```

## TelemetryOptions.endpointUrl

The URL for the endpoint to which telemetry data should be sent, in the Open Telemetry format. By default, data will be sent to Firebase.
Expand Down
11 changes: 11 additions & 0 deletions docs-devsite/telemetry_react.telemetryoptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,19 @@ export interface TelemetryOptions

| Property | Type | Description |
| --- | --- | --- |
| [appVersion](./telemetry_react.telemetryoptions.md#telemetryoptionsappversion) | string | The version of the application. This should be a unique string that identifies the snapshot of code to be deployed, such as "1.0.2". If not specified, other default locations will be checked for an identifier. Setting a value here takes precedent over any other values. |
| [endpointUrl](./telemetry_react.telemetryoptions.md#telemetryoptionsendpointurl) | string | The URL for the endpoint to which telemetry data should be sent, in the Open Telemetry format. By default, data will be sent to Firebase. |

## TelemetryOptions.appVersion

The version of the application. This should be a unique string that identifies the snapshot of code to be deployed, such as "1.0.2". If not specified, other default locations will be checked for an identifier. Setting a value here takes precedent over any other values.

<b>Signature:</b>

```typescript
appVersion?: string;
```

## TelemetryOptions.endpointUrl

The URL for the endpoint to which telemetry data should be sent, in the Open Telemetry format. By default, data will be sent to Firebase.
Expand Down
2 changes: 1 addition & 1 deletion packages/telemetry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
"@angular/core": "19.2.15",
"@angular/platform-browser": "19.2.15",
"@angular/router": "19.2.15",
"@firebase/app": "0.14.4",
"@firebase/app": "0.14.6",
"@opentelemetry/sdk-trace-web": "2.1.0",
"@rollup/plugin-json": "6.1.0",
"@testing-library/dom": "10.4.1",
Expand Down
34 changes: 30 additions & 4 deletions packages/telemetry/src/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ describe('Top level API', () => {
expect(log.body).to.equal('This is a test error');
expect(log.attributes).to.deep.equal({
'error.type': 'TestError',
'error.stack': '...stack trace...'
'error.stack': '...stack trace...',
'app.version': 'unset'
});
});

Expand All @@ -140,7 +141,8 @@ describe('Top level API', () => {
expect(log.body).to.equal('error with no stack');
expect(log.attributes).to.deep.equal({
'error.type': 'Error',
'error.stack': 'No stack trace available'
'error.stack': 'No stack trace available',
'app.version': 'unset'
});
});

Expand All @@ -151,7 +153,9 @@ describe('Top level API', () => {
const log = emittedLogs[0];
expect(log.severityNumber).to.equal(SeverityNumber.ERROR);
expect(log.body).to.equal('a string error');
expect(log.attributes).to.deep.equal({});
expect(log.attributes).to.deep.equal({
'app.version': 'unset'
});
});

it('should capture an unknown error type correctly', () => {
Expand All @@ -161,7 +165,9 @@ describe('Top level API', () => {
const log = emittedLogs[0];
expect(log.severityNumber).to.equal(SeverityNumber.ERROR);
expect(log.body).to.equal('Unknown error type: number');
expect(log.attributes).to.deep.equal({});
expect(log.attributes).to.deep.equal({
'app.version': 'unset'
});
});

it('should propagate trace context', async () => {
Expand All @@ -187,6 +193,7 @@ describe('Top level API', () => {
expect(emittedLogs[0].attributes).to.deep.equal({
'error.type': 'TestError',
'error.stack': '...stack trace...',
'app.version': 'unset',
'logging.googleapis.com/trace': `projects/${PROJECT_ID}/traces/my-trace`,
'logging.googleapis.com/spanId': `my-span`
});
Expand All @@ -211,6 +218,7 @@ describe('Top level API', () => {
expect(log.attributes).to.deep.equal({
'error.type': 'TestError',
'error.stack': '...stack trace...',
'app.version': 'unset',
strAttr: 'string attribute',
mapAttr: {
boolAttr: true,
Expand All @@ -219,6 +227,24 @@ describe('Top level API', () => {
arrAttr: [1, 2, 3]
});
});

it('should use explicit app version when provided', () => {
const telemetry = new TelemetryService(
fakeTelemetry.app,
fakeTelemetry.loggerProvider
);
telemetry.options = {
appVersion: '1.0.0'
};

captureError(telemetry, 'a string error');

expect(emittedLogs.length).to.equal(1);
const log = emittedLogs[0];
expect(log.attributes).to.deep.equal({
'app.version': '1.0.0'
});
});
});

describe('flush()', () => {
Expand Down
34 changes: 20 additions & 14 deletions packages/telemetry/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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

Choose a reason for hiding this comment

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

medium

The current check for appVersion uses a truthiness check, which will incorrectly handle an empty string ('') as a version. If a user explicitly provides an empty string, it will be treated as falsy, and the appVersion will default to 'unset'. To correctly handle all string values, including empty ones, you should check for null or undefined instead.

Suggested change
if ((telemetry as TelemetryService).options?.appVersion) {
appVersion = (telemetry as TelemetryService).options!.appVersion!;
}
if ((telemetry as TelemetryService).options?.appVersion != null) {
appVersion = (telemetry as TelemetryService).options.appVersion;
}

Copy link
Author

Choose a reason for hiding this comment

The 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({
Expand All @@ -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
});
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/telemetry/src/public-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,11 @@ export interface TelemetryOptions {
* By default, data will be sent to Firebase.
*/
endpointUrl?: string;

/**
* The version of the application. This should be a unique string that identifies the snapshot of
* code to be deployed, such as "1.0.2". If not specified, other default locations will be checked
* for an identifier. Setting a value here takes precedent over any other values.
*/
appVersion?: string;
}
12 changes: 11 additions & 1 deletion packages/telemetry/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
private _options?: TelemetryOptions;

constructor(public app: FirebaseApp, public loggerProvider: LoggerProvider) {}

_delete(): Promise<void> {
return Promise.resolve();
}

set options(optionsToSet: TelemetryOptions) {
this._options = optionsToSet;
}

get options(): TelemetryOptions | undefined {
return this._options;
}
}
11 changes: 0 additions & 11 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1360,17 +1360,6 @@
resolved "https://registry.npmjs.org/@eslint/js/-/js-8.57.1.tgz#de633db3ec2ef6a3c89e2f19038063e8a122e2c2"
integrity sha512-d9zaMRSTIKDLhctzH12MtXvJKSSUhaHcjV+2Z+GK+EEY7XKpP5yR4x+N3TAcHTcu963nIr+TMcCb4DBCYX1z6Q==

"@firebase/app@0.14.4":
version "0.14.4"
resolved "https://registry.npmjs.org/@firebase/app/-/app-0.14.4.tgz#1d2ce74c09752dec9664e2f981b20335c4efbec1"
integrity sha512-pUxEGmR+uu21OG/icAovjlu1fcYJzyVhhT0rsCrn+zi+nHtrS43Bp9KPn9KGa4NMspCUE++nkyiqziuIvJdwzw==
dependencies:
"@firebase/component" "0.7.0"
"@firebase/logger" "0.5.0"
"@firebase/util" "1.13.0"
idb "7.1.1"
tslib "^2.1.0"

"@gar/promisify@^1.0.1":
version "1.1.3"
resolved "https://registry.npmjs.org/@gar/promisify/-/promisify-1.1.3.tgz#555193ab2e3bb3b6adc3d551c9c030d9e860daf6"
Expand Down
Loading