From 7e5096e8f1c65da8e834ab0606814d0737f0835d Mon Sep 17 00:00:00 2001 From: David Luna Date: Tue, 1 Oct 2024 18:28:38 +0200 Subject: [PATCH 1/5] chore(exporter-zipkin): remove usages of Span constructor --- .../test/common/transform.test.ts | 108 ++++++++---------- 1 file changed, 48 insertions(+), 60 deletions(-) diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts index 223257c45a..04f646fe29 100644 --- a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts @@ -58,14 +58,14 @@ const spanContext: api.SpanContext = { describe('transform', () => { describe('toZipkinSpan', () => { it('should convert an OpenTelemetry span to a Zipkin span', () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER, - parentId - ); + { kind: api.SpanKind.SERVER }, + api.ROOT_CONTEXT, + ) as unknown as Span; + (span as any)['_spanContext'] = spanContext; + (span as any)['parentSpanId'] = parentId; + span.setAttributes({ key1: 'value1', key2: 'value2', @@ -112,13 +112,11 @@ describe('transform', () => { }); }); it("should skip parentSpanId if doesn't exist", () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER - ); + { kind: api.SpanKind.SERVER }, + api.ROOT_CONTEXT, + ) as unknown as Span; span.end(); const zipkinSpan = toZipkinSpan( @@ -163,13 +161,11 @@ describe('transform', () => { it(`should map OpenTelemetry SpanKind ${ api.SpanKind[item.ot] } to Zipkin ${item.zipkin}`, () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - item.ot - ); + { kind: item.ot }, + api.ROOT_CONTEXT, + ) as unknown as Span; span.end(); const zipkinSpan = toZipkinSpan( @@ -208,14 +204,12 @@ describe('transform', () => { describe('_toZipkinTags', () => { it('should convert OpenTelemetry attributes to Zipkin tags', () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER, - parentId - ); + { kind: api.SpanKind.SERVER }, + api.ROOT_CONTEXT, + ) as unknown as Span; + span.setAttributes({ key1: 'value1', key2: 'value2', @@ -239,21 +233,18 @@ describe('transform', () => { }); }); it('should map OpenTelemetry constructor attributes to a Zipkin tag', () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER, - parentId, - [], - undefined, - undefined, { - key1: 'value1', - key2: 'value2', - } - ); + kind: api.SpanKind.SERVER, + attributes: { + key1: 'value1', + key2: 'value2', + } + }, + api.ROOT_CONTEXT, + ) as unknown as Span; + const tags: zipkinTypes.Tags = _toZipkinTags( span, defaultStatusCodeTagName, @@ -273,14 +264,13 @@ describe('transform', () => { }); }); it('should map OpenTelemetry SpanStatus.code to a Zipkin tag', () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER, - parentId - ); + { + kind: api.SpanKind.SERVER, + }, + api.ROOT_CONTEXT, + ) as unknown as Span; const status: api.SpanStatus = { code: api.SpanStatusCode.ERROR, }; @@ -309,14 +299,13 @@ describe('transform', () => { }); }); it('should map OpenTelemetry SpanStatus.message to a Zipkin tag', () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER, - parentId - ); + { + kind: api.SpanKind.SERVER, + }, + api.ROOT_CONTEXT, + ) as unknown as Span; const status: api.SpanStatus = { code: api.SpanStatusCode.ERROR, message: 'my-message', @@ -350,14 +339,13 @@ describe('transform', () => { describe('_toZipkinAnnotations', () => { it('should convert OpenTelemetry events to Zipkin annotations', () => { - const span = new Span( - tracer, - api.ROOT_CONTEXT, + const span = tracer.startSpan( 'my-span', - spanContext, - api.SpanKind.SERVER, - parentId - ); + { + kind: api.SpanKind.SERVER, + }, + api.ROOT_CONTEXT, + ) as unknown as Span; span.addEvent('my-event1'); span.addEvent('my-event2', { key1: 'value1' }); From 2a52e0b43024c626150b33879d58ac70928d6204 Mon Sep 17 00:00:00 2001 From: David Luna Date: Tue, 1 Oct 2024 18:32:17 +0200 Subject: [PATCH 2/5] chore(exporter-zipkin): fix lint issues --- .../test/common/transform.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts index 04f646fe29..d14735a82c 100644 --- a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts @@ -61,7 +61,7 @@ describe('transform', () => { const span = tracer.startSpan( 'my-span', { kind: api.SpanKind.SERVER }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; (span as any)['_spanContext'] = spanContext; (span as any)['parentSpanId'] = parentId; @@ -115,7 +115,7 @@ describe('transform', () => { const span = tracer.startSpan( 'my-span', { kind: api.SpanKind.SERVER }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; span.end(); @@ -164,7 +164,7 @@ describe('transform', () => { const span = tracer.startSpan( 'my-span', { kind: item.ot }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; span.end(); @@ -207,7 +207,7 @@ describe('transform', () => { const span = tracer.startSpan( 'my-span', { kind: api.SpanKind.SERVER }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; span.setAttributes({ @@ -240,9 +240,9 @@ describe('transform', () => { attributes: { key1: 'value1', key2: 'value2', - } + }, }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; const tags: zipkinTypes.Tags = _toZipkinTags( @@ -269,7 +269,7 @@ describe('transform', () => { { kind: api.SpanKind.SERVER, }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; const status: api.SpanStatus = { code: api.SpanStatusCode.ERROR, @@ -304,7 +304,7 @@ describe('transform', () => { { kind: api.SpanKind.SERVER, }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; const status: api.SpanStatus = { code: api.SpanStatusCode.ERROR, @@ -344,7 +344,7 @@ describe('transform', () => { { kind: api.SpanKind.SERVER, }, - api.ROOT_CONTEXT, + api.ROOT_CONTEXT ) as unknown as Span; span.addEvent('my-event1'); span.addEvent('my-event2', { key1: 'value1' }); From ff4f950b596f5642d0c58ea6fce33f6aaaedf4b1 Mon Sep 17 00:00:00 2001 From: David Luna Date: Tue, 1 Oct 2024 18:50:35 +0200 Subject: [PATCH 3/5] chore(exporter-zipkin): update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dc782cfe6..60398f8bbc 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 +* chore(exporter-zipkin): remove usages of Span constructor [#5030](https://github.com/open-telemetry/opentelemetry-js/pull/5030) @david-luna ## 1.26.0 From 0ce64ef9bd12c48f94034cb03dbd7eaa0999370e Mon Sep 17 00:00:00 2001 From: David Luna Date: Wed, 2 Oct 2024 15:48:21 +0200 Subject: [PATCH 4/5] chore(exporter-zipkin): remove usage of traces in transform tests --- .../test/common/transform.test.ts | 213 +++++++++--------- 1 file changed, 106 insertions(+), 107 deletions(-) diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts index d14735a82c..9afb73094e 100644 --- a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts @@ -16,17 +16,16 @@ import * as api from '@opentelemetry/api'; import { + hrTime, hrTimeDuration, hrTimeToMicroseconds, + millisToHrTime, VERSION, } from '@opentelemetry/core'; -import { Resource } from '@opentelemetry/resources'; -import { BasicTracerProvider, Span } from '@opentelemetry/sdk-trace-base'; +import { IResource } from '@opentelemetry/resources'; +import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; -import { - SEMRESATTRS_SERVICE_NAME, - SEMRESATTRS_TELEMETRY_SDK_LANGUAGE, -} from '@opentelemetry/semantic-conventions'; +import { SEMRESATTRS_SERVICE_NAME } from '@opentelemetry/semantic-conventions'; import { defaultStatusCodeTagName, defaultStatusErrorTagName, @@ -35,43 +34,69 @@ import { _toZipkinTags, } from '../../src/transform'; import * as zipkinTypes from '../../src/types'; -const tracer = new BasicTracerProvider({ - resource: Resource.default().merge( - new Resource({ - [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test', - cost: '112.12', - service: 'ui', - version: '1', - }) - ), -}).getTracer('default'); - -const language = tracer.resource.attributes[SEMRESATTRS_TELEMETRY_SDK_LANGUAGE]; +const resource = { + attributes: { + [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test', + cost: '112.12', + service: 'ui', + version: '1', + 'telemetry.sdk.language': 'nodejs', + 'telemetry.sdk.name': 'opentelemetry', + 'telemetry.sdk.version': VERSION, + }, +} as unknown as IResource; const parentId = '5c1c63257de34c67'; const spanContext: api.SpanContext = { traceId: 'd4cda95b652f4a1592b449d5929fda1b', spanId: '6e0c63257de34c92', traceFlags: api.TraceFlags.SAMPLED, }; +const currentTime = Date.now(); +const durationMs = 10; +const startTime = hrTime(currentTime - durationMs); +const endTime = hrTime(currentTime); +const duration = millisToHrTime(durationMs); + +function getSpan(options: Partial): ReadableSpan { + const span = { + name: options.name || 'my-span', + kind: typeof options.kind === 'number' ? options.kind : api.SpanKind.SERVER, + startTime: options.startTime || startTime, + endTime: options.endTime || endTime, + duration: options.duration || duration, + spanContext: () => spanContext, + parentSpanId: options.parentSpanId || parentId, + attributes: options.attributes || {}, + events: options.events || [], + status: options.status || { code: api.SpanStatusCode.UNSET }, + resource, + } as ReadableSpan; + + // Expicit `undefined` properties fro options will be removed from the + // result span. + Object.keys(options).forEach(k => { + if (options[k as keyof ReadableSpan] === undefined) { + delete span[k as keyof ReadableSpan]; + } + }); + + return span; +} describe('transform', () => { describe('toZipkinSpan', () => { it('should convert an OpenTelemetry span to a Zipkin span', () => { - const span = tracer.startSpan( - 'my-span', - { kind: api.SpanKind.SERVER }, - api.ROOT_CONTEXT - ) as unknown as Span; - (span as any)['_spanContext'] = spanContext; - (span as any)['parentSpanId'] = parentId; - - span.setAttributes({ - key1: 'value1', - key2: 'value2', + const span = getSpan({ + attributes: { key1: 'value1', key2: 'value2' }, + events: [ + { + name: 'my-event', + time: hrTime(Date.now() + 5), + attributes: { key3: 'value 3' }, + }, + ], }); - span.addEvent('my-event', { key3: 'value3' }); - span.end(); const zipkinSpan = toZipkinSpan( span, @@ -103,7 +128,7 @@ describe('transform', () => { cost: '112.12', service: 'ui', version: '1', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, }, @@ -112,12 +137,9 @@ describe('transform', () => { }); }); it("should skip parentSpanId if doesn't exist", () => { - const span = tracer.startSpan( - 'my-span', - { kind: api.SpanKind.SERVER }, - api.ROOT_CONTEXT - ) as unknown as Span; - span.end(); + const span = getSpan({ + parentSpanId: undefined, + }); const zipkinSpan = toZipkinSpan( span, @@ -142,7 +164,7 @@ describe('transform', () => { cost: '112.12', service: 'ui', version: '1', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, }, @@ -161,13 +183,10 @@ describe('transform', () => { it(`should map OpenTelemetry SpanKind ${ api.SpanKind[item.ot] } to Zipkin ${item.zipkin}`, () => { - const span = tracer.startSpan( - 'my-span', - { kind: item.ot }, - api.ROOT_CONTEXT - ) as unknown as Span; - span.end(); - + const span = getSpan({ + kind: item.ot, + parentSpanId: undefined, + }); const zipkinSpan = toZipkinSpan( span, 'my-service', @@ -191,7 +210,7 @@ describe('transform', () => { cost: '112.12', service: 'ui', version: '1', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, }, @@ -204,15 +223,12 @@ describe('transform', () => { describe('_toZipkinTags', () => { it('should convert OpenTelemetry attributes to Zipkin tags', () => { - const span = tracer.startSpan( - 'my-span', - { kind: api.SpanKind.SERVER }, - api.ROOT_CONTEXT - ) as unknown as Span; - - span.setAttributes({ - key1: 'value1', - key2: 'value2', + const span = getSpan({ + parentSpanId: undefined, + attributes: { + key1: 'value1', + key2: 'value2', + }, }); const tags: zipkinTypes.Tags = _toZipkinTags( span, @@ -224,7 +240,7 @@ describe('transform', () => { key1: 'value1', key2: 'value2', [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, cost: '112.12', @@ -233,17 +249,13 @@ describe('transform', () => { }); }); it('should map OpenTelemetry constructor attributes to a Zipkin tag', () => { - const span = tracer.startSpan( - 'my-span', - { - kind: api.SpanKind.SERVER, - attributes: { - key1: 'value1', - key2: 'value2', - }, + const span = getSpan({ + parentSpanId: undefined, + attributes: { + key1: 'value1', + key2: 'value2', }, - api.ROOT_CONTEXT - ) as unknown as Span; + }); const tags: zipkinTypes.Tags = _toZipkinTags( span, @@ -255,7 +267,7 @@ describe('transform', () => { key1: 'value1', key2: 'value2', [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, cost: '112.12', @@ -264,20 +276,13 @@ describe('transform', () => { }); }); it('should map OpenTelemetry SpanStatus.code to a Zipkin tag', () => { - const span = tracer.startSpan( - 'my-span', - { - kind: api.SpanKind.SERVER, + const span = getSpan({ + parentSpanId: undefined, + attributes: { + key1: 'value1', + key2: 'value2', }, - api.ROOT_CONTEXT - ) as unknown as Span; - const status: api.SpanStatus = { - code: api.SpanStatusCode.ERROR, - }; - span.setStatus(status); - span.setAttributes({ - key1: 'value1', - key2: 'value2', + status: { code: api.SpanStatusCode.ERROR }, }); const tags: zipkinTypes.Tags = _toZipkinTags( span, @@ -290,7 +295,7 @@ describe('transform', () => { key2: 'value2', [defaultStatusCodeTagName]: 'ERROR', [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, cost: '112.12', @@ -299,21 +304,13 @@ describe('transform', () => { }); }); it('should map OpenTelemetry SpanStatus.message to a Zipkin tag', () => { - const span = tracer.startSpan( - 'my-span', - { - kind: api.SpanKind.SERVER, + const span = getSpan({ + parentSpanId: undefined, + attributes: { + key1: 'value1', + key2: 'value2', }, - api.ROOT_CONTEXT - ) as unknown as Span; - const status: api.SpanStatus = { - code: api.SpanStatusCode.ERROR, - message: 'my-message', - }; - span.setStatus(status); - span.setAttributes({ - key1: 'value1', - key2: 'value2', + status: { code: api.SpanStatusCode.ERROR, message: 'my-message' }, }); const tags: zipkinTypes.Tags = _toZipkinTags( span, @@ -325,9 +322,9 @@ describe('transform', () => { key1: 'value1', key2: 'value2', [defaultStatusCodeTagName]: 'ERROR', - [defaultStatusErrorTagName]: status.message, + [defaultStatusErrorTagName]: 'my-message', [SEMRESATTRS_SERVICE_NAME]: 'zipkin-test', - 'telemetry.sdk.language': language, + 'telemetry.sdk.language': 'nodejs', 'telemetry.sdk.name': 'opentelemetry', 'telemetry.sdk.version': VERSION, cost: '112.12', @@ -339,15 +336,17 @@ describe('transform', () => { describe('_toZipkinAnnotations', () => { it('should convert OpenTelemetry events to Zipkin annotations', () => { - const span = tracer.startSpan( - 'my-span', - { - kind: api.SpanKind.SERVER, - }, - api.ROOT_CONTEXT - ) as unknown as Span; - span.addEvent('my-event1'); - span.addEvent('my-event2', { key1: 'value1' }); + const span = getSpan({ + parentSpanId: undefined, + events: [ + { name: 'my-event1', time: hrTime(Date.now()) }, + { + name: 'my-event2', + time: hrTime(Date.now()), + attributes: { key1: 'value1' }, + }, + ], + }); const annotations = _toZipkinAnnotations(span.events); assert.deepStrictEqual(annotations, [ From 69e1d0b54e5498df8c4c43d5b88b7cffd3f37ea0 Mon Sep 17 00:00:00 2001 From: David Luna Date: Thu, 3 Oct 2024 09:00:50 +0200 Subject: [PATCH 5/5] chore(exporter-zipkin): fix typo --- .../opentelemetry-exporter-zipkin/test/common/transform.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts index 9afb73094e..0c78dbbcf8 100644 --- a/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/common/transform.test.ts @@ -73,7 +73,7 @@ function getSpan(options: Partial): ReadableSpan { resource, } as ReadableSpan; - // Expicit `undefined` properties fro options will be removed from the + // Expicit `undefined` properties in options will be removed from the // result span. Object.keys(options).forEach(k => { if (options[k as keyof ReadableSpan] === undefined) {