From 344d9c0d2d6d8aa22041c1f0aa30db0105a47549 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Wed, 8 Apr 2020 15:38:17 +0200 Subject: [PATCH] feat(http-plugin): add options to disable new spans if no parent #931 --- packages/opentelemetry-plugin-http/README.md | 2 + .../opentelemetry-plugin-http/src/http.ts | 54 +++++++++++-- .../opentelemetry-plugin-http/src/types.ts | 4 + .../test/functionals/http-enable.test.ts | 79 +++++++++++++++++++ 4 files changed, 132 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-plugin-http/README.md b/packages/opentelemetry-plugin-http/README.md index 8daf06f8446..bcdc59e230e 100644 --- a/packages/opentelemetry-plugin-http/README.md +++ b/packages/opentelemetry-plugin-http/README.md @@ -55,6 +55,8 @@ Http plugin has few options available to choose from. You can set the following: | [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all incoming requests that match paths | | [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all outgoing requests that match urls | | [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `string` | The primary server name of the matched virtual host. | +| `requireParentforOutgoingSpans` | Boolean | Require that is a parent span to create new span for outgoing requests. | +| `requireParentforIncomingSpans` | Boolean | Require that is a parent span to create new span for incoming requests. | ## Useful links - For more information on OpenTelemetry, visit: diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 53bbebe5bbd..ee9bb05df0e 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -22,8 +22,15 @@ import { SpanKind, SpanOptions, Status, + SpanContext, + TraceFlags, + NoopSpan, } from '@opentelemetry/api'; -import { BasePlugin } from '@opentelemetry/core'; +import { + BasePlugin, + NoRecordingSpan, + getExtractedSpanContext, +} from '@opentelemetry/core'; import { ClientRequest, IncomingMessage, @@ -57,6 +64,12 @@ export class HttpPlugin extends BasePlugin { /** keep track on spans not ended */ private readonly _spanNotEnded: WeakSet; + private readonly _emptySpanContext: SpanContext = { + traceId: '', + spanId: '', + traceFlags: TraceFlags.NONE, + }; + constructor(readonly moduleName: string, readonly version: string) { super(`@opentelemetry/plugin-${moduleName}`, VERSION); // For now component is equal to moduleName but it can change in the future. @@ -295,10 +308,23 @@ export class HttpPlugin extends BasePlugin { }; return context.with(propagation.extract(headers), () => { - const span = plugin._startHttpSpan( - `${method} ${pathname}`, - spanOptions - ); + let span: Span = new NoopSpan(); + const hasParent = plugin._tracer.getCurrentSpan() !== undefined; + /* + * If a parent is required but not present, we use a `NoRecordingSpan` to still + * propagate context without recording it. + */ + if ( + plugin._config.requireParentforIncomingSpans === true && + hasParent === false + ) { + const spanContext = + getExtractedSpanContext(context.active()) ?? + plugin._emptySpanContext; + span = new NoRecordingSpan(spanContext); + } else { + span = plugin._startHttpSpan(`${method} ${pathname}`, spanOptions); + } return plugin._tracer.withSpan(span, () => { context.bind(request); @@ -396,8 +422,22 @@ export class HttpPlugin extends BasePlugin { const spanOptions: SpanOptions = { kind: SpanKind.CLIENT, }; - - const span = plugin._startHttpSpan(operationName, spanOptions); + const hasParent = plugin._tracer.getCurrentSpan() !== undefined; + let span: Span = new NoopSpan(); + /* + * If a parent is required but not present, we use a `NoRecordingSpan` to still + * propagate context without recording it. + */ + if ( + plugin._config.requireParentforOutgoingSpans === true && + hasParent === false + ) { + const spanContext = + getExtractedSpanContext(context.active()) ?? plugin._emptySpanContext; + span = new NoRecordingSpan(spanContext); + } else { + span = plugin._startHttpSpan(operationName, spanOptions); + } return plugin._tracer.withSpan(span, () => { if (!optionsParsed.headers) optionsParsed.headers = {}; diff --git a/packages/opentelemetry-plugin-http/src/types.ts b/packages/opentelemetry-plugin-http/src/types.ts index 335e0bbc304..b7e230d37a1 100644 --- a/packages/opentelemetry-plugin-http/src/types.ts +++ b/packages/opentelemetry-plugin-http/src/types.ts @@ -69,6 +69,10 @@ export interface HttpPluginConfig extends PluginConfig { applyCustomAttributesOnSpan?: HttpCustomAttributeFunction; /** The primary server name of the matched virtual host. */ serverName?: string; + /** Require parent to create span for outgoing requests */ + requireParentforOutgoingSpans?: boolean; + /** Require parent to create span for incoming requests */ + requireParentforIncomingSpans?: boolean; } export interface Err extends Error { diff --git a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts index 94c9c410f60..1707db8c4e3 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -696,5 +696,84 @@ describe('HttpPlugin', () => { req.end(); }); }); + + describe('with require parent span', () => { + beforeEach(() => { + memoryExporter.reset(); + plugin.enable(http, provider, provider.logger, {}); + server = http.createServer((request, response) => { + response.end('Test Server Response'); + }); + server.listen(serverPort); + }); + + afterEach(() => { + server.close(); + plugin.disable(); + }); + + it(`should not trace without parent with options enabled (both client & server)`, async () => { + plugin.disable(); + const config: HttpPluginConfig = { + requireParentforIncomingSpans: true, + requireParentforOutgoingSpans: true, + }; + plugin.enable(http, provider, provider.logger, config); + const testPath = `/test/test`; + await httpRequest.get( + `${protocol}://${hostname}:${serverPort}${testPath}` + ); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 0); + }); + + it(`should not trace without parent with options enabled (client only)`, async () => { + plugin.disable(); + const config: HttpPluginConfig = { + requireParentforOutgoingSpans: true, + }; + plugin.enable(http, provider, provider.logger, config); + const testPath = `/test/test`; + const result = await httpRequest.get( + `${protocol}://${hostname}:${serverPort}${testPath}` + ); + assert( + result.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY] !== undefined + ); + assert( + result.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY] !== undefined + ); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assert.strictEqual( + spans.every(span => span.kind === SpanKind.SERVER), + true + ); + }); + + it(`should not trace without parent with options enabled (server only)`, async () => { + plugin.disable(); + const config: HttpPluginConfig = { + requireParentforIncomingSpans: true, + }; + plugin.enable(http, provider, provider.logger, config); + const testPath = `/test/test`; + const result = await httpRequest.get( + `${protocol}://${hostname}:${serverPort}${testPath}` + ); + assert( + result.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY] !== undefined + ); + assert( + result.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY] !== undefined + ); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assert.strictEqual( + spans.every(span => span.kind === SpanKind.CLIENT), + true + ); + }); + }); }); });