diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index dbdd0d1786..074f554f01 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -23,6 +23,7 @@ All notable changes to experimental packages in this project will be documented * fix(prometheus-sanitization): replace repeated `_` with a single `_` [3470](https://github.com/open-telemetry/opentelemetry-js/pull/3470) @samimusallam * fix(prometheus-serializer): correct string used for NaN [#3477](https://github.com/open-telemetry/opentelemetry-js/pull/3477) @JacksonWeber * fix(instrumentation-http): close server span when response finishes [#3407](https://github.com/open-telemetry/opentelemetry-js/pull/3407) @legendecas +* fix(sdk-node): fix exporter to be read only OTEL_TRACES_EXPORTER is set to a valid exporter [3492] @svetlanabrennan ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-sdk-node/src/TracerProviderWithEnvExporter.ts b/experimental/packages/opentelemetry-sdk-node/src/TracerProviderWithEnvExporter.ts index 20082a9b6e..add81c7c91 100644 --- a/experimental/packages/opentelemetry-sdk-node/src/TracerProviderWithEnvExporter.ts +++ b/experimental/packages/opentelemetry-sdk-node/src/TracerProviderWithEnvExporter.ts @@ -84,10 +84,22 @@ export class TracerProviderWithEnvExporters extends NodeTracerProvider { Array.from(new Set(getEnv().OTEL_TRACES_EXPORTER.split(','))) ); - if (traceExportersList.length === 0 || traceExportersList[0] === 'none') { + if (traceExportersList[0] === 'none') { diag.warn( - 'OTEL_TRACES_EXPORTER contains "none" or is empty. SDK will not be initialized.' + 'OTEL_TRACES_EXPORTER contains "none". SDK will not be initialized.' ); + } else if (traceExportersList.length === 0) { + diag.warn('OTEL_TRACES_EXPORTER is empty. Using default otlp exporter.'); + + traceExportersList = ['otlp']; + this.createExportersFromList(traceExportersList); + + this._spanProcessors = this.configureSpanProcessors( + this._configuredExporters + ); + this._spanProcessors.forEach(processor => { + this.addSpanProcessor(processor); + }); } else { if ( traceExportersList.length > 1 && @@ -99,16 +111,7 @@ export class TracerProviderWithEnvExporters extends NodeTracerProvider { traceExportersList = ['otlp']; } - traceExportersList.forEach(exporterName => { - const exporter = this._getSpanExporter(exporterName); - if (exporter) { - this._configuredExporters.push(exporter); - } else { - diag.warn( - `Unrecognized OTEL_TRACES_EXPORTER value: ${exporterName}.` - ); - } - }); + this.createExportersFromList(traceExportersList); if (this._configuredExporters.length > 0) { this._spanProcessors = this.configureSpanProcessors( @@ -136,6 +139,17 @@ export class TracerProviderWithEnvExporters extends NodeTracerProvider { } } + private createExportersFromList(exporterList: string[]) { + exporterList.forEach(exporterName => { + const exporter = this._getSpanExporter(exporterName); + if (exporter) { + this._configuredExporters.push(exporter); + } else { + diag.warn(`Unrecognized OTEL_TRACES_EXPORTER value: ${exporterName}.`); + } + }); + } + private configureSpanProcessors( exporters: SpanExporter[] ): (BatchSpanProcessor | SimpleSpanProcessor)[] { diff --git a/experimental/packages/opentelemetry-sdk-node/test/TracerProviderWithEnvExporter.test.ts b/experimental/packages/opentelemetry-sdk-node/test/TracerProviderWithEnvExporter.test.ts index c99d378617..7ceb5e3dce 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/TracerProviderWithEnvExporter.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/TracerProviderWithEnvExporter.test.ts @@ -96,15 +96,29 @@ describe('set up trace exporter with env exporters', () => { delete env.OTEL_EXPORTER_OTLP_PROTOCOL; delete env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL; }); - it('do not use any exporters when empty value is provided for exporter', async () => { + it('use default otlp exporter when user does not set exporter via env or config', async () => { + const sdk = new TracerProviderWithEnvExporters(); + const listOfProcessors = sdk['_spanProcessors']!; + const listOfExporters = sdk['_configuredExporters']; + + assert(listOfExporters[0] instanceof OTLPProtoTraceExporter); + assert(listOfExporters.length === 1); + + assert(listOfProcessors.length === 1); + assert(listOfProcessors[0] instanceof BatchSpanProcessor); + }); + it('use default otlp exporter when empty value is provided for exporter via env', async () => { env.OTEL_TRACES_EXPORTER = ''; const sdk = new TracerProviderWithEnvExporters(); - const listOfProcessors = sdk['_spanProcessors']; + const listOfProcessors = sdk['_spanProcessors']!; const listOfExporters = sdk['_configuredExporters']; - assert(spyGetOtlpProtocol.notCalled); - assert(listOfExporters.length === 0); - assert(listOfProcessors === undefined); + assert(listOfExporters[0] instanceof OTLPProtoTraceExporter); + assert(listOfExporters.length === 1); + + assert(listOfProcessors.length === 1); + assert(listOfProcessors[0] instanceof BatchSpanProcessor); + env.OTEL_TRACES_EXPORTER = ''; }); it('do not use any exporters when none value is only provided', async () => { @@ -124,7 +138,7 @@ describe('set up trace exporter with env exporters', () => { assert.strictEqual( stubLoggerError.args[0][0], - 'OTEL_TRACES_EXPORTER contains "none" or is empty. SDK will not be initialized.' + 'OTEL_TRACES_EXPORTER contains "none". SDK will not be initialized.' ); delete env.OTEL_TRACES_EXPORTER; }); @@ -174,6 +188,11 @@ describe('set up trace exporter with env exporters', () => { assert.strictEqual( stubLoggerError.args[0][0], + 'OTEL_TRACES_EXPORTER is empty. Using default otlp exporter.' + ); + + assert.strictEqual( + stubLoggerError.args[1][0], 'Unsupported OTLP traces protocol: invalid. Using http/protobuf.' ); delete env.OTEL_EXPORTER_OTLP_PROTOCOL; diff --git a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts index bad3d7f4b7..2f7957a8ec 100644 --- a/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts +++ b/experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts @@ -769,21 +769,30 @@ describe('setup exporter from env', () => { assert.strictEqual( stubLoggerError.args[0][0], - 'OTEL_TRACES_EXPORTER contains "none" or is empty. SDK will not be initialized.' + 'OTEL_TRACES_EXPORTER contains "none". SDK will not be initialized.' ); delete env.OTEL_TRACES_EXPORTER; }); - it('do not use any exporters when empty value is provided for exporter', async () => { - env.OTEL_TRACES_EXPORTER = ''; + it('use default otlp exporter when user does not set exporter via env or config', async () => { const sdk = new NodeSDK(); await sdk.start(); const listOfProcessors = sdk['_tracerProvider']!['_registeredSpanProcessors']!; - const activeProcessor = sdk['_tracerProvider']?.getActiveSpanProcessor(); + assert(sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters); + assert(listOfProcessors.length === 1); + assert(listOfProcessors[0] instanceof BatchSpanProcessor); + }); + it('use default otlp exporter when empty value is provided for exporter via env', async () => { + env.OTEL_TRACES_EXPORTER = ''; + const sdk = new NodeSDK(); + await sdk.start(); - assert(listOfProcessors.length === 0); - assert(activeProcessor instanceof NoopSpanProcessor); + const listOfProcessors = + sdk['_tracerProvider']!['_registeredSpanProcessors']!; + assert(sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters); + assert(listOfProcessors.length === 1); + assert(listOfProcessors[0] instanceof BatchSpanProcessor); env.OTEL_TRACES_EXPORTER = ''; }); diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 36cfe0f702..01da17f293 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -172,7 +172,7 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: DEFAULT_ATTRIBUTE_COUNT_LIMIT, OTEL_SPAN_EVENT_COUNT_LIMIT: 128, OTEL_SPAN_LINK_COUNT_LIMIT: 128, - OTEL_TRACES_EXPORTER: 'otlp', + OTEL_TRACES_EXPORTER: '', OTEL_TRACES_SAMPLER: TracesSamplerValues.ParentBasedAlwaysOn, OTEL_TRACES_SAMPLER_ARG: '', OTEL_EXPORTER_OTLP_INSECURE: '', diff --git a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts index 5a0889271b..179b6b1d3e 100644 --- a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts +++ b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts @@ -275,7 +275,7 @@ export class BasicTracerProvider implements TracerProvider { protected _buildExporterFromEnv(): SpanExporter | undefined { const exporterName = getEnv().OTEL_TRACES_EXPORTER; - if (exporterName === 'none') return; + if (exporterName === 'none' || exporterName === '') return; const exporter = this._getSpanExporter(exporterName); if (!exporter) { diag.error( diff --git a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts index 8dd49a9c14..d7a900cf9f 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts @@ -90,6 +90,29 @@ describe('BasicTracerProvider', () => { const tracer = new BasicTracerProvider(); assert.ok(tracer.activeSpanProcessor instanceof NoopSpanProcessor); }); + it('should use noop span processor by default and no diag error', () => { + const errorStub = sinon.spy(diag, 'error'); + const tracer = new BasicTracerProvider(); + assert.ok(tracer.activeSpanProcessor instanceof NoopSpanProcessor); + + sinon.assert.notCalled(errorStub); + }); + }); + + describe('when user sets unavailable exporter', () => { + it('should use noop span processor by default and show diag error', () => { + const errorStub = sinon.spy(diag, 'error'); + envSource.OTEL_TRACES_EXPORTER = 'someExporter'; + + const tracer = new BasicTracerProvider(); + assert.ok(tracer.activeSpanProcessor instanceof NoopSpanProcessor); + + sinon.assert.calledWith( + errorStub, + 'Exporter "someExporter" requested through environment variable is unavailable.' + ); + delete envSource.OTEL_TRACES_EXPORTER; + }); }); describe('when "sampler" option defined', () => {