From 7b1ed399eefa250fcf443171a793890cb88aa5cf Mon Sep 17 00:00:00 2001 From: Vera Reynolds Date: Thu, 29 Apr 2021 02:29:55 -0600 Subject: [PATCH] feat(shim-opentracing): add propagator configuration (#2153) Co-authored-by: Valentin Marchaud --- .../opentelemetry-shim-opentracing/README.md | 14 +- .../package.json | 2 + .../src/shim.ts | 105 +++++---- .../test/Shim.test.ts | 217 +++++++++++++----- .../tsconfig.json | 6 + 5 files changed, 244 insertions(+), 100 deletions(-) diff --git a/packages/opentelemetry-shim-opentracing/README.md b/packages/opentelemetry-shim-opentracing/README.md index 9e904937b9..7ed08f4f5b 100644 --- a/packages/opentelemetry-shim-opentracing/README.md +++ b/packages/opentelemetry-shim-opentracing/README.md @@ -7,8 +7,6 @@ OpenTracing shim allows existing OpenTracing instrumentation to report to OpenTelemetry -Note: Baggage is currently not propagated, see [issues/329](https://github.com/open-telemetry/opentelemetry-js/issues/329). - ## Installation ```bash @@ -34,6 +32,18 @@ opentracing.initGlobalTracer(new TracerShim(tracer)); ``` +Optionally, you can specify propagators to be used for the OpenTracing `TextMap` and `HttpHeaders` formats: + +```javascript +var b3Propagator = new B3Propagator(); +new TracerShim(tracer, { + textMapPropagator: b3Propagator, + httpHeadersPropagator: b3Propagator +}) +``` + +If propagators are not specified, OpenTelemetry's global propagator will be used. + See [examples/opentracing-shim](https://github.com/open-telemetry/opentelemetry-js/tree/main/examples/opentracing-shim) for a short example. ## License diff --git a/packages/opentelemetry-shim-opentracing/package.json b/packages/opentelemetry-shim-opentracing/package.json index c0be2b54d1..8362a8e8b4 100644 --- a/packages/opentelemetry-shim-opentracing/package.json +++ b/packages/opentelemetry-shim-opentracing/package.json @@ -40,6 +40,8 @@ "devDependencies": { "@opentelemetry/api": "^1.0.0-rc.0", "@opentelemetry/tracing": "0.19.0", + "@opentelemetry/propagator-b3": "0.19.0", + "@opentelemetry/propagator-jaeger": "0.19.0", "@types/mocha": "8.2.2", "@types/node": "14.14.41", "codecov": "3.8.1", diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts index 84d786d70d..0fedcb5e01 100644 --- a/packages/opentelemetry-shim-opentracing/src/shim.ts +++ b/packages/opentelemetry-shim-opentracing/src/shim.ts @@ -20,6 +20,7 @@ import { createBaggage, SpanAttributes, SpanAttributeValue, + TextMapPropagator, } from '@opentelemetry/api'; function translateReferences(references: opentracing.Reference[]): api.Link[] { @@ -65,7 +66,7 @@ function getContextWithParent(options: opentracing.SpanOptions) { } /** - * SpanContextShim wraps a {@link types.SpanContext} and implements the + * SpanContextShim wraps a {@link api.SpanContext} and implements the * OpenTracing span context API. */ export class SpanContextShim extends opentracing.SpanContext { @@ -79,7 +80,7 @@ export class SpanContextShim extends opentracing.SpanContext { } /** - * Returns the underlying {@link types.SpanContext} + * Returns the underlying {@link api.SpanContext} */ getSpanContext(): api.SpanContext { return this._spanContext; @@ -116,16 +117,18 @@ export class SpanContextShim extends opentracing.SpanContext { } /** - * TracerShim wraps a {@link types.Tracer} and implements the + * TracerShim wraps a {@link api.Tracer} and implements the * OpenTracing tracer API. */ export class TracerShim extends opentracing.Tracer { private readonly _tracer: api.Tracer; + private readonly _propagators: ShimPropagators | undefined; - constructor(tracer: api.Tracer) { + constructor(tracer: api.Tracer, propagators?: ShimPropagators) { super(); this._tracer = tracer; + this._propagators = propagators; } startSpan( @@ -163,60 +166,62 @@ export class TracerShim extends opentracing.Tracer { const oTelSpanBaggage: api.Baggage = spanContextShim.getBaggage(); if (!carrier || typeof carrier !== 'object') return; - switch (format) { - case opentracing.FORMAT_HTTP_HEADERS: - case opentracing.FORMAT_TEXT_MAP: { - api.propagation.inject( - api.setBaggage( - api.setSpanContext(api.ROOT_CONTEXT, oTelSpanContext), - oTelSpanBaggage - ), - carrier - ); - return; - } - case opentracing.FORMAT_BINARY: { - api.diag.warn( - 'OpentracingShim.inject() does not support FORMAT_BINARY' - ); - // @todo: Implement binary formats - return; - } - default: + + if (format === opentracing.FORMAT_BINARY) { + api.diag.warn('OpentracingShim.inject() does not support FORMAT_BINARY'); + // @todo: Implement binary format + return; + } + + const propagator = this._getPropagator(format); + if (propagator !== undefined) { + const context = api.setBaggage( + api.setSpanContext(api.ROOT_CONTEXT, oTelSpanContext), + oTelSpanBaggage + ); + propagator.inject(context, carrier, api.defaultTextMapSetter); } } _extract(format: string, carrier: unknown): opentracing.SpanContext | null { - switch (format) { - case opentracing.FORMAT_HTTP_HEADERS: - case opentracing.FORMAT_TEXT_MAP: { - const context: api.Context = api.propagation.extract( - api.ROOT_CONTEXT, - carrier - ); - const spanContext = api.getSpanContext(context); - const baggage = api.getBaggage(context); - - if (!spanContext) { - return null; - } - return new SpanContextShim(spanContext, baggage || createBaggage()); - } - case opentracing.FORMAT_BINARY: { - // @todo: Implement binary format - api.diag.warn( - 'OpentracingShim.extract() does not support FORMAT_BINARY' - ); + if (format === opentracing.FORMAT_BINARY) { + api.diag.warn('OpentracingShim.extract() does not support FORMAT_BINARY'); + // @todo: Implement binary format + return null; + } + + const propagator = this._getPropagator(format); + if (propagator !== undefined) { + const context: api.Context = propagator.extract( + api.ROOT_CONTEXT, + carrier, + api.defaultTextMapGetter + ); + const spanContext = api.getSpanContext(context); + const baggage = api.getBaggage(context); + + if (!spanContext) { return null; } - default: + return new SpanContextShim(spanContext, baggage || createBaggage()); } return null; } + + private _getPropagator(format: string): TextMapPropagator | undefined { + switch (format) { + case opentracing.FORMAT_TEXT_MAP: + return this._propagators?.textMapPropagator ?? api.propagation; + case opentracing.FORMAT_HTTP_HEADERS: + return this._propagators?.httpHeadersPropagator ?? api.propagation; + default: + return; + } + } } /** - * SpanShim wraps an {@link types.Span} and implements the OpenTracing Span API + * SpanShim wraps an {@link api.Span} and implements the OpenTracing Span API * around it. * **/ @@ -334,3 +339,11 @@ export class SpanShim extends opentracing.Span { return this._span; } } + +/** + * Propagator configuration for the {@link TracerShim} + */ +export interface ShimPropagators { + textMapPropagator?: TextMapPropagator; + httpHeadersPropagator?: TextMapPropagator; +} diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts index b571150013..f33b45877d 100644 --- a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts +++ b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts @@ -17,26 +17,28 @@ import * as assert from 'assert'; import * as opentracing from 'opentracing'; import { BasicTracerProvider, Span } from '@opentelemetry/tracing'; -import { TracerShim, SpanShim, SpanContextShim } from '../src/shim'; +import { SpanContextShim, SpanShim, TracerShim } from '../src/shim'; import { - timeInputToHrTime, - HttpTraceContext, CompositePropagator, HttpBaggage, + HttpTraceContext, + timeInputToHrTime, } from '@opentelemetry/core'; import { createBaggage, + defaultTextMapGetter, + defaultTextMapSetter, + getSpanContext, INVALID_SPAN_CONTEXT, propagation, + ROOT_CONTEXT, + setSpanContext, } from '@opentelemetry/api'; import { performance } from 'perf_hooks'; +import { B3Propagator } from '@opentelemetry/propagator-b3'; +import { JaegerHttpTracePropagator } from '@opentelemetry/propagator-jaeger'; describe('OpenTracing Shim', () => { - const provider = new BasicTracerProvider(); - const shimTracer: opentracing.Tracer = new TracerShim( - provider.getTracer('default') - ); - opentracing.initGlobalTracer(shimTracer); const compositePropagator = new CompositePropagator({ propagators: [new HttpTraceContext(), new HttpBaggage()], }); @@ -44,15 +46,22 @@ describe('OpenTracing Shim', () => { propagation.setGlobalPropagator(compositePropagator); describe('TracerShim', () => { + let shimTracer: opentracing.Tracer; let span: opentracing.Span; let context: opentracing.SpanContext; - beforeEach(() => { - span = shimTracer.startSpan('my-span'); - context = span.context(); - }); + describe('propagation using default propagators', () => { + before(() => { + const provider = new BasicTracerProvider(); + shimTracer = new TracerShim(provider.getTracer('default')); + opentracing.initGlobalTracer(shimTracer); + }); + + beforeEach(() => { + span = shimTracer.startSpan('my-span'); + context = span.context(); + }); - describe('propagation', () => { it('injects/extracts a span object', () => { const carrier: { [key: string]: unknown } = {}; shimTracer.inject(span, opentracing.FORMAT_HTTP_HEADERS, carrier); @@ -116,48 +125,145 @@ describe('OpenTracing Shim', () => { }); }); - it('creates parent/child relationship using a span object', () => { - const childSpan = shimTracer.startSpan('other-span', { - childOf: span, - }) as SpanShim; - assert.strictEqual( - (childSpan.getSpan() as Span).parentSpanId, - context.toSpanId() - ); - assert.strictEqual( - childSpan.context().toTraceId(), - span.context().toTraceId() - ); - }); + describe('propagation using configured propagators', () => { + const jaegerHttpTracePropagator = new JaegerHttpTracePropagator(); + const b3Propagator = new B3Propagator(); + before(() => { + const provider = new BasicTracerProvider(); + shimTracer = new TracerShim(provider.getTracer('default'), { + textMapPropagator: b3Propagator, + httpHeadersPropagator: jaegerHttpTracePropagator, + }); + opentracing.initGlobalTracer(shimTracer); + }); - it('creates parent/child relationship using a context object', () => { - const childSpan = shimTracer.startSpan('other-span', { - childOf: context, - }) as SpanShim; - assert.strictEqual( - (childSpan.getSpan() as Span).parentSpanId, - context.toSpanId() - ); - assert.strictEqual( - childSpan.context().toTraceId(), - span.context().toTraceId() - ); + beforeEach(() => { + span = shimTracer.startSpan('my-span'); + context = span.context(); + }); + + it('injects HTTP carriers', () => { + const carrier: { [key: string]: unknown } = {}; + shimTracer.inject(context, opentracing.FORMAT_HTTP_HEADERS, carrier); + const extractedContext = getSpanContext( + jaegerHttpTracePropagator.extract( + ROOT_CONTEXT, + carrier, + defaultTextMapGetter + ) + ); + assert.ok(extractedContext !== null); + assert.strictEqual(extractedContext?.traceId, context.toTraceId()); + assert.strictEqual(extractedContext?.spanId, context.toSpanId()); + }); + + it('extracts HTTP carriers', () => { + const carrier: { [key: string]: unknown } = {}; + jaegerHttpTracePropagator.inject( + setSpanContext( + ROOT_CONTEXT, + (context as SpanContextShim).getSpanContext() + ), + carrier, + defaultTextMapSetter + ); + + const extractedContext = shimTracer.extract( + opentracing.FORMAT_HTTP_HEADERS, + carrier + ); + assert.ok(extractedContext !== null); + assert.strictEqual(extractedContext!.toTraceId(), context.toTraceId()); + assert.strictEqual(extractedContext!.toSpanId(), context.toSpanId()); + }); + + it('injects TextMap carriers', () => { + const carrier: { [key: string]: unknown } = {}; + shimTracer.inject(context, opentracing.FORMAT_TEXT_MAP, carrier); + const extractedContext = getSpanContext( + b3Propagator.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter) + ); + assert.ok(extractedContext !== null); + assert.strictEqual(extractedContext?.traceId, context.toTraceId()); + assert.strictEqual(extractedContext?.spanId, context.toSpanId()); + }); + + it('extracts TextMap carriers', () => { + const carrier: { [key: string]: unknown } = {}; + b3Propagator.inject( + setSpanContext( + ROOT_CONTEXT, + (context as SpanContextShim).getSpanContext() + ), + carrier, + defaultTextMapSetter + ); + + const extractedContext = shimTracer.extract( + opentracing.FORMAT_TEXT_MAP, + carrier + ); + assert.ok(extractedContext !== null); + assert.strictEqual(extractedContext!.toTraceId(), context.toTraceId()); + assert.strictEqual(extractedContext!.toSpanId(), context.toSpanId()); + }); }); - it('translates span options correctly', () => { - const now = performance.now(); - const opentracingOptions: opentracing.SpanOptions = { - startTime: now, - tags: { key: 'value', count: 1 }, - references: [opentracing.followsFrom(context)], - }; - span = shimTracer.startSpan('my-span', opentracingOptions); - - const otSpan = (span as SpanShim).getSpan() as Span; - - assert.strictEqual(otSpan.links.length, 1); - assert.deepStrictEqual(otSpan.startTime, timeInputToHrTime(now)); - assert.deepStrictEqual(otSpan.attributes, opentracingOptions.tags); + describe('starting spans', () => { + before(() => { + const provider = new BasicTracerProvider(); + shimTracer = new TracerShim(provider.getTracer('default')); + opentracing.initGlobalTracer(shimTracer); + }); + + beforeEach(() => { + span = shimTracer.startSpan('my-span'); + context = span.context(); + }); + + it('creates parent/child relationship using a span object', () => { + const childSpan = shimTracer.startSpan('other-span', { + childOf: span, + }) as SpanShim; + assert.strictEqual( + (childSpan.getSpan() as Span).parentSpanId, + context.toSpanId() + ); + assert.strictEqual( + childSpan.context().toTraceId(), + span.context().toTraceId() + ); + }); + + it('creates parent/child relationship using a context object', () => { + const childSpan = shimTracer.startSpan('other-span', { + childOf: context, + }) as SpanShim; + assert.strictEqual( + (childSpan.getSpan() as Span).parentSpanId, + context.toSpanId() + ); + assert.strictEqual( + childSpan.context().toTraceId(), + span.context().toTraceId() + ); + }); + + it('translates span options correctly', () => { + const now = performance.now(); + const opentracingOptions: opentracing.SpanOptions = { + startTime: now, + tags: { key: 'value', count: 1 }, + references: [opentracing.followsFrom(context)], + }; + span = shimTracer.startSpan('my-span', opentracingOptions); + + const otSpan = (span as SpanShim).getSpan() as Span; + + assert.strictEqual(otSpan.links.length, 1); + assert.deepStrictEqual(otSpan.startTime, timeInputToHrTime(now)); + assert.deepStrictEqual(otSpan.attributes, opentracingOptions.tags); + }); }); }); @@ -171,9 +277,16 @@ describe('OpenTracing Shim', () => { }); describe('span', () => { + let shimTracer: opentracing.Tracer; let span: SpanShim; let otSpan: Span; + before(() => { + const provider = new BasicTracerProvider(); + shimTracer = new TracerShim(provider.getTracer('default')); + opentracing.initGlobalTracer(shimTracer); + }); + beforeEach(() => { span = shimTracer.startSpan('my-span', { startTime: performance.now(), diff --git a/packages/opentelemetry-shim-opentracing/tsconfig.json b/packages/opentelemetry-shim-opentracing/tsconfig.json index ee62966fd1..343988c19f 100644 --- a/packages/opentelemetry-shim-opentracing/tsconfig.json +++ b/packages/opentelemetry-shim-opentracing/tsconfig.json @@ -12,6 +12,12 @@ { "path": "../opentelemetry-core" }, + { + "path": "../opentelemetry-propagator-b3" + }, + { + "path": "../opentelemetry-propagator-jaeger" + }, { "path": "../opentelemetry-tracing" }