Skip to content
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

feat(sdk-trace-base)!: drop ability to auto-instantiate propagators beyond defaults #5355

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,19 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* (user-facing): deprecated `AlwaysOffSampler` has moved to `@opentelemetry/sdk-trace-base`
* (user-facing): deprecated `TraceIdRatioSampler` has moved to `@opentelemetry/sdk-trace-base`
* (user-facing): deprecated `TraceIdRatioSampler` has moved to `@opentelemetry/sdk-trace-base`
* feat(sdk-trace-base)!: drop ability to instantiate propagators beyond defaults [#5355](https://github.com/open-telemetry/opentelemetry-js/pull/5355) @pichlermarc
* (user-facing): only a non-env-var based default is now used on `BasicTracerProvider#register()`.
* propagators can now not be configured via `OTEL_PROPAGATORS` or `window.OTEL_PROPAGATORS` anymore, please pass the propagator to `NodeTracerProvider#register()` instead.
* if not configured directly via code, `BasicTracerProvider#register()` will now fall back to defaults (`tracecontext` and `baggage`)
* feat(sdk-trace-node)!: drop ability to instantiate propagators beyond defaults [#5355](https://github.com/open-telemetry/opentelemetry-js/pull/5355) @pichlermarc
* (user-facing): only a non-env-var based default is now used on `NodeTracerProvider#register()`.
* propagators can now not be configured via `OTEL_PROPAGATORS` anymore, please pass the propagator to `NodeTracerProvider#register()` instead.
* if not configured via code, `NodeTracerProvider#register()` will now fall back to the defaults (`tracecontext` and `baggage`)
* if autoconfiguration based on enviornment variables is needed, please use `NodeSDK` from `@opentelemetry/sdk-node`.
* feat(sdk-trace-web)!: drop ability to instantiate propagators beyond defaults [#5355](https://github.com/open-telemetry/opentelemetry-js/pull/5355) @pichlermarc
* (user-facing): only a non-env-var based default is now used on `WebTracerProvider#register()`.
* propagators can now not be configured via `window.OTEL_PROPAGATORS` anymore, please pass the propagator to `WebTracerProvider#register()` instead.
* if not configured via code, `WebTracerProvider#register()` will now fall back to defaults (`tracecontext` and `baggage`)

### :rocket: (Enhancement)

Expand Down
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ All notable changes to experimental packages in this project will be documented
* chore(instrumentation-grpc): remove unused findIndex() function [#5372](https://github.com/open-telemetry/opentelemetry-js/pull/5372) @cjihrig
* refactor(otlp-exporter-base): remove unnecessary isNaN() checks [#5374](https://github.com/open-telemetry/opentelemetry-js/pull/5374) @cjihrig
* refactor(exporter-prometheus): remove unnecessary isNaN() check [#5377](https://github.com/open-telemetry/opentelemetry-js/pull/5377) @cjihrig
* refactor(sdk-node): move code to auto-instantiate propagators into utils [#5355](https://github.com/open-telemetry/opentelemetry-js/pull/5355) @pichlermarc

## 0.57.0

Expand Down
4 changes: 3 additions & 1 deletion experimental/packages/opentelemetry-sdk-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@
"@opentelemetry/sdk-metrics": "1.30.0",
"@opentelemetry/sdk-trace-base": "1.30.0",
"@opentelemetry/sdk-trace-node": "1.30.0",
"@opentelemetry/semantic-conventions": "1.28.0"
"@opentelemetry/semantic-conventions": "1.28.0",
"@opentelemetry/propagator-jaeger": "1.30.0",
"@opentelemetry/propagator-b3": "1.30.0"
},
"peerDependencies": {
"@opentelemetry/api": ">=1.3.0 <1.10.0"
Expand Down
5 changes: 4 additions & 1 deletion experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import {
getResourceDetectorsFromEnv,
getSpanProcessorsFromEnv,
filterBlanksAndNulls,
getPropagatorFromEnv,
} from './utils';

/** This class represents everything needed to register a fully configured OpenTelemetry Node.js SDK */
Expand Down Expand Up @@ -376,7 +377,9 @@ export class NodeSDK {
this._tracerProviderConfig?.contextManager ??
// _tracerProviderConfig may be undefined if trace-specific settings are not provided - fall back to raw config
this._configuration?.contextManager,
propagator: this._tracerProviderConfig?.textMapPropagator,
propagator:
this._tracerProviderConfig?.textMapPropagator ??
getPropagatorFromEnv(),
});
}

Expand Down
74 changes: 72 additions & 2 deletions experimental/packages/opentelemetry-sdk-node/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@
* limitations under the License.
*/

import { diag } from '@opentelemetry/api';
import { getEnv, getEnvWithoutDefaults } from '@opentelemetry/core';
import { diag, TextMapPropagator } from '@opentelemetry/api';
import {
CompositePropagator,
getEnv,
getEnvWithoutDefaults,
W3CTraceContextPropagator,
} from '@opentelemetry/core';
import { OTLPTraceExporter as OTLPProtoTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto';
import { OTLPTraceExporter as OTLPHttpTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
import { OTLPTraceExporter as OTLPGrpcTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc';
Expand All @@ -35,6 +40,8 @@ import {
SpanExporter,
SpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import { B3InjectEncoding, B3Propagator } from '@opentelemetry/propagator-b3';
import { JaegerPropagator } from '@opentelemetry/propagator-jaeger';

const RESOURCE_DETECTOR_ENVIRONMENT = 'env';
const RESOURCE_DETECTOR_HOST = 'host';
Expand Down Expand Up @@ -180,3 +187,66 @@ export function getSpanProcessorsFromEnv(): SpanProcessor[] {

return processors;
}

/**
* Get a propagator as defined by environment variables
*/
export function getPropagatorFromEnv(): TextMapPropagator | null | undefined {
// Empty and undefined MUST be treated equal.
if (
process.env.OTEL_PROPAGATORS === undefined ||
process.env.OTEL_PROPAGATORS?.trim() === ''
) {
// return undefined to fall back to default
return undefined;
}

// Implementation note: this only contains specification required propagators that are actually hosted in this repo.
// Any other propagators (like aws, aws-lambda, should go into `@opentelemetry/auto-configuration-propagators` instead).
const propagatorsFactory = new Map<string, () => TextMapPropagator>([
['tracecontext', () => new W3CTraceContextPropagator()],
['baggage', () => new W3CTraceContextPropagator()],
['b3', () => new B3Propagator()],
[
'b3multi',
() => new B3Propagator({ injectEncoding: B3InjectEncoding.MULTI_HEADER }),
],
['jaeger', () => new JaegerPropagator()],
]);

// Values MUST be deduplicated in order to register a Propagator only once.
const uniquePropagatorNames = Array.from(new Set(getEnv().OTEL_PROPAGATORS));

const propagators = uniquePropagatorNames.map(name => {
const propagator = propagatorsFactory.get(name)?.();
if (!propagator) {
diag.warn(
`Propagator "${name}" requested through environment variable is unavailable.`
);
return undefined;
}

return propagator;
});

const validPropagators = propagators.reduce<TextMapPropagator[]>(
(list, item) => {
if (item) {
list.push(item);
}
return list;
},
[]
);

if (validPropagators.length === 0) {
// null to signal that the default should **not** be used in its place.
return null;
} else if (uniquePropagatorNames.length === 1) {
return validPropagators[0];
} else {
return new CompositePropagator({
propagators: validPropagators,
});
}
}
15 changes: 15 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,21 @@ describe('Node SDK', () => {
assert.equal(actualContextManager, expectedContextManager);
await sdk.shutdown();
});

it('should register propagators as defined in OTEL_PROPAGATORS if trace SDK is configured', async () => {
process.env.OTEL_PROPAGATORS = 'b3';
const sdk = new NodeSDK({
traceExporter: new ConsoleSpanExporter(),
autoDetectResources: false,
});

sdk.start();

assert.deepStrictEqual(propagation.fields(), ['b3']);

await sdk.shutdown();
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
delete process.env.OTEL_PROPAGATORS;
});
});

async function waitForNumberOfMetrics(
Expand Down
102 changes: 102 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/test/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { getPropagatorFromEnv } from '../src/utils';
import * as assert from 'assert';
import * as sinon from 'sinon';
import { diag } from '@opentelemetry/api';

describe('getPropagatorFromEnv', function () {
afterEach(() => {
delete process.env.OTEL_PROPAGATORS;
sinon.restore();
});

describe('should default to undefined', function () {
it('when not defined', function () {
delete process.env.OTEL_PROPAGATORS;

const propagator = getPropagatorFromEnv();

assert.deepStrictEqual(propagator, undefined);
});

it('on empty string', function () {
(process.env as any).OTEL_PROPAGATORS = '';

const propagator = getPropagatorFromEnv();

assert.deepStrictEqual(propagator, undefined);
});

it('on space-only string', function () {
(process.env as any).OTEL_PROPAGATORS = ' ';

const propagator = getPropagatorFromEnv();

assert.deepStrictEqual(propagator, undefined);
});
});

it('should return the selected propagator when one is in the list', () => {
process.env.OTEL_PROPAGATORS = 'tracecontext';
assert.deepStrictEqual(getPropagatorFromEnv()?.fields(), [
'traceparent',
'tracestate',
]);
});

it('should return the selected propagators when multiple are in the list', () => {
process.env.OTEL_PROPAGATORS = 'tracecontext,baggage,b3,b3multi,jaeger';
assert.deepStrictEqual(getPropagatorFromEnv()?.fields(), [
'traceparent',
'tracestate',
'b3',
'x-b3-traceid',
'x-b3-spanid',
'x-b3-flags',
'x-b3-sampled',
'x-b3-parentspanid',
'uber-trace-id',
]);
});

it('should return null and warn if propagators are unknown', () => {
const warnStub = sinon.stub(diag, 'warn');

process.env.OTEL_PROPAGATORS = 'my, unknown, propagators';
assert.deepStrictEqual(getPropagatorFromEnv(), null);
sinon.assert.calledWithExactly(
warnStub,
'Propagator "my" requested through environment variable is unavailable.'
);
sinon.assert.calledWithExactly(
warnStub,
'Propagator "unknown" requested through environment variable is unavailable.'
);
sinon.assert.calledWithExactly(
warnStub,
'Propagator "propagators" requested through environment variable is unavailable.'
);
sinon.assert.calledThrice(warnStub);
});

it('should return null if only "none" is selected', () => {
process.env.OTEL_PROPAGATORS = 'none';

assert.deepStrictEqual(getPropagatorFromEnv(), null);
});
});
6 changes: 6 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
{
"path": "../../../packages/opentelemetry-exporter-zipkin"
},
{
"path": "../../../packages/opentelemetry-propagator-b3"
},
{
"path": "../../../packages/opentelemetry-propagator-jaeger"
},
{
"path": "../../../packages/opentelemetry-resources"
},
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading