From 1218a3c7e0b397fff9bc3ec54eafd6506603e5fa Mon Sep 17 00:00:00 2001 From: legendecas Date: Mon, 10 May 2021 23:13:16 +0800 Subject: [PATCH 1/3] fix: remove redundant try-catch from http/https server examples (#2195) --- examples/http/server.js | 26 +++++++++++--------------- examples/https/server.js | 26 +++++++++++--------------- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/examples/http/server.js b/examples/http/server.js index d7a90310fb..604f688f84 100644 --- a/examples/http/server.js +++ b/examples/http/server.js @@ -29,21 +29,17 @@ function handleRequest(request, response) { }); // Annotate our span to capture metadata about the operation span.addEvent('invoking handleRequest'); - try { - const body = []; - request.on('error', (err) => console.log(err)); - request.on('data', (chunk) => body.push(chunk)); - request.on('end', () => { - // deliberately sleeping to mock some action. - setTimeout(() => { - span.end(); - response.end('Hello World!'); - }, 2000); - }); - } catch (err) { - console.error(err); - span.end(); - } + + const body = []; + request.on('error', (err) => console.log(err)); + request.on('data', (chunk) => body.push(chunk)); + request.on('end', () => { + // deliberately sleeping to mock some action. + setTimeout(() => { + span.end(); + response.end('Hello World!'); + }, 2000); + }); } startServer(8080); diff --git a/examples/https/server.js b/examples/https/server.js index a3d480f568..d4a2357368 100644 --- a/examples/https/server.js +++ b/examples/https/server.js @@ -34,21 +34,17 @@ function handleRequest(request, response) { }); // Annotate our span to capture metadata about the operation span.addEvent('invoking handleRequest'); - try { - const body = []; - request.on('error', (err) => console.log(err)); - request.on('data', (chunk) => body.push(chunk)); - request.on('end', () => { - // deliberately sleeping to mock some action. - setTimeout(() => { - span.end(); - response.end('Hello World!'); - }, 2000); - }); - } catch (err) { - console.log(err); - span.end(); - } + + const body = []; + request.on('error', (err) => console.log(err)); + request.on('data', (chunk) => body.push(chunk)); + request.on('end', () => { + // deliberately sleeping to mock some action. + setTimeout(() => { + span.end(); + response.end('Hello World!'); + }, 2000); + }); } startServer(443); From 5cd02aa12e0bf08a9736f11f09ebaaad582eed27 Mon Sep 17 00:00:00 2001 From: Valentin Marchaud Date: Tue, 11 May 2021 20:44:02 +0200 Subject: [PATCH 2/3] chore(tracing): dont use internal core type #2177 (#2178) Co-authored-by: Daniel Dyla --- packages/opentelemetry-core/src/index.ts | 1 + packages/opentelemetry-tracing/src/config.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index 7d0de201c0..9cbf364276 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -30,6 +30,7 @@ export * from './trace/sampler/AlwaysOnSampler'; export * from './trace/sampler/ParentBasedSampler'; export * from './trace/sampler/TraceIdRatioBasedSampler'; export * from './trace/TraceState'; +export * from './utils/environment'; export * from './utils/sampling'; export * from './utils/url'; export * from './utils/wrap'; diff --git a/packages/opentelemetry-tracing/src/config.ts b/packages/opentelemetry-tracing/src/config.ts index 4e770eb5fa..2db3db8adc 100644 --- a/packages/opentelemetry-tracing/src/config.ts +++ b/packages/opentelemetry-tracing/src/config.ts @@ -21,9 +21,9 @@ import { getEnv, TracesSamplerValues, ParentBasedSampler, + ENVIRONMENT, TraceIdRatioBasedSampler, } from '@opentelemetry/core'; -import { ENVIRONMENT } from '@opentelemetry/core/src/utils/environment'; const env = getEnv(); const FALLBACK_OTEL_TRACES_SAMPLER = TracesSamplerValues.AlwaysOn; From 5f7ec008e407b554ee69480eb740435add0c1134 Mon Sep 17 00:00:00 2001 From: Valentin Marchaud Date: Tue, 11 May 2021 21:04:01 +0200 Subject: [PATCH 3/3] feat(jaeger-propagator): propagate/extract baggage #2137 (#2158) --- .../opentelemetry-propagator-jaeger/README.md | 5 ++ .../src/JaegerPropagator.ts | 77 +++++++++++++---- .../test/JaegerPropagator.test.ts | 84 +++++++++++++++++++ 3 files changed, 148 insertions(+), 18 deletions(-) diff --git a/packages/opentelemetry-propagator-jaeger/README.md b/packages/opentelemetry-propagator-jaeger/README.md index 836c3ab743..f324221271 100644 --- a/packages/opentelemetry-propagator-jaeger/README.md +++ b/packages/opentelemetry-propagator-jaeger/README.md @@ -35,6 +35,10 @@ provider.register({ }); ``` +## Baggage Notes + +Jeager Baggage is represented as multiple headers where the names are carrier dependent. For this reason, they are omitted from the `fields` method. This behavior should be taken into account if your application relies on the `fields` functionality. See the [specification][fields-spec-url] for more details. + ## Trace on Jaeger UI ![example image](jaeger-tracing.png) @@ -58,3 +62,4 @@ Apache 2.0 - See [LICENSE][license-url] for more information. [devDependencies-url]: https://david-dm.org/open-telemetry/opentelemetry-js?path=packages%2Fopentelemetry-propagator-jaeger&type=dev [npm-url]: https://www.npmjs.com/package/@opentelemetry/propagator-jaeger [npm-img]: https://badge.fury.io/js/%40opentelemetry%2Fpropagator-jaeger.svg +[fields-spec-url]: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#fields diff --git a/packages/opentelemetry-propagator-jaeger/src/JaegerPropagator.ts b/packages/opentelemetry-propagator-jaeger/src/JaegerPropagator.ts index c631f5b81d..7f9daf1fff 100644 --- a/packages/opentelemetry-propagator-jaeger/src/JaegerPropagator.ts +++ b/packages/opentelemetry-propagator-jaeger/src/JaegerPropagator.ts @@ -16,7 +16,9 @@ import { Context, + getBaggage, getSpanContext, + setBaggage, setSpanContext, SpanContext, TraceFlags, @@ -24,9 +26,12 @@ import { TextMapPropagator, TextMapSetter, isInstrumentationSuppressed, + createBaggage, } from '@opentelemetry/api'; export const UBER_TRACE_ID_HEADER = 'uber-trace-id'; +export const UBER_BAGGAGE_HEADER_PREFIX = 'uberctx'; +const UBER_BAGGAGE_HEADER_REGEX = /^uberctx-(.+)/i; /** * Propagates {@link SpanContext} through Trace Context format propagation. @@ -55,17 +60,28 @@ export class JaegerPropagator implements TextMapPropagator { inject(context: Context, carrier: unknown, setter: TextMapSetter) { const spanContext = getSpanContext(context); - if (!spanContext || isInstrumentationSuppressed(context)) return; - - const traceFlags = `0${(spanContext.traceFlags || TraceFlags.NONE).toString( - 16 - )}`; - - setter.set( - carrier, - this._jaegerTraceHeader, - `${spanContext.traceId}:${spanContext.spanId}:0:${traceFlags}` - ); + const baggage = getBaggage(context); + if (spanContext && isInstrumentationSuppressed(context) === false) { + const traceFlags = `0${( + spanContext.traceFlags || TraceFlags.NONE + ).toString(16)}`; + + setter.set( + carrier, + this._jaegerTraceHeader, + `${spanContext.traceId}:${spanContext.spanId}:0:${traceFlags}` + ); + } + + if (baggage) { + for (const [key, entry] of baggage.getAllEntries()) { + setter.set( + carrier, + `${UBER_BAGGAGE_HEADER_PREFIX}-${key}`, + encodeURIComponent(entry.value) + ); + } + } } extract(context: Context, carrier: unknown, getter: TextMapGetter): Context { @@ -73,13 +89,38 @@ export class JaegerPropagator implements TextMapPropagator { const uberTraceId = Array.isArray(uberTraceIdHeader) ? uberTraceIdHeader[0] : uberTraceIdHeader; - - if (typeof uberTraceId !== 'string') return context; - - const spanContext = deserializeSpanContext(uberTraceId); - if (!spanContext) return context; - - return setSpanContext(context, spanContext); + const baggageValues = getter + .keys(carrier) + .filter(key => UBER_BAGGAGE_HEADER_REGEX.test(key)) + .map(key => { + const value = getter.get(carrier, key); + return { + key: key.substring(UBER_BAGGAGE_HEADER_PREFIX.length + 1), + value: Array.isArray(value) ? value[0] : value, + }; + }); + + let newContext = context; + // if the trace id header is present and valid, inject it into the context + if (typeof uberTraceId === 'string') { + const spanContext = deserializeSpanContext(uberTraceId); + if (spanContext) { + newContext = setSpanContext(newContext, spanContext); + } + } + if (baggageValues.length === 0) return newContext; + + // if baggage values are present, inject it into the current baggage + let currentBaggage = getBaggage(context) ?? createBaggage(); + for (const baggageEntry of baggageValues) { + if (baggageEntry.value === undefined) continue; + currentBaggage = currentBaggage.setEntry(baggageEntry.key, { + value: decodeURIComponent(baggageEntry.value), + }); + } + newContext = setBaggage(newContext, currentBaggage); + + return newContext; } fields(): string[] { diff --git a/packages/opentelemetry-propagator-jaeger/test/JaegerPropagator.test.ts b/packages/opentelemetry-propagator-jaeger/test/JaegerPropagator.test.ts index e5a6f82d85..e12e6bfc62 100644 --- a/packages/opentelemetry-propagator-jaeger/test/JaegerPropagator.test.ts +++ b/packages/opentelemetry-propagator-jaeger/test/JaegerPropagator.test.ts @@ -15,10 +15,13 @@ */ import { + createBaggage, defaultTextMapGetter, defaultTextMapSetter, + getBaggage, getSpanContext, ROOT_CONTEXT, + setBaggage, setSpanContext, SpanContext, suppressInstrumentation, @@ -29,6 +32,7 @@ import * as assert from 'assert'; import { JaegerPropagator, UBER_TRACE_ID_HEADER, + UBER_BAGGAGE_HEADER_PREFIX, } from '../src/JaegerPropagator'; describe('JaegerPropagator', () => { @@ -92,6 +96,28 @@ describe('JaegerPropagator', () => { ); assert.strictEqual(carrier[UBER_TRACE_ID_HEADER], undefined); }); + + it('should propagate baggage with url encoded values', () => { + const baggage = createBaggage({ + test: { + value: '1', + }, + myuser: { + value: '%id%', + }, + }); + + jaegerPropagator.inject( + setBaggage(ROOT_CONTEXT, baggage), + carrier, + defaultTextMapSetter + ); + assert.strictEqual(carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-test`], '1'); + assert.strictEqual( + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-myuser`], + encodeURIComponent('%id%') + ); + }); }); describe('.extract()', () => { @@ -177,6 +203,64 @@ describe('JaegerPropagator', () => { undefined ); }); + + it('should extract baggage from carrier', () => { + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-test`] = 'value'; + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-myuser`] = '%25id%25'; + const extractedBaggage = getBaggage( + jaegerPropagator.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) + ); + + const firstEntry = extractedBaggage?.getEntry('test'); + assert(typeof firstEntry !== 'undefined'); + assert(firstEntry.value === 'value'); + const secondEntry = extractedBaggage?.getEntry('myuser'); + assert(typeof secondEntry !== 'undefined'); + assert(secondEntry.value === '%id%'); + }); + + it('should extract baggage from carrier and not override current one', () => { + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-test`] = 'value'; + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-myuser`] = '%25id%25'; + const extractedBaggage = getBaggage( + jaegerPropagator.extract( + setBaggage(ROOT_CONTEXT, createBaggage({ one: { value: 'two' } })), + carrier, + defaultTextMapGetter + ) + ); + + const firstEntry = extractedBaggage?.getEntry('test'); + assert(typeof firstEntry !== 'undefined'); + assert(firstEntry.value === 'value'); + const secondEntry = extractedBaggage?.getEntry('myuser'); + assert(typeof secondEntry !== 'undefined'); + assert(secondEntry.value === '%id%'); + const alreadyExistingEntry = extractedBaggage?.getEntry('one'); + assert(typeof alreadyExistingEntry !== 'undefined'); + assert(alreadyExistingEntry.value === 'two'); + }); + + it('should handle invalid baggage from carrier (undefined)', () => { + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-test`] = undefined; + const extractedBaggage = getBaggage( + jaegerPropagator.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) + ); + + const firstEntry = extractedBaggage?.getEntry('test'); + assert(typeof firstEntry === 'undefined'); + }); + + it('should handle invalid baggage from carrier (array)', () => { + carrier[`${UBER_BAGGAGE_HEADER_PREFIX}-test`] = ['one', 'two']; + const extractedBaggage = getBaggage( + jaegerPropagator.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) + ); + + const firstEntry = extractedBaggage?.getEntry('test'); + assert(typeof firstEntry !== 'undefined'); + assert(firstEntry.value === 'one'); + }); }); describe('.fields()', () => {