From 7baa493f50fd0a967d169ddfa2b38137b8a3b330 Mon Sep 17 00:00:00 2001 From: David Luna Date: Fri, 4 Oct 2024 10:00:03 +0200 Subject: [PATCH] test(instr-http): remove usages of `new Span` (#5035) --- CHANGELOG.md | 1 + .../test/functionals/utils.test.ts | 120 +++++++++--------- 2 files changed, 64 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dc782cfe6..1e42a90f7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se [#4992](https://github.com/open-telemetry/opentelemetry-js/pull/4992) * refactor(sdk-metrics): replace `MetricsAttributes` with `Attributes` [#5021](https://github.com/open-telemetry/opentelemetry-js/pull/5021) @david-luna * refactor(instrumentation-http): replace `SpanAttributes` and `MetricsAttributes` with `Attributes` [#5023](https://github.com/open-telemetry/opentelemetry-js/pull/5023) @david-luna +* test(instrumentation-http): remove usages of `new Span` in tests [#5035](https://github.com/open-telemetry/opentelemetry-js/pull/5035) @david-luna ## 1.26.0 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 c731a296d9..c091529c9f 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -16,12 +16,10 @@ import { Attributes, SpanStatusCode, - ROOT_CONTEXT, SpanKind, - TraceFlags, context, + Span, } from '@opentelemetry/api'; -import { BasicTracerProvider, Span } from '@opentelemetry/sdk-trace-base'; import { SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, @@ -257,26 +255,28 @@ describe('Utility', () => { describe('setSpanWithError()', () => { it('should have error attributes', () => { const errorMessage = 'test error'; - const span = new Span( - new BasicTracerProvider().getTracer('default'), - ROOT_CONTEXT, - 'test', - { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, - SpanKind.INTERNAL - ); - utils.setSpanWithError( - span, - new Error(errorMessage), - SemconvStability.OLD - ); - const attributes = span.attributes; - assert.strictEqual( - attributes[AttributeNames.HTTP_ERROR_MESSAGE], - errorMessage - ); - assert.strictEqual(span.events.length, 1); - assert.strictEqual(span.events[0].name, 'exception'); - assert.ok(attributes[AttributeNames.HTTP_ERROR_NAME]); + const error = new Error(errorMessage); + const span = { + setAttribute: () => undefined, + setStatus: () => undefined, + recordException: () => undefined, + } as unknown as Span; + const mock = sinon.mock(span); + + mock + .expects('setAttribute') + .calledWithExactly(AttributeNames.HTTP_ERROR_NAME, 'error'); + mock + .expects('setAttribute') + .calledWithExactly(AttributeNames.HTTP_ERROR_MESSAGE, errorMessage); + mock.expects('setStatus').calledWithExactly({ + code: SpanStatusCode.ERROR, + message: errorMessage, + }); + mock.expects('recordException').calledWithExactly(error); + + utils.setSpanWithError(span, error, SemconvStability.OLD); + mock.verify(); }); }); @@ -537,40 +537,51 @@ describe('Utility', () => { describe('headers to span attributes capture', () => { let span: Span; + let mock: sinon.SinonMock; beforeEach(() => { - span = new Span( - new BasicTracerProvider().getTracer('default'), - ROOT_CONTEXT, - 'test', - { spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED }, - SpanKind.INTERNAL - ); + 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('request', ['Origin'])(span, () => 'localhost'); utils.headerCapture('response', ['Cookie'])(span, () => 'token=123'); - assert.deepStrictEqual(span.attributes['http.request.header.origin'], [ - 'localhost', - ]); - assert.deepStrictEqual(span.attributes['http.response.header.cookie'], [ - 'token=123', - ]); + mock.verify(); }); it('should set attributes for multiple values', () => { + mock + .expects('setAttribute') + .calledWithExactly('http.request.header.origin', [ + 'localhost', + 'www.example.com', + ]); + utils.headerCapture('request', ['Origin'])(span, () => [ 'localhost', 'www.example.com', ]); - assert.deepStrictEqual(span.attributes['http.request.header.origin'], [ - 'localhost', - 'www.example.com', - ]); + mock.verify(); }); it('sets attributes for multiple headers', () => { + mock + .expects('setAttribute') + .calledWithExactly('http.request.header.origin', ['localhost']); + mock + .expects('setAttribute') + .calledWithExactly('http.request.header.foo', [42]); + utils.headerCapture('request', ['Origin', 'Foo'])(span, header => { if (header === 'origin') { return 'localhost'; @@ -582,22 +593,24 @@ describe('Utility', () => { return undefined; }); - - assert.deepStrictEqual(span.attributes['http.request.header.origin'], [ - 'localhost', - ]); - assert.deepStrictEqual(span.attributes['http.request.header.foo'], [42]); + mock.verify(); }); it('should normalize header names', () => { + mock + .expects('setAttribute') + .calledWithExactly('http.request.header.x_forwarded_for', ['foo']); + utils.headerCapture('request', ['X-Forwarded-For'])(span, () => 'foo'); - assert.deepStrictEqual( - span.attributes['http.request.header.x_forwarded_for'], - ['foo'] - ); + mock.verify(); }); it('ignores non-existent headers', () => { + mock + .expects('setAttribute') + .once() + .calledWithExactly('http.request.header.origin', ['localhost']); + utils.headerCapture('request', ['Origin', 'Accept'])(span, header => { if (header === 'origin') { return 'localhost'; @@ -605,14 +618,7 @@ describe('Utility', () => { return undefined; }); - - assert.deepStrictEqual(span.attributes['http.request.header.origin'], [ - 'localhost', - ]); - assert.deepStrictEqual( - span.attributes['http.request.header.accept'], - undefined - ); + mock.verify(); }); });