-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(opentelemetry): Respect OTEL_SERVICE_NAME, OTEL_RESOURCE_ATTRIBUTES #20509
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
base: develop
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -39,18 +39,58 @@ class SentryResource { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Parses `OTEL_RESOURCE_ATTRIBUTES` env var (comma-separated `key=value` pairs). | ||
| * Values are URL-decoded per the OTel spec. | ||
| */ | ||
| function parseOtelResourceAttributes(raw: string | undefined): Attributes { | ||
| if (!raw) { | ||
| return {}; | ||
| } | ||
| const result: Attributes = {}; | ||
| for (const pair of raw.split(',')) { | ||
| const eq = pair.trim().indexOf('='); | ||
| if (eq === -1) { | ||
| continue; | ||
| } | ||
| const key = pair.substring(0, eq).trim(); | ||
| const value = pair.substring(eq + 1).trim(); | ||
|
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. Leading whitespace corrupts parsed key/value pairsHigh Severity In Reviewed by Cursor Bugbot for commit 65f6897. Configure here. |
||
| if (key) { | ||
| try { | ||
| result[key] = decodeURIComponent(value); | ||
| } catch { | ||
| result[key] = value; | ||
| } | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| /** | ||
| * Returns a Resource for use in Sentry's OpenTelemetry TracerProvider setup. | ||
| * | ||
| * Combines the default OTel SDK telemetry attributes with Sentry-specific | ||
| * service attributes, equivalent to what was previously done via: | ||
| * `defaultResource().merge(resourceFromAttributes({ ... }))` | ||
| * | ||
| * Respects OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES environment variables | ||
| * per the OpenTelemetry specification. | ||
| */ | ||
| export function getSentryResource(serviceName: string): SentryResource { | ||
| export function getSentryResource(serviceNameFallback: string): SentryResource { | ||
| const env = typeof process !== 'undefined' ? process.env : {}; | ||
| const otelServiceName = env.OTEL_SERVICE_NAME; | ||
| const otelResourceAttrs = parseOtelResourceAttributes(env.OTEL_RESOURCE_ATTRIBUTES); | ||
|
|
||
| return new SentryResource({ | ||
| [ATTR_SERVICE_NAME]: serviceName, | ||
| // Lowest priority: Sentry defaults | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| [SEMRESATTRS_SERVICE_NAMESPACE]: 'sentry', | ||
| [ATTR_SERVICE_NAME]: serviceNameFallback, | ||
| // OTEL_RESOURCE_ATTRIBUTES overrides defaults (including service.name and service.namespace) | ||
| ...otelResourceAttrs, | ||
| // OTEL_SERVICE_NAME explicitly overrides service.name | ||
| ...(otelServiceName ? { [ATTR_SERVICE_NAME]: otelServiceName } : {}), | ||
| // Highest priority: Sentry SDK telemetry attrs (cannot be overridden by env vars) | ||
| [ATTR_SERVICE_VERSION]: SDK_VERSION, | ||
| [ATTR_TELEMETRY_SDK_LANGUAGE]: SDK_INFO[ATTR_TELEMETRY_SDK_LANGUAGE], | ||
| [ATTR_TELEMETRY_SDK_NAME]: SDK_INFO[ATTR_TELEMETRY_SDK_NAME], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| import { | ||
| ATTR_SERVICE_NAME, | ||
| ATTR_SERVICE_VERSION, | ||
| ATTR_TELEMETRY_SDK_LANGUAGE, | ||
| ATTR_TELEMETRY_SDK_NAME, | ||
| ATTR_TELEMETRY_SDK_VERSION, | ||
| SEMRESATTRS_SERVICE_NAMESPACE, | ||
| } from '@opentelemetry/semantic-conventions'; | ||
| import { SDK_VERSION } from '@sentry/core'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { getSentryResource } from '../src/resource'; | ||
|
|
||
| describe('getSentryResource', () => { | ||
| const originalEnv = process.env; | ||
|
|
||
| beforeEach(() => { | ||
| // Clone env so mutations are isolated | ||
| process.env = { ...originalEnv }; | ||
| delete process.env['OTEL_SERVICE_NAME']; | ||
| delete process.env['OTEL_RESOURCE_ATTRIBUTES']; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.env = originalEnv; | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('uses serviceNameFallback when no env vars are set', () => { | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_SERVICE_NAME]).toBe('node'); | ||
| }); | ||
|
|
||
| it('uses OTEL_SERVICE_NAME over the fallback', () => { | ||
| process.env['OTEL_SERVICE_NAME'] = 'my-service'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_SERVICE_NAME]).toBe('my-service'); | ||
| }); | ||
|
|
||
| it('ignores empty OTEL_SERVICE_NAME and falls back to serviceNameFallback', () => { | ||
| process.env['OTEL_SERVICE_NAME'] = ''; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_SERVICE_NAME]).toBe('node'); | ||
| }); | ||
|
|
||
| it('includes OTEL_RESOURCE_ATTRIBUTES key=value pairs', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'custom.key=custom-value,another.key=another-value'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes['custom.key']).toBe('custom-value'); | ||
| expect(resource.attributes['another.key']).toBe('another-value'); | ||
| }); | ||
|
|
||
| it('OTEL_RESOURCE_ATTRIBUTES can override service.name (but OTEL_SERVICE_NAME takes precedence over it)', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'service.name=from-attrs'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_SERVICE_NAME]).toBe('from-attrs'); | ||
| }); | ||
|
|
||
| it('OTEL_SERVICE_NAME takes precedence over service.name from OTEL_RESOURCE_ATTRIBUTES', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'service.name=from-attrs'; | ||
| process.env['OTEL_SERVICE_NAME'] = 'from-service-name'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_SERVICE_NAME]).toBe('from-service-name'); | ||
| }); | ||
|
|
||
| it('OTEL_RESOURCE_ATTRIBUTES can override service.namespace', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'service.namespace=my-namespace'; | ||
| const resource = getSentryResource('node'); | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| expect(resource.attributes[SEMRESATTRS_SERVICE_NAMESPACE]).toBe('my-namespace'); | ||
| }); | ||
|
|
||
| it('Sentry SDK telemetry attrs cannot be overridden by OTEL_RESOURCE_ATTRIBUTES', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = | ||
| 'telemetry.sdk.name=evil,telemetry.sdk.language=evil,telemetry.sdk.version=0.0.0'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_TELEMETRY_SDK_NAME]).not.toBe('evil'); | ||
| expect(resource.attributes[ATTR_TELEMETRY_SDK_LANGUAGE]).not.toBe('evil'); | ||
| expect(resource.attributes[ATTR_TELEMETRY_SDK_VERSION]).not.toBe('0.0.0'); | ||
| }); | ||
|
|
||
| it('Sentry SDK telemetry attrs cannot be overridden by OTEL_SERVICE_NAME (service.version)', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'service.version=0.0.0'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_SERVICE_VERSION]).toBe(SDK_VERSION); | ||
| }); | ||
|
|
||
| it('always includes Sentry SDK telemetry attributes', () => { | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes[ATTR_TELEMETRY_SDK_LANGUAGE]).toBeDefined(); | ||
| expect(resource.attributes[ATTR_TELEMETRY_SDK_NAME]).toBeDefined(); | ||
| expect(resource.attributes[ATTR_TELEMETRY_SDK_VERSION]).toBeDefined(); | ||
| expect(resource.attributes[ATTR_SERVICE_VERSION]).toBe(SDK_VERSION); | ||
| }); | ||
|
|
||
| it('always sets service.namespace to sentry by default', () => { | ||
| const resource = getSentryResource('node'); | ||
| // eslint-disable-next-line deprecation/deprecation | ||
| expect(resource.attributes[SEMRESATTRS_SERVICE_NAMESPACE]).toBe('sentry'); | ||
| }); | ||
|
|
||
| it('URL-decodes values in OTEL_RESOURCE_ATTRIBUTES', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'custom.key=hello%20world'; | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes['custom.key']).toBe('hello world'); | ||
| }); | ||
|
|
||
| it('handles malformed OTEL_RESOURCE_ATTRIBUTES gracefully (no = sign)', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = 'badentry,custom.key=value'; | ||
| expect(() => getSentryResource('node')).not.toThrow(); | ||
| const resource = getSentryResource('node'); | ||
| expect(resource.attributes['custom.key']).toBe('value'); | ||
| }); | ||
|
|
||
| it('handles empty OTEL_RESOURCE_ATTRIBUTES gracefully', () => { | ||
| process.env['OTEL_RESOURCE_ATTRIBUTES'] = ''; | ||
| expect(() => getSentryResource('node')).not.toThrow(); | ||
| }); | ||
|
|
||
| it('does not crash when process is undefined', () => { | ||
| const saved = global.process; | ||
| // @ts-expect-error — simulating edge runtime where process may be undefined | ||
| global.process = undefined; | ||
| try { | ||
| expect(() => getSentryResource('node')).not.toThrow(); | ||
| } finally { | ||
| global.process = saved; | ||
| } | ||
| }); | ||
| }); |


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.
Bug: The code calculates an index on a trimmed string but applies it to the untrimmed string, causing incorrect parsing of resource attributes with leading spaces.
Severity: HIGH
Suggested Fix
Trim the
pairstring first and store it in a variable. Use this new trimmed variable for both finding the index of the=separator and for the subsequentsubstringcalls to extract the key and value.Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.