From b0cbbe002b7013386ab1763de6162be301127328 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 6 Feb 2025 19:35:48 +0100 Subject: [PATCH 1/2] feat(resources)!: do not read environment variables from window --- CHANGELOG.md | 3 + .../src/detectors/EnvDetector.ts | 7 +- .../detectors/browser/EnvDetector.test.ts | 71 ++----------------- 3 files changed, 10 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cd26659814..688f281f8fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,9 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se * (user-facing): `baggageUtils.parsePairKeyValue` was an internal utility function that was unintentionally exported. It has been removed without replacement. * (user-facing): `TimeOriginLegacy` has been removed without replacement. * (user-facing): `isAttributeKey` was an internal utility function that was unintentionally exported. It has been removed without replacement. +* feat(resources)!: do not read environment variables from window in browsers [#5466](https://github.com/open-telemetry/opentelemetry-js/pull/5466) @pichlermarc + * (user-facing): all configuration previously possible via `window.OTEL_*` is now not supported anymore, please pass configuration options to constructors instead. + * Note: Node.js environment variable configuration continues to work as-is. ### :rocket: (Enhancement) diff --git a/packages/opentelemetry-resources/src/detectors/EnvDetector.ts b/packages/opentelemetry-resources/src/detectors/EnvDetector.ts index 5c42a4d5e07..dd9712a7a21 100644 --- a/packages/opentelemetry-resources/src/detectors/EnvDetector.ts +++ b/packages/opentelemetry-resources/src/detectors/EnvDetector.ts @@ -15,10 +15,10 @@ */ import { Attributes, diag } from '@opentelemetry/api'; -import { getEnv } from '@opentelemetry/core'; import { SEMRESATTRS_SERVICE_NAME } from '@opentelemetry/semantic-conventions'; import { ResourceDetectionConfig } from '../config'; import { DetectedResource, ResourceDetector } from '../types'; +import { getStringFromEnv } from '@opentelemetry/core'; /** * EnvDetector can be used to detect the presence of and create a Resource @@ -53,10 +53,9 @@ class EnvDetector implements ResourceDetector { */ detect(_config?: ResourceDetectionConfig): DetectedResource { const attributes: Attributes = {}; - const env = getEnv(); - const rawAttributes = env.OTEL_RESOURCE_ATTRIBUTES; - const serviceName = env.OTEL_SERVICE_NAME; + const rawAttributes = getStringFromEnv('OTEL_RESOURCE_ATTRIBUTES'); + const serviceName = getStringFromEnv('OTEL_SERVICE_NAME'); if (rawAttributes) { try { diff --git a/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts b/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts index a9e4f64dfcf..573700698e1 100644 --- a/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts +++ b/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts @@ -14,76 +14,13 @@ * limitations under the License. */ -import { RAW_ENVIRONMENT } from '@opentelemetry/core'; -import * as assert from 'assert'; import { envDetector } from '../../../src'; import { describeBrowser } from '../../util'; -import { - assertEmptyResource, - assertWebEngineResource, -} from '../../util/resource-assertions'; +import { assertEmptyResource } from '../../util/resource-assertions'; describeBrowser('envDetector() on web browser', () => { - describe('with valid env', () => { - before(() => { - ( - globalThis as typeof globalThis & RAW_ENVIRONMENT - ).OTEL_RESOURCE_ATTRIBUTES = - 'webengine.name="chromium",webengine.version="99",webengine.description="Chromium",custom.key="custom%20value"'; - }); - - after(() => { - delete (globalThis as typeof globalThis & RAW_ENVIRONMENT) - .OTEL_RESOURCE_ATTRIBUTES; - }); - - it('should return resource information from environment variable', async () => { - const resource = envDetector.detect(); - assert.ok(resource.attributes); - assertWebEngineResource(resource, { - name: 'chromium', - version: '99', - description: 'Chromium', - }); - assert.strictEqual(resource.attributes['custom.key'], 'custom value'); - }); - }); - - describe('with invalid env', () => { - const values = ['webengine.description="with spaces"']; - - for (const value of values) { - describe(`value: '${value}'`, () => { - before(() => { - ( - globalThis as typeof globalThis & RAW_ENVIRONMENT - ).OTEL_RESOURCE_ATTRIBUTES = value; - }); - - after(() => { - delete (globalThis as typeof globalThis & RAW_ENVIRONMENT) - .OTEL_RESOURCE_ATTRIBUTES; - }); - - it('should return empty resource', async () => { - const resource = envDetector.detect(); - assertEmptyResource(resource); - }); - }); - } - }); - - describe('with empty env', () => { - it('should return empty resource', async () => { - const resource = envDetector.detect(); - assertEmptyResource(resource); - }); - }); - - describe('with empty env', () => { - it('should return empty resource', async () => { - const resource = envDetector.detect(); - assertEmptyResource(resource); - }); + it('should return empty resource', async () => { + const resource = envDetector.detect(); + assertEmptyResource(resource); }); }); From b100bd66b68150f2a348bdc3fc2d081735290bfe Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 13 Feb 2025 12:21:11 +0100 Subject: [PATCH 2/2] fixup! feat(resources)!: do not read environment variables from window --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 688f281f8fa..c2826c05c18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,7 +93,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se * (user-facing): `TimeOriginLegacy` has been removed without replacement. * (user-facing): `isAttributeKey` was an internal utility function that was unintentionally exported. It has been removed without replacement. * feat(resources)!: do not read environment variables from window in browsers [#5466](https://github.com/open-telemetry/opentelemetry-js/pull/5466) @pichlermarc - * (user-facing): all configuration previously possible via `window.OTEL_*` is now not supported anymore, please pass configuration options to constructors instead. + * (user-facing): all configuration previously possible via `window.OTEL_*` is now not supported anymore + * If you have been using the `envDetector` in browser environments, please migrate to manually creating a resource. * Note: Node.js environment variable configuration continues to work as-is. ### :rocket: (Enhancement)