diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index ebff2b41f0b..8dbe3a0a6ba 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -12,6 +12,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 ### :rocket: Features +* feat(instrumentation-http): provide `http.request.header.` at server span creation time [#6396](https://github.com/open-telemetry/opentelemetry-js/pull/6396) @vitorvasc + ### :bug: Bug Fixes * fix(instrumentation-http): guard against double-instrumentation if loaded with `require('http')` and `import 'http'` [#6428](https://github.com/open-telemetry/opentelemetry-js/issues/6428) @trentm diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 8a2b6a9068b..b615f4ab3b6 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -469,12 +469,15 @@ export class HttpInstrumentation extends InstrumentationBase - request.getHeader(header) + span.setAttributes( + this._headerCapture.client.captureRequestHeaders(header => + request.getHeader(header) + ) ); - this._headerCapture.client.captureResponseHeaders( - span, - header => response.headers[header] + span.setAttributes( + this._headerCapture.client.captureResponseHeaders( + header => response.headers[header] + ) ); context.bind(context.active(), response); @@ -633,6 +636,13 @@ export class HttpInstrumentation extends InstrumentationBase request.headers[header] + ) + ); + const spanOptions: SpanOptions = { kind: SpanKind.SERVER, attributes: spanAttributes, @@ -674,11 +684,6 @@ export class HttpInstrumentation extends InstrumentationBase request.headers[header] - ); - // After 'error', no further events other than 'close' should be emitted. let hasError = false; response.on('close', () => { @@ -899,8 +904,10 @@ export class HttpInstrumentation extends InstrumentationBase - response.getHeader(header) + span.setAttributes( + this._headerCapture.server.captureResponseHeaders(header => + response.getHeader(header) + ) ); span.setAttributes(attributes).setStatus({ diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index d760b620901..7fcaa1d0016 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -1097,9 +1097,9 @@ export function headerCapture( } return ( - span: Span, getHeader: (key: string) => undefined | string | string[] | number - ) => { + ): Attributes => { + const attributes: Attributes = {}; for (const capturedHeader of normalizedHeaders.keys()) { const value = getHeader(capturedHeader); @@ -1111,13 +1111,14 @@ export function headerCapture( const key = `http.${type}.header.${normalizedHeader}`; if (typeof value === 'string') { - span.setAttribute(key, [value]); + attributes[key] = [value]; } else if (Array.isArray(value)) { - span.setAttribute(key, value); + attributes[key] = value; } else { - span.setAttribute(key, [value]); + attributes[key] = [value]; } } + return attributes; }; } diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-sampler.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-sampler.test.ts new file mode 100644 index 00000000000..7a320ce73a3 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-sampler.test.ts @@ -0,0 +1,106 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + Attributes, + Context, + ContextManager, + Link, + SpanKind, + context, +} from '@opentelemetry/api'; +import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; +import { + InMemorySpanExporter, + Sampler, + SamplingDecision, + SamplingResult, + SimpleSpanProcessor, +} from '@opentelemetry/sdk-trace-base'; +import * as assert from 'assert'; +import { HttpInstrumentation } from '../../src/http'; +import { httpRequest } from '../utils/httpRequest'; + +class CapturingSampler implements Sampler { + public capturedAttributes: Attributes | undefined; + + shouldSample( + _context: Context, + _traceId: string, + _spanName: string, + _spanKind: SpanKind, + attributes: Attributes, + _links: Link[] + ): SamplingResult { + this.capturedAttributes = attributes; + return { decision: SamplingDecision.RECORD_AND_SAMPLED }; + } + + toString(): string { + return 'CapturingSampler'; + } +} + +const sampler = new CapturingSampler(); + +const instrumentation = new HttpInstrumentation({ + headersToSpanAttributes: { + server: { requestHeaders: ['x-custom-header'] }, + }, +}); +instrumentation.enable(); +instrumentation.disable(); + +import * as http from 'http'; +import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; + +const memoryExporter = new InMemorySpanExporter(); +const provider = new NodeTracerProvider({ + sampler, + spanProcessors: [new SimpleSpanProcessor(memoryExporter)], +}); +instrumentation.setTracerProvider(provider); + +describe('HttpInstrumentation sampler integration', () => { + const PORT = 22399; + let server: http.Server; + let contextManager: ContextManager; + + before(async () => { + instrumentation.enable(); + server = http.createServer((_req, res) => { + res.writeHead(200); + res.end(); + }); + await new Promise(resolve => server.listen(PORT, resolve)); + }); + + after(done => { + instrumentation.disable(); + server.close(done); + }); + + beforeEach(() => { + contextManager = new AsyncHooksContextManager(); + context.setGlobalContextManager(contextManager); + memoryExporter.reset(); + sampler.capturedAttributes = undefined; + }); + + afterEach(() => { + context.disable(); + }); + + it('provides http.request.header.* attributes to shouldSample', async () => { + await httpRequest.get(`http://localhost:${PORT}/`, { + headers: { 'x-custom-header': 'test-value' }, + }); + + assert.deepStrictEqual( + sampler.capturedAttributes?.['http.request.header.x_custom_header'], + ['test-value'] + ); + }); +}); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index 93f7e3914a7..d45f42c4170 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -579,66 +579,44 @@ describe('Utility', () => { }); describe('headers to span attributes capture', () => { - let span: Span; - let mock: sinon.SinonMock; - - beforeEach(() => { - span = { - setAttribute: () => undefined, - } as unknown as Span; - mock = sinon.mock(span); - }); - - it('should set attributes for request and response keys', () => { - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.origin', ['localhost']); - mock - .expects('setAttribute') - .calledWithExactly('http.response.header.cookie', ['token=123']); - - utils.headerCapture( + it('should capture attributes for request and response keys', () => { + const reqAttrs = utils.headerCapture( 'request', ['Origin'], SemconvStability.OLD - )(span, () => 'localhost'); - utils.headerCapture( + )(() => 'localhost'); + const resAttrs = utils.headerCapture( 'response', ['Cookie'], SemconvStability.OLD - )(span, () => 'token=123'); - mock.verify(); - }); + )(() => 'token=123'); - it('should set attributes for multiple values', () => { - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.origin', [ - 'localhost', - 'www.example.com', - ]); + assert.deepStrictEqual(reqAttrs, { + 'http.request.header.origin': ['localhost'], + }); + assert.deepStrictEqual(resAttrs, { + 'http.response.header.cookie': ['token=123'], + }); + }); - utils.headerCapture( + it('should capture attributes for multiple values', () => { + const attrs = utils.headerCapture( 'request', ['Origin'], SemconvStability.OLD - )(span, () => ['localhost', 'www.example.com']); - mock.verify(); - }); + )(() => ['localhost', 'www.example.com']); - it('sets attributes for multiple headers', () => { - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.origin', ['localhost']); - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.foo', [42]); + assert.deepStrictEqual(attrs, { + 'http.request.header.origin': ['localhost', 'www.example.com'], + }); + }); - utils.headerCapture( + it('should capture attributes for multiple headers', () => { + const attrs = utils.headerCapture( 'request', ['Origin', 'Foo'], SemconvStability.OLD - )(span, header => { + )(header => { if (header === 'origin') { return 'localhost'; } @@ -649,67 +627,63 @@ describe('Utility', () => { return undefined; }); - mock.verify(); + + assert.deepStrictEqual(attrs, { + 'http.request.header.origin': ['localhost'], + 'http.request.header.foo': [42], + }); }); it('should normalize header names (SemconvStability.OLD)', () => { - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.x_forwarded_for', ['foo']); - - utils.headerCapture( + const attrs = utils.headerCapture( 'request', ['X-Forwarded-For'], SemconvStability.OLD - )(span, () => 'foo'); - mock.verify(); + )(() => 'foo'); + assert.deepStrictEqual(attrs, { + 'http.request.header.x_forwarded_for': ['foo'], + }); }); it('should normalize header names (SemconvStability.STABLE)', () => { - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.x-forwarded-for', ['foo']); - - utils.headerCapture( + const attrs = utils.headerCapture( 'request', ['X-Forwarded-For'], SemconvStability.STABLE - )(span, () => 'foo'); - mock.verify(); + )(() => 'foo'); + assert.deepStrictEqual(attrs, { + 'http.request.header.x-forwarded-for': ['foo'], + }); }); it('should normalize header names (SemconvStability.DUPLICATE)', () => { // STABLE semconv wins over OLD when "DUPLICATE" is selected. - mock - .expects('setAttribute') - .calledWithExactly('http.request.header.x-forwarded-for', ['foo']); - - utils.headerCapture( + const attrs = utils.headerCapture( 'request', ['X-Forwarded-For'], SemconvStability.DUPLICATE - )(span, () => 'foo'); - mock.verify(); + )(() => 'foo'); + assert.deepStrictEqual(attrs, { + 'http.request.header.x-forwarded-for': ['foo'], + }); }); it('ignores non-existent headers', () => { - mock - .expects('setAttribute') - .once() - .calledWithExactly('http.request.header.origin', ['localhost']); - - utils.headerCapture( + const attrs = utils.headerCapture( 'request', ['Origin', 'Accept'], SemconvStability.OLD - )(span, header => { + )(header => { if (header === 'origin') { return 'localhost'; } return undefined; }); - mock.verify(); + + assert.deepStrictEqual(attrs, { + 'http.request.header.origin': ['localhost'], + }); }); });