-
Notifications
You must be signed in to change notification settings - Fork 149
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
refactor: Add settings attributes to trace spans. #2181
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 |
---|---|---|
|
@@ -25,10 +25,17 @@ import { | |
} from '@opentelemetry/api'; | ||
|
||
import {Span} from './span'; | ||
import {Attributes, TraceUtil} from './trace-util'; | ||
import {ATTRIBUTE_SETTINGS_PREFIX, Attributes, TraceUtil} from './trace-util'; | ||
|
||
import {interfaces} from '../v1/firestore_client_config.json'; | ||
import {FirestoreClient} from '../v1'; | ||
import {DEFAULT_DATABASE_ID} from '../path'; | ||
import {DEFAULT_MAX_IDLE_CHANNELS} from '../index'; | ||
const serviceConfig = interfaces['google.firestore.v1.Firestore']; | ||
|
||
export class EnabledTraceUtil implements TraceUtil { | ||
private tracer: Tracer; | ||
private settingsAttributes: Attributes; | ||
|
||
constructor(settings: Settings) { | ||
let tracerProvider = settings.openTelemetryOptions?.tracerProvider; | ||
|
@@ -42,6 +49,85 @@ export class EnabledTraceUtil implements TraceUtil { | |
const libVersion = require('../../../package.json').version; | ||
const libName = require('../../../package.json').name; | ||
this.tracer = tracerProvider.getTracer(libName, libVersion); | ||
|
||
this.settingsAttributes = {}; | ||
this.settingsAttributes['otel.scope.name'] = libName; | ||
this.settingsAttributes['otel.scope.version'] = libVersion; | ||
|
||
if (settings.projectId) { | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.project_id`] = | ||
settings.projectId; | ||
} | ||
|
||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.database_id`] = | ||
settings.databaseId || DEFAULT_DATABASE_ID; | ||
|
||
const host = | ||
settings.servicePath ?? settings.host ?? 'firestore.googleapis.com'; | ||
const port = settings.port ?? FirestoreClient.port; | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.host`] = | ||
`${host}:${port}`; | ||
|
||
if (settings.preferRest !== undefined) { | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.prefer_REST`] = | ||
settings.preferRest; | ||
} | ||
|
||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.max_idle_channels`] = | ||
settings.maxIdleChannels ?? DEFAULT_MAX_IDLE_CHANNELS; | ||
|
||
const defaultRetrySettings = serviceConfig.retry_params.default; | ||
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. why there are retry and timeout configuration? how this is related to otel tracing? 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. this code is adding trace attributes to the spans. The purpose is that if a user has misconfigured their SDK, their configurations will surface in the trace and may provide insights into the issues they're running into. This is similar to the Java impl. Although not all java configurations exist in the node.js SDK. |
||
const customRetrySettings = | ||
settings.clientConfig?.interfaces?.['google.firestore.v1.Firestore']?.[ | ||
'retry_params' | ||
]?.['default']; | ||
this.settingsAttributes[ | ||
`${ATTRIBUTE_SETTINGS_PREFIX}.initial_retry_delay` | ||
] = this.millisToSecondString( | ||
customRetrySettings?.initial_retry_delay_millis ?? | ||
defaultRetrySettings.initial_retry_delay_millis | ||
); | ||
this.settingsAttributes[ | ||
`${ATTRIBUTE_SETTINGS_PREFIX}.initial_rpc_timeout` | ||
] = this.millisToSecondString( | ||
customRetrySettings?.initial_rpc_timeout_millis ?? | ||
defaultRetrySettings.initial_rpc_timeout_millis | ||
); | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.total_timeout`] = | ||
this.millisToSecondString( | ||
customRetrySettings?.total_timeout_millis ?? | ||
defaultRetrySettings.total_timeout_millis | ||
); | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.max_retry_delay`] = | ||
this.millisToSecondString( | ||
customRetrySettings?.max_retry_delay_millis ?? | ||
defaultRetrySettings.max_retry_delay_millis | ||
); | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.max_rpc_timeout`] = | ||
this.millisToSecondString( | ||
customRetrySettings?.max_rpc_timeout_millis ?? | ||
defaultRetrySettings.max_rpc_timeout_millis | ||
); | ||
this.settingsAttributes[ | ||
`${ATTRIBUTE_SETTINGS_PREFIX}.retry_delay_multiplier` | ||
] = | ||
customRetrySettings?.retry_delay_multiplier.toString() ?? | ||
defaultRetrySettings.retry_delay_multiplier.toString(); | ||
this.settingsAttributes[ | ||
`${ATTRIBUTE_SETTINGS_PREFIX}.rpc_timeout_multiplier` | ||
] = | ||
customRetrySettings?.rpc_timeout_multiplier.toString() ?? | ||
defaultRetrySettings.rpc_timeout_multiplier.toString(); | ||
} | ||
|
||
recordProjectId(projectId: string): void { | ||
this.settingsAttributes[`${ATTRIBUTE_SETTINGS_PREFIX}.project_id`] = | ||
projectId; | ||
this.currentSpan().setAttributes(this.settingsAttributes); | ||
} | ||
|
||
private millisToSecondString(millis: number): string { | ||
return `${millis / 1000}s`; | ||
} | ||
|
||
private endSpan(otelSpan: OpenTelemetrySpan, error: Error): void { | ||
|
@@ -64,6 +150,8 @@ export class EnabledTraceUtil implements TraceUtil { | |
attributes: attributes, | ||
}, | ||
(otelSpan: OpenTelemetrySpan) => { | ||
this.addCommonAttributes(otelSpan); | ||
|
||
// Note that if `fn` returns a `Promise`, we want the otelSpan to end | ||
// after the `Promise` has resolved, NOT after the `fn` has returned. | ||
// Therefore, we should not use a `finally` clause to end the otelSpan. | ||
|
@@ -94,10 +182,16 @@ export class EnabledTraceUtil implements TraceUtil { | |
} | ||
|
||
startSpan(name: string): Span { | ||
return new Span(this.tracer.startSpan(name, undefined, context.active())); | ||
const otelSpan = this.tracer.startSpan(name, undefined, context.active()); | ||
this.addCommonAttributes(otelSpan); | ||
return new Span(otelSpan); | ||
} | ||
|
||
currentSpan(): Span { | ||
return new Span(trace.getActiveSpan()); | ||
} | ||
|
||
addCommonAttributes(otelSpan: OpenTelemetrySpan): void { | ||
otelSpan.setAttributes(this.settingsAttributes); | ||
} | ||
} |
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.
Since we are now capturing settings as part of the trace attributes, if the settings change we need to also change the traceUtil instance. So we should not re-use the existing one.