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(auto-instrumentations-node): add getPropagator to get propagators from environment #2232

Closed
Show file tree
Hide file tree
Changes from 3 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
7 changes: 6 additions & 1 deletion metapackages/auto-instrumentations-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,12 @@
"@opentelemetry/resource-detector-container": "^0.3.9",
"@opentelemetry/resource-detector-gcp": "^0.29.9",
"@opentelemetry/resources": "^1.24.0",
"@opentelemetry/sdk-node": "^0.51.0"
"@opentelemetry/sdk-node": "^0.51.0",
"@opentelemetry/core": "^1.24.1",
"@opentelemetry/propagator-b3": "^1.24.1",
"@opentelemetry/propagator-jaeger": "^1.24.1",
"@opentelemetry/propagator-aws-xray": "^1.24.1",
"@opentelemetry/propagator-ot-trace": "^0.27.2"
},
"files": [
"build/src/**/*.js",
Expand Down
1 change: 1 addition & 0 deletions metapackages/auto-instrumentations-node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
export {
getNodeAutoInstrumentations,
getResourceDetectorsFromEnv as getResourceDetectors,
getPropagator,
InstrumentationConfigMap,
} from './utils';
2 changes: 2 additions & 0 deletions metapackages/auto-instrumentations-node/src/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as opentelemetry from '@opentelemetry/sdk-node';
import { diag, DiagConsoleLogger } from '@opentelemetry/api';
import {
getNodeAutoInstrumentations,
getPropagator,
getResourceDetectorsFromEnv,
} from './utils';

Expand All @@ -28,6 +29,7 @@ diag.setLogger(
const sdk = new opentelemetry.NodeSDK({
instrumentations: getNodeAutoInstrumentations(),
resourceDetectors: getResourceDetectorsFromEnv(),
textMapPropagator: getPropagator(),
});

try {
Expand Down
64 changes: 63 additions & 1 deletion metapackages/auto-instrumentations-node/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { diag } from '@opentelemetry/api';
import { diag, TextMapPropagator } from '@opentelemetry/api';
import { Instrumentation } from '@opentelemetry/instrumentation';

import { AmqplibInstrumentation } from '@opentelemetry/instrumentation-amqplib';
Expand Down Expand Up @@ -80,6 +80,15 @@
azureFunctionsDetector,
azureVmDetector,
} from '@opentelemetry/resource-detector-azure';
import {
CompositePropagator,
W3CBaggagePropagator,
W3CTraceContextPropagator,
} from '@opentelemetry/core';
import { B3InjectEncoding, B3Propagator } from '@opentelemetry/propagator-b3';
import { JaegerPropagator } from '@opentelemetry/propagator-jaeger';
import { OTTracePropagator } from '@opentelemetry/propagator-ot-trace';
import { AWSXRayPropagator } from '@opentelemetry/propagator-aws-xray';

const RESOURCE_DETECTOR_CONTAINER = 'container';
const RESOURCE_DETECTOR_ENVIRONMENT = 'env';
Expand Down Expand Up @@ -253,3 +262,56 @@
return resourceDetector || [];
});
}

type PropagatorFactoryFunction = () => TextMapPropagator;

const propagatorMap = new Map<string, PropagatorFactoryFunction>([
['tracecontext', () => new W3CTraceContextPropagator()],
['baggage', () => new W3CTraceContextPropagator()],

Check warning on line 270 in metapackages/auto-instrumentations-node/src/utils.ts

View check run for this annotation

Codecov / codecov/patch

metapackages/auto-instrumentations-node/src/utils.ts#L270

Added line #L270 was not covered by tests
[
'b3',
() => new B3Propagator({ injectEncoding: B3InjectEncoding.SINGLE_HEADER }),
],
[
'b3multi',
() => new B3Propagator({ injectEncoding: B3InjectEncoding.MULTI_HEADER }),

Check warning on line 277 in metapackages/auto-instrumentations-node/src/utils.ts

View check run for this annotation

Codecov / codecov/patch

metapackages/auto-instrumentations-node/src/utils.ts#L277

Added line #L277 was not covered by tests
],
['jaeger', () => new JaegerPropagator()],
['xray', () => new AWSXRayPropagator()],
['ottrace', () => new OTTracePropagator()],

Check warning on line 281 in metapackages/auto-instrumentations-node/src/utils.ts

View check run for this annotation

Codecov / codecov/patch

metapackages/auto-instrumentations-node/src/utils.ts#L280-L281

Added lines #L280 - L281 were not covered by tests
]);

/**
* Get a propagator based on the OTEL_PROPAGATORS env var.
*/
export function getPropagator(): TextMapPropagator {
if (process.env.OTEL_PROPAGATORS == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should empty string be treated as "use the default" as well? (That's what I infer from the language in this section: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#enum-value)

Also the spec mentions handling "none" as a value to say "no propagators": https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should empty string be treated as "use the default" as well? (That's what I infer from the language in this section: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#enum-value)

Yes that's correct, good catch 🙂 I added that to the default case. cf3bb0d

Also the spec mentions handling "none" as a value to say "no propagators": https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration

Also good catch, it would've incorrectly logged that it's unkown before (the Propgator would still have been a no-op CompositePropagator with no sub-propgators, so I think that would've been spec compliant). I added an info log that lets the user know that none. Though I'm not sure what to do if there's none among other valid values so I've opted to still take the rest of the propagators in that case. My guess is that users would be least surpirsed if they set tracecontext, none and the W3CTraceContextPropagator would be the one that gets set. cf3bb0d

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comparison points on "none" handling:

Anyway, this is something that could perhaps share some common processing at some point. Doesn't have to be this PR, and perhaps would wait for work related to supporting file-config.

return new CompositePropagator({
propagators: [
new W3CTraceContextPropagator(),
new W3CBaggagePropagator(),
],
});
}

const propagatorsFromEnv = process.env.OTEL_PROPAGATORS?.split(',').map(
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
value => value.toLowerCase()
);

const propagators = propagatorsFromEnv.flatMap(propagatorName => {
const propagatorFactoryFunction = propagatorMap.get(propagatorName);
if (propagatorFactoryFunction == null) {
diag.error(

Check warning on line 304 in metapackages/auto-instrumentations-node/src/utils.ts

View check run for this annotation

Codecov / codecov/patch

metapackages/auto-instrumentations-node/src/utils.ts#L304

Added line #L304 was not covered by tests
`Invalid propagator "${propagatorName}" specified in the environment variable OTEL_PROPAGATORS`
);
return [];

Check warning on line 307 in metapackages/auto-instrumentations-node/src/utils.ts

View check run for this annotation

Codecov / codecov/patch

metapackages/auto-instrumentations-node/src/utils.ts#L307

Added line #L307 was not covered by tests
}
return propagatorFactoryFunction();
});

if (propagators.length === 1) {
return propagators[0];
}

return new CompositePropagator({ propagators });
}
29 changes: 28 additions & 1 deletion metapackages/auto-instrumentations-node/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http';
import * as assert from 'assert';
import * as sinon from 'sinon';
import { getNodeAutoInstrumentations } from '../src';
import { getResourceDetectorsFromEnv } from '../src/utils';
import { getPropagator, getResourceDetectorsFromEnv } from '../src/utils';

describe('utils', () => {
describe('getNodeAutoInstrumentations', () => {
Expand Down Expand Up @@ -161,4 +161,31 @@ describe('utils', () => {
delete process.env.OTEL_NODE_RESOURCE_DETECTORS;
});
});

describe('getPropagator', () => {
afterEach(() => {
delete process.env.OTEL_PROPAGATORS;
});

it('should return default when env var is unset', () => {
assert.deepStrictEqual(getPropagator().fields(), [
'traceparent',
'tracestate',
'baggage',
]);
});

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

it('should return the selected propagators when multiple are in the list', () => {
process.env.OTEL_PROPAGATORS = 'b3,jaeger';
assert.deepStrictEqual(getPropagator().fields(), ['b3', 'uber-trace-id']);
});
});
});
10 changes: 10 additions & 0 deletions package-lock.json

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

Loading