From 3800ac81079e0721a5ca549b0502906a1496aaaa Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 28 Dec 2023 21:48:58 -0800 Subject: [PATCH 01/21] fix(sdk-node): Allow tracerProvider to be created when exporter is defined in the env. --- .../opentelemetry-sdk-node/src/sdk.ts | 6 ++++- .../opentelemetry-sdk-node/test/sdk.test.ts | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 30a1bea790..d80a241139 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -126,7 +126,11 @@ export class NodeSDK { this._autoDetectResources = configuration.autoDetectResources ?? true; - if (configuration.spanProcessor || configuration.traceExporter) { + if ( + configuration.spanProcessor || + configuration.traceExporter || + env.OTEL_TRACES_EXPORTER + ) { const tracerProviderConfig: NodeTracerConfig = {}; if (configuration.sampler) { diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 6fb76f3dad..1df9f5c22d 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -176,6 +176,29 @@ describe('Node SDK', () => { assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider); }); + it('should register a tracer provider if an exporter is provided via env', async () => { + env.OTEL_TRACES_EXPORTER = 'console'; + const sdk = new NodeSDK({ + autoDetectResources: false, + }); + + sdk.start(); + + assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider)); + + assert.ok( + context['_getContextManager']().constructor.name === + DefaultContextManager.name + ); + assert.ok( + propagation['_getGlobalPropagator']() instanceof CompositePropagator + ); + const apiTracerProvider = + trace.getTracerProvider() as ProxyTracerProvider; + assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider); + delete env.OTEL_TRACES_EXPORTER; + }); + it('should register a tracer provider if a span processor is provided', async () => { const exporter = new ConsoleSpanExporter(); const spanProcessor = new SimpleSpanProcessor(exporter); From 971b5c96845ed8443e4d4e81ad4b1ceda8d0c546 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 28 Dec 2023 21:59:22 -0800 Subject: [PATCH 02/21] fix(node-sdk): Update to not accept when exporter is set to none. --- experimental/packages/opentelemetry-sdk-node/src/sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index d80a241139..f490851057 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -129,7 +129,7 @@ export class NodeSDK { if ( configuration.spanProcessor || configuration.traceExporter || - env.OTEL_TRACES_EXPORTER + (env.OTEL_TRACES_EXPORTER && env.OTEL_TRACES_EXPORTER !== 'none') ) { const tracerProviderConfig: NodeTracerConfig = {}; From 26c83e3fff29bdc2eecc64afdb892ffd9dfc7711 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 28 Dec 2023 22:03:23 -0800 Subject: [PATCH 03/21] fix(sdk-node): Update Changelog. --- experimental/CHANGELOG.md | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 4ffc9cdc6f..340f8c1579 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -10,37 +10,12 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) -fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc +* fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber ### :books: (Refine Doc) ### :house: (Internal) -## 0.46.0 - -### :boom: Breaking Change - -* fix(exporter-metrics-otlp-grpc): programmatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4334) @Vunovati -* fix(exporter-metrics-otlp-http): programmatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4334) @Vunovati -* fix(exporter-metrics-otlp-proto): programmatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4334) @Vunovati -* fix(otlp-exporter-base)!: decrease default concurrency limit to 30 [#4211](https://github.com/open-telemetry/opentelemetry-js/pull/4211) @pichlermarc - * fixes a memory leak on prolonged collector unavailability - * this change is marked as breaking as it changes defaults - -### :rocket: (Enhancement) - -* feat(sdk-logs): add droppedAttributesCount field to ReadableLogRecord - -### :bug: (Bug Fix) - -* fix(sdk-logs): await async resources in log processors -* fix(sdk-logs): avoid map attribute set when count limit exceeded -* fix(instrumentation-fetch): only access navigator if it is defined [#4063](https://github.com/open-telemetry/opentelemetry-js/pull/4063) - * allows for experimental usage of this instrumentation with non-browser runtimes -* fix(instrumentation-http): memory leak when responses are not resumed -* fix(instrumentation-http): Do not mutate given headers object for outgoing http requests. Fixes aws-sdk signing error on retries. [#4346](https://github.com/open-telemetry/opentelemetry-js/pull/4346) -* fix(instrumentation): support Node.js v18.19.0 by using import-in-the-middle@1.6.0 - ## 0.45.1 ### :bug: (Bug Fix) @@ -206,6 +181,8 @@ fix(instrumentation): use caret range on import-in-the-middle [#4380](https://gi * doc(instrumentation): add limitiations section to readme [#3786](https://github.com/open-telemetry/opentelemetry-js/pull/3786) @flarna +### :house: (Internal) + ## 0.38.0 ### :boom: Breaking Change @@ -422,6 +399,10 @@ fix(instrumentation): use caret range on import-in-the-middle [#4380](https://gi * fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc * fix(instrumentation-grpc): always set grpc semcov status code attribute with numeric value [#3076](https://github.com/open-telemetry/opentelemetry-js/pull/3076) @blumamir +### :books: (Refine Doc) + +### :house: (Internal) + ## 0.30.0 ### :boom: Breaking Change @@ -496,6 +477,10 @@ fix(instrumentation): use caret range on import-in-the-middle [#4380](https://gi * fix(metrics): specification compliant default metric unit [#2983](https://github.com/open-telemetry/opentelemetry-js/pull/2983) @andyfleming * fix(opentelemetry-instrumentation): use all provided patches for the same file [#2963](https://github.com/open-telemetry/opentelemetry-js/pull/2963) @Ugzuzg +### :books: (Refine Doc) + +### :house: (Internal) + ## 0.28.0 ### :boom: Breaking Change From 2f998f50d5d1421cdd3e9a0fc33771fbe2eb70e0 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 28 Dec 2023 22:07:28 -0800 Subject: [PATCH 04/21] fix(sdk-node): Fix Changelog. --- experimental/CHANGELOG.md | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 340f8c1579..52b3c3009c 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -9,13 +9,39 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) ### :bug: (Bug Fix) - * fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber +fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc + ### :books: (Refine Doc) ### :house: (Internal) +## 0.46.0 + +### :boom: Breaking Change + +* fix(exporter-metrics-otlp-grpc): programmatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4334) @Vunovati +* fix(exporter-metrics-otlp-http): programmatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4334) @Vunovati +* fix(exporter-metrics-otlp-proto): programmatic headers take precedence over environment variables [#2370](https://github.com/open-telemetry/opentelemetry-js/pull/4334) @Vunovati +* fix(otlp-exporter-base)!: decrease default concurrency limit to 30 [#4211](https://github.com/open-telemetry/opentelemetry-js/pull/4211) @pichlermarc + * fixes a memory leak on prolonged collector unavailability + * this change is marked as breaking as it changes defaults + +### :rocket: (Enhancement) + +* feat(sdk-logs): add droppedAttributesCount field to ReadableLogRecord + +### :bug: (Bug Fix) + +* fix(sdk-logs): await async resources in log processors +* fix(sdk-logs): avoid map attribute set when count limit exceeded +* fix(instrumentation-fetch): only access navigator if it is defined [#4063](https://github.com/open-telemetry/opentelemetry-js/pull/4063) + * allows for experimental usage of this instrumentation with non-browser runtimes +* fix(instrumentation-http): memory leak when responses are not resumed +* fix(instrumentation-http): Do not mutate given headers object for outgoing http requests. Fixes aws-sdk signing error on retries. [#4346](https://github.com/open-telemetry/opentelemetry-js/pull/4346) +* fix(instrumentation): support Node.js v18.19.0 by using import-in-the-middle@1.6.0 + ## 0.45.1 ### :bug: (Bug Fix) From 8e3f2662a03ae6c8314a156a45134c24370e01b4 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 28 Dec 2023 22:09:30 -0800 Subject: [PATCH 05/21] fix(sdk-node): cleanup changelog. --- experimental/CHANGELOG.md | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 52b3c3009c..035df3efe8 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -207,8 +207,6 @@ fix(instrumentation): use caret range on import-in-the-middle [#4380](https://gi * doc(instrumentation): add limitiations section to readme [#3786](https://github.com/open-telemetry/opentelemetry-js/pull/3786) @flarna -### :house: (Internal) - ## 0.38.0 ### :boom: Breaking Change @@ -425,10 +423,6 @@ fix(instrumentation): use caret range on import-in-the-middle [#4380](https://gi * fix(histogram): fix maximum when only values < -1 are provided [#3086](https://github.com/open-telemetry/opentelemetry-js/pull/3086) @pichlermarc * fix(instrumentation-grpc): always set grpc semcov status code attribute with numeric value [#3076](https://github.com/open-telemetry/opentelemetry-js/pull/3076) @blumamir -### :books: (Refine Doc) - -### :house: (Internal) - ## 0.30.0 ### :boom: Breaking Change @@ -503,10 +497,6 @@ fix(instrumentation): use caret range on import-in-the-middle [#4380](https://gi * fix(metrics): specification compliant default metric unit [#2983](https://github.com/open-telemetry/opentelemetry-js/pull/2983) @andyfleming * fix(opentelemetry-instrumentation): use all provided patches for the same file [#2963](https://github.com/open-telemetry/opentelemetry-js/pull/2963) @Ugzuzg -### :books: (Refine Doc) - -### :house: (Internal) - ## 0.28.0 ### :boom: Breaking Change From 306e08dd99cbd7e5cbfb91538da76384a306e708 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 28 Dec 2023 22:10:42 -0800 Subject: [PATCH 06/21] fix(sdk-node): lint fix --- experimental/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 035df3efe8..3f0b91598c 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) ### :bug: (Bug Fix) + * fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc From 7eca9426a67fe6a9b92d2fa0bdb124f58130ca33 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Fri, 29 Dec 2023 14:27:18 -0800 Subject: [PATCH 07/21] fix(sdk-node): Fix logic for creating tracerProviders. --- experimental/packages/opentelemetry-sdk-node/src/sdk.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index f490851057..1fe4dab3fd 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -101,6 +101,7 @@ export class NodeSDK { public constructor(configuration: Partial = {}) { const env = getEnv(); const envWithoutDefaults = getEnvWithoutDefaults(); + const envExporterWithConfiguration = env.OTEL_TRACES_EXPORTER && (configuration.sampler || configuration.spanLimits || configuration.idGenerator); if (env.OTEL_SDK_DISABLED) { this._disabled = true; @@ -126,11 +127,8 @@ export class NodeSDK { this._autoDetectResources = configuration.autoDetectResources ?? true; - if ( - configuration.spanProcessor || - configuration.traceExporter || - (env.OTEL_TRACES_EXPORTER && env.OTEL_TRACES_EXPORTER !== 'none') - ) { + // If there are configuration options to read, we need to create a new TracerProvider even if the exporter is configured in the environment + if ((configuration.traceExporter || configuration.spanProcessor) || envExporterWithConfiguration) { const tracerProviderConfig: NodeTracerConfig = {}; if (configuration.sampler) { From 02574b36546b557d3a028cec7798971be2dba0be Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Wed, 3 Jan 2024 09:56:18 -0800 Subject: [PATCH 08/21] Fix lint. --- .../packages/opentelemetry-sdk-node/src/sdk.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 1fe4dab3fd..3b02933a37 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -101,7 +101,11 @@ export class NodeSDK { public constructor(configuration: Partial = {}) { const env = getEnv(); const envWithoutDefaults = getEnvWithoutDefaults(); - const envExporterWithConfiguration = env.OTEL_TRACES_EXPORTER && (configuration.sampler || configuration.spanLimits || configuration.idGenerator); + const envExporterWithConfiguration = + env.OTEL_TRACES_EXPORTER && + (configuration.sampler || + configuration.spanLimits || + configuration.idGenerator); if (env.OTEL_SDK_DISABLED) { this._disabled = true; @@ -128,7 +132,11 @@ export class NodeSDK { this._autoDetectResources = configuration.autoDetectResources ?? true; // If there are configuration options to read, we need to create a new TracerProvider even if the exporter is configured in the environment - if ((configuration.traceExporter || configuration.spanProcessor) || envExporterWithConfiguration) { + if ( + configuration.traceExporter || + configuration.spanProcessor || + envExporterWithConfiguration + ) { const tracerProviderConfig: NodeTracerConfig = {}; if (configuration.sampler) { From 630211199aa52148315cce86a9e5af45894b5360 Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Wed, 10 Jan 2024 10:56:45 -0800 Subject: [PATCH 09/21] Update experimental/CHANGELOG.md Co-authored-by: Marc Pichler --- experimental/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 0254c89f8f..d4ccbd261d 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -16,7 +16,6 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) * fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber -fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc * fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc * fix(instrumentation): do not import 'path' in browser runtimes [#4378](https://github.com/open-telemetry/opentelemetry-js/pull/4378) @pichlermarc * Fixes a bug where bundling for web would fail due to `InstrumentationNodeModuleDefinition` importing `path` From a7bc759e64cd9ea2643124967aee3c048fe83de0 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Mon, 15 Jan 2024 15:56:59 -0800 Subject: [PATCH 10/21] Fix manual sampler and environment exporter case. --- .../packages/opentelemetry-sdk-node/src/sdk.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 3b02933a37..a26dfed1ad 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -305,6 +305,7 @@ export class NodeSDK { * Call this method to construct SDK components and register them with the OpenTelemetry API. */ public start(): void { + const env = getEnv(); if (this._disabled) { return; } @@ -326,9 +327,14 @@ export class NodeSDK { }) ); - const Provider = this._tracerProviderConfig - ? NodeTracerProvider - : TracerProviderWithEnvExporters; + // if there is a defined tracerProviderConfig, the traces exporter is defined and not none and there is no traceExporter defined in manual config + const Provider = + this._tracerProviderConfig && + !env.OTEL_TRACES_EXPORTER && + env.OTEL_TRACES_EXPORTER !== 'none' && + !this._tracerProviderConfig.spanProcessor + ? NodeTracerProvider + : TracerProviderWithEnvExporters; const tracerProvider = new Provider({ ...this._tracerProviderConfig?.tracerConfig, From f62d05f414f27fedc745879eb9d6e7ca135cc70a Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Mon, 15 Jan 2024 16:52:19 -0800 Subject: [PATCH 11/21] Update logic to check for a defined traceExporter on the config before using the NodeTracerProvider. --- .../packages/opentelemetry-sdk-node/src/sdk.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index a26dfed1ad..bec0be488e 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -40,6 +40,7 @@ import { LogRecordProcessor, LoggerProvider } from '@opentelemetry/sdk-logs'; import { MeterProvider, MetricReader, View } from '@opentelemetry/sdk-metrics'; import { BatchSpanProcessor, + SpanExporter, SpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { @@ -92,6 +93,7 @@ export class NodeSDK { private _loggerProvider?: LoggerProvider; private _meterProvider?: MeterProvider; private _serviceName?: string; + private _traceExporter?: SpanExporter; private _disabled?: boolean; @@ -121,6 +123,7 @@ export class NodeSDK { }); } + this._traceExporter = configuration.traceExporter; this._resource = configuration.resource ?? new Resource({}); this._resourceDetectors = configuration.resourceDetectors ?? [ envDetector, @@ -329,10 +332,9 @@ export class NodeSDK { // if there is a defined tracerProviderConfig, the traces exporter is defined and not none and there is no traceExporter defined in manual config const Provider = - this._tracerProviderConfig && - !env.OTEL_TRACES_EXPORTER && - env.OTEL_TRACES_EXPORTER !== 'none' && - !this._tracerProviderConfig.spanProcessor + (this._tracerProviderConfig && + (!env.OTEL_TRACES_EXPORTER || env.OTEL_TRACES_EXPORTER == 'none')) || + this._traceExporter ? NodeTracerProvider : TracerProviderWithEnvExporters; From 996172fcff0f776188a7b4c187d2d05d36fc4ac3 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Mon, 15 Jan 2024 16:58:30 -0800 Subject: [PATCH 12/21] Fix equality check. --- experimental/packages/opentelemetry-sdk-node/src/sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index bec0be488e..534ded6303 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -333,7 +333,7 @@ export class NodeSDK { // if there is a defined tracerProviderConfig, the traces exporter is defined and not none and there is no traceExporter defined in manual config const Provider = (this._tracerProviderConfig && - (!env.OTEL_TRACES_EXPORTER || env.OTEL_TRACES_EXPORTER == 'none')) || + (!env.OTEL_TRACES_EXPORTER || env.OTEL_TRACES_EXPORTER === 'none')) || this._traceExporter ? NodeTracerProvider : TracerProviderWithEnvExporters; From 685dd6c8cf74a00eda19cadf5c17d097a7ed9792 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Tue, 16 Jan 2024 10:00:09 -0800 Subject: [PATCH 13/21] Update env exporter configuration logic and add tests. --- .../opentelemetry-sdk-node/src/sdk.ts | 54 +++++++++---------- .../opentelemetry-sdk-node/test/sdk.test.ts | 18 +++++++ 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 534ded6303..218d8bb8ca 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -40,7 +40,6 @@ import { LogRecordProcessor, LoggerProvider } from '@opentelemetry/sdk-logs'; import { MeterProvider, MetricReader, View } from '@opentelemetry/sdk-metrics'; import { BatchSpanProcessor, - SpanExporter, SpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { @@ -93,7 +92,7 @@ export class NodeSDK { private _loggerProvider?: LoggerProvider; private _meterProvider?: MeterProvider; private _serviceName?: string; - private _traceExporter?: SpanExporter; + private _configuration?: Partial; private _disabled?: boolean; @@ -103,11 +102,6 @@ export class NodeSDK { public constructor(configuration: Partial = {}) { const env = getEnv(); const envWithoutDefaults = getEnvWithoutDefaults(); - const envExporterWithConfiguration = - env.OTEL_TRACES_EXPORTER && - (configuration.sampler || - configuration.spanLimits || - configuration.idGenerator); if (env.OTEL_SDK_DISABLED) { this._disabled = true; @@ -123,7 +117,8 @@ export class NodeSDK { }); } - this._traceExporter = configuration.traceExporter; + this._configuration = configuration; + this._resource = configuration.resource ?? new Resource({}); this._resourceDetectors = configuration.resourceDetectors ?? [ envDetector, @@ -134,12 +129,8 @@ export class NodeSDK { this._autoDetectResources = configuration.autoDetectResources ?? true; - // If there are configuration options to read, we need to create a new TracerProvider even if the exporter is configured in the environment - if ( - configuration.traceExporter || - configuration.spanProcessor || - envExporterWithConfiguration - ) { + // If a tracer provider can be created from manual configuration, create it + if (configuration.traceExporter || configuration.spanProcessor) { const tracerProviderConfig: NodeTracerConfig = {}; if (configuration.sampler) { @@ -308,7 +299,6 @@ export class NodeSDK { * Call this method to construct SDK components and register them with the OpenTelemetry API. */ public start(): void { - const env = getEnv(); if (this._disabled) { return; } @@ -330,18 +320,28 @@ export class NodeSDK { }) ); - // if there is a defined tracerProviderConfig, the traces exporter is defined and not none and there is no traceExporter defined in manual config - const Provider = - (this._tracerProviderConfig && - (!env.OTEL_TRACES_EXPORTER || env.OTEL_TRACES_EXPORTER === 'none')) || - this._traceExporter - ? NodeTracerProvider - : TracerProviderWithEnvExporters; - - const tracerProvider = new Provider({ - ...this._tracerProviderConfig?.tracerConfig, - resource: this._resource, - }); + // if there is a tracerProviderConfig (traceExporter/spanProcessor was set manually) or the traceExporter is set manually, use NodeTracerProvider + const Provider = this._tracerProviderConfig + ? NodeTracerProvider + : TracerProviderWithEnvExporters; + + let tracerProvider; + + // If the Provider is configured with Env Exporters, we need to check if the SDK had any manual configurations, else configure normally + if ( + typeof Provider === typeof TracerProviderWithEnvExporters && + this._configuration + ) { + tracerProvider = new Provider({ + ...this._configuration, + resource: this._resource, + }); + } else { + tracerProvider = new Provider({ + ...this._tracerProviderConfig?.tracerConfig, + resource: this._resource, + }); + } this._tracerProvider = tracerProvider; diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 1df9f5c22d..9424617bbb 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -46,6 +46,7 @@ import { BatchSpanProcessor, NoopSpanProcessor, IdGenerator, + AlwaysOffSampler, } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import * as semver from 'semver'; @@ -896,6 +897,23 @@ describe('setup exporter from env', () => { assert(listOfProcessors[0] instanceof BatchSpanProcessor); delete env.OTEL_TRACES_EXPORTER; }); + it('should only create one span processor when configured using env vars and config', async () => { + env.OTEL_TRACES_EXPORTER = 'console'; + const sdk = new NodeSDK({ + sampler: new AlwaysOffSampler(), + }); + sdk.start(); + const listOfProcessors = + sdk['_tracerProvider']!['_registeredSpanProcessors']!; + assert( + sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters === true + ); + assert( + sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler + ) + assert(listOfProcessors.length === 1); + delete env.OTEL_TRACES_EXPORTER; + }); it('use otlp exporter and defined exporter protocol env value', async () => { env.OTEL_TRACES_EXPORTER = 'otlp'; env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc'; From 2c3e88a493c1dd96391c66ede07420f91bbac11d Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Tue, 16 Jan 2024 10:03:57 -0800 Subject: [PATCH 14/21] Update experimental/CHANGELOG.md Co-authored-by: Marc Pichler --- experimental/CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index d44c25447f..dde0044b83 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -11,10 +11,6 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) * fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber -* fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc -* fix(instrumentation): do not import 'path' in browser runtimes [#4378](https://github.com/open-telemetry/opentelemetry-js/pull/4378) @pichlermarc - * Fixes a bug where bundling for web would fail due to `InstrumentationNodeModuleDefinition` importing `path` - ### :books: (Refine Doc) ### :house: (Internal) From 6c1f2263568e1379d563976704e32a81414a218d Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Tue, 16 Jan 2024 10:06:30 -0800 Subject: [PATCH 15/21] Fix changelog issues. --- experimental/CHANGELOG.md | 3 --- experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index d44c25447f..820dd18b58 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -11,9 +11,6 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) * fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber -* fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc -* fix(instrumentation): do not import 'path' in browser runtimes [#4378](https://github.com/open-telemetry/opentelemetry-js/pull/4378) @pichlermarc - * Fixes a bug where bundling for web would fail due to `InstrumentationNodeModuleDefinition` importing `path` ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 9424617bbb..416ac9825a 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -910,7 +910,7 @@ describe('setup exporter from env', () => { ); assert( sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler - ) + ); assert(listOfProcessors.length === 1); delete env.OTEL_TRACES_EXPORTER; }); From ad5ec3092faf523f7c914daaf49de05119add7b6 Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Tue, 16 Jan 2024 14:47:06 -0800 Subject: [PATCH 16/21] Clean up tracerProvider logic. --- .../opentelemetry-sdk-node/src/sdk.ts | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 218d8bb8ca..c44521163f 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -325,23 +325,11 @@ export class NodeSDK { ? NodeTracerProvider : TracerProviderWithEnvExporters; - let tracerProvider; - - // If the Provider is configured with Env Exporters, we need to check if the SDK had any manual configurations, else configure normally - if ( - typeof Provider === typeof TracerProviderWithEnvExporters && - this._configuration - ) { - tracerProvider = new Provider({ - ...this._configuration, - resource: this._resource, - }); - } else { - tracerProvider = new Provider({ - ...this._tracerProviderConfig?.tracerConfig, - resource: this._resource, - }); - } + // If the Provider is configured with Env Exporters, we need to check if the SDK had any manual configurations and set them here + const tracerProvider = new Provider({ + ...this._configuration, + resource: this._resource, + }); this._tracerProvider = tracerProvider; From 88aa3a0fea18fbf4f0d02ed3b08515ffe29541bc Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 18 Jan 2024 11:48:26 -0800 Subject: [PATCH 17/21] Update sdk.ts --- experimental/packages/opentelemetry-sdk-node/src/sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index c44521163f..21769d151f 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -331,7 +331,7 @@ export class NodeSDK { resource: this._resource, }); - this._tracerProvider = tracerProvider; + // this._tracerProvider = tracerProvider; if (this._tracerProviderConfig) { tracerProvider.addSpanProcessor(this._tracerProviderConfig.spanProcessor); From 1d01797c1f2b57863a372eed020878c3a6b0b03e Mon Sep 17 00:00:00 2001 From: Jackson Weber Date: Thu, 18 Jan 2024 11:48:34 -0800 Subject: [PATCH 18/21] Update sdk.ts --- experimental/packages/opentelemetry-sdk-node/src/sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts index 21769d151f..c44521163f 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/sdk.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/sdk.ts @@ -331,7 +331,7 @@ export class NodeSDK { resource: this._resource, }); - // this._tracerProvider = tracerProvider; + this._tracerProvider = tracerProvider; if (this._tracerProviderConfig) { tracerProvider.addSpanProcessor(this._tracerProviderConfig.spanProcessor); From 35b889ebd0f51cc4aa41e056a49680b8d676be61 Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Tue, 30 Jan 2024 10:25:44 -0800 Subject: [PATCH 19/21] Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Co-authored-by: Marc Pichler --- experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 416ac9825a..c1cbae520c 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -905,9 +905,7 @@ describe('setup exporter from env', () => { sdk.start(); const listOfProcessors = sdk['_tracerProvider']!['_registeredSpanProcessors']!; - assert( - sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters === true - ); + assert.ok(sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters); assert( sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler ); From 5c65cc036e3269e55f28e9df25a7ec078d9c53bc Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Tue, 30 Jan 2024 10:25:55 -0800 Subject: [PATCH 20/21] Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Co-authored-by: Marc Pichler --- experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index c1cbae520c..73a52c0f3c 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -906,7 +906,7 @@ describe('setup exporter from env', () => { const listOfProcessors = sdk['_tracerProvider']!['_registeredSpanProcessors']!; assert.ok(sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters); - assert( + assert.ok( sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler ); assert(listOfProcessors.length === 1); From a639e1cdeaa9b1135e64bfd8c8affea219dc5ed9 Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Tue, 30 Jan 2024 10:26:01 -0800 Subject: [PATCH 21/21] Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts Co-authored-by: Marc Pichler --- experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index 73a52c0f3c..d6a93c8a60 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -909,7 +909,7 @@ describe('setup exporter from env', () => { assert.ok( sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler ); - assert(listOfProcessors.length === 1); + assert.strictEqual(listOfProcessors.length, 1); delete env.OTEL_TRACES_EXPORTER; }); it('use otlp exporter and defined exporter protocol env value', async () => {