From 8bc801936053a56438dccea4c5f67e3968137db4 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Sat, 19 Sep 2020 17:57:52 +0200 Subject: [PATCH 1/3] fix: do not set outgoing http span as active in the context #1479 --- .../opentelemetry-plugin-http/src/http.ts | 30 +++++++++++-------- .../test/functionals/http-enable.test.ts | 10 ++++++- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 96c87b1375..699d6ec438 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -24,6 +24,7 @@ import { SpanContext, TraceFlags, getExtractedSpanContext, + setActiveSpan } from '@opentelemetry/api'; import { BasePlugin, NoRecordingSpan } from '@opentelemetry/core'; import type { @@ -410,21 +411,24 @@ export class HttpPlugin extends BasePlugin { kind: SpanKind.CLIENT, }; const span = plugin._startHttpSpan(operationName, spanOptions); + if (!optionsParsed.headers) { + optionsParsed.headers = {}; + } + propagation.inject( + optionsParsed.headers, + undefined, + setActiveSpan(context.active(), span) + ); - return plugin._tracer.withSpan(span, () => { - if (!optionsParsed.headers) optionsParsed.headers = {}; - propagation.inject(optionsParsed.headers); - - const request: ClientRequest = plugin._safeExecute( - span, - () => original.apply(this, [optionsParsed, ...args]), - true - ); + const request: ClientRequest = plugin._safeExecute( + span, + () => original.apply(this, [optionsParsed, ...args]), + true + ); - plugin._logger.debug('%s plugin outgoingRequest', plugin.moduleName); - plugin._tracer.bind(request); - return plugin._traceClientRequest(request, optionsParsed, span); - }); + plugin._logger.debug('%s plugin outgoingRequest', plugin.moduleName); + plugin._tracer.bind(request); + return plugin._traceClientRequest(request, optionsParsed, span); }; } 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 b78f504304..b51cb7541e 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -20,7 +20,7 @@ import { Span as ISpan, SpanKind, } from '@opentelemetry/api'; -import { NoopLogger } from '@opentelemetry/core'; +import { NoopLogger, getActiveSpan } from '@opentelemetry/core'; import { NodeTracerProvider } from '@opentelemetry/node'; import { InMemorySpanExporter, @@ -709,6 +709,14 @@ describe('HttpPlugin', () => { SpanKind.CLIENT ); }); + + it('should not set span as active in context for outgoing request', async () => { + assert.deepStrictEqual(getActiveSpan(context.active()), undefined); + await httpRequest.get(`${protocol}://${hostname}:${serverPort}/test`); + assert.deepStrictEqual(getActiveSpan(context.active()), undefined); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 2); + }); }); describe('with require parent span', () => { From e9e1cbe2b272c3bac5cc6c38290a130bb087dbd0 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Tue, 6 Oct 2020 11:43:41 +0200 Subject: [PATCH 2/3] chore: update test to check context inside http callback --- .../test/functionals/http-enable.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 b51cb7541e..07ad043a27 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -19,8 +19,9 @@ import { propagation, Span as ISpan, SpanKind, + getActiveSpan } from '@opentelemetry/api'; -import { NoopLogger, getActiveSpan } from '@opentelemetry/core'; +import { NoopLogger } from '@opentelemetry/core'; import { NodeTracerProvider } from '@opentelemetry/node'; import { InMemorySpanExporter, @@ -710,12 +711,12 @@ describe('HttpPlugin', () => { ); }); - it('should not set span as active in context for outgoing request', async () => { + it('should not set span as active in context for outgoing request', (done) => { assert.deepStrictEqual(getActiveSpan(context.active()), undefined); - await httpRequest.get(`${protocol}://${hostname}:${serverPort}/test`); - assert.deepStrictEqual(getActiveSpan(context.active()), undefined); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 2); + http.get(`${protocol}://${hostname}:${serverPort}/test`, (res) => { + assert.deepStrictEqual(getActiveSpan(context.active()), undefined); + done(); + }); }); }); From 9c9c087d020407e2b44e199fac71331d280bfdb4 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Tue, 6 Oct 2020 12:11:04 +0200 Subject: [PATCH 3/3] chore: lint --- packages/opentelemetry-plugin-http/src/http.ts | 2 +- .../test/functionals/http-enable.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 699d6ec438..cceda23e45 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -24,7 +24,7 @@ import { SpanContext, TraceFlags, getExtractedSpanContext, - setActiveSpan + setActiveSpan, } from '@opentelemetry/api'; import { BasePlugin, NoRecordingSpan } from '@opentelemetry/core'; import type { 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 07ad043a27..b08f868e8e 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -19,7 +19,7 @@ import { propagation, Span as ISpan, SpanKind, - getActiveSpan + getActiveSpan, } from '@opentelemetry/api'; import { NoopLogger } from '@opentelemetry/core'; import { NodeTracerProvider } from '@opentelemetry/node'; @@ -711,9 +711,9 @@ describe('HttpPlugin', () => { ); }); - it('should not set span as active in context for outgoing request', (done) => { + it('should not set span as active in context for outgoing request', done => { assert.deepStrictEqual(getActiveSpan(context.active()), undefined); - http.get(`${protocol}://${hostname}:${serverPort}/test`, (res) => { + http.get(`${protocol}://${hostname}:${serverPort}/test`, res => { assert.deepStrictEqual(getActiveSpan(context.active()), undefined); done(); });