From 46ce5357528694c5fefa07680eef883257538e21 Mon Sep 17 00:00:00 2001 From: David W Date: Mon, 6 Jul 2020 10:56:00 -0400 Subject: [PATCH 1/7] feat: Collector Metric Exporter[2/x] Create CollectorMetricExporterBase (#1258) Co-authored-by: Daniel Dyla --- .../package.json | 1 + .../src/CollectorMetricExporterBase.ts | 119 ++++++++++ .../src/CollectorTraceExporterBase.ts | 30 ++- .../platform/node/CollectorTraceExporter.ts | 4 +- .../src/platform/node/protos | 2 +- .../src/types.ts | 21 +- .../browser/CollectorTraceExporter.test.ts | 6 +- .../common/CollectorMetricExporter.test.ts | 220 ++++++++++++++++++ .../common/CollectorTraceExporter.test.ts | 59 ++++- .../test/helper.ts | 44 +++- .../test/node/CollectorTraceExporter.test.ts | 4 +- 11 files changed, 469 insertions(+), 41 deletions(-) create mode 100644 packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts create mode 100644 packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts diff --git a/packages/opentelemetry-exporter-collector/package.json b/packages/opentelemetry-exporter-collector/package.json index 8ca9a93c69a..d25876c93b8 100644 --- a/packages/opentelemetry-exporter-collector/package.json +++ b/packages/opentelemetry-exporter-collector/package.json @@ -87,6 +87,7 @@ "@opentelemetry/api": "^0.9.0", "@opentelemetry/core": "^0.9.0", "@opentelemetry/resources": "^0.9.0", + "@opentelemetry/metrics": "^0.9.0", "@opentelemetry/tracing": "^0.9.0", "google-protobuf": "^3.11.4", "grpc": "^1.24.2" diff --git a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts new file mode 100644 index 00000000000..7580e5f8a73 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts @@ -0,0 +1,119 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { MetricExporter, MetricRecord } from '@opentelemetry/metrics'; +import { Attributes, Logger } from '@opentelemetry/api'; +import { CollectorExporterConfigBase } from './types'; +import { NoopLogger, ExportResult } from '@opentelemetry/core'; +import * as collectorTypes from './types'; + +const DEFAULT_SERVICE_NAME = 'collector-metric-exporter'; + +/** + * Collector Metric Exporter abstract base class + */ +export abstract class CollectorMetricExporterBase< + T extends CollectorExporterConfigBase +> implements MetricExporter { + public readonly serviceName: string; + public readonly url: string; + public readonly logger: Logger; + public readonly hostname: string | undefined; + public readonly attributes?: Attributes; + protected readonly _startTime = new Date().getTime() * 1000000; + private _isShutdown: boolean = false; + + /** + * @param config + */ + constructor(config: T = {} as T) { + this.logger = config.logger || new NoopLogger(); + this.serviceName = config.serviceName || DEFAULT_SERVICE_NAME; + this.url = this.getDefaultUrl(config.url); + this.attributes = config.attributes; + if (typeof config.hostname === 'string') { + this.hostname = config.hostname; + } + this.onInit(); + } + + /** + * Export metrics + * @param metrics + * @param resultCallback + */ + export( + metrics: MetricRecord[], + resultCallback: (result: ExportResult) => void + ) { + if (this._isShutdown) { + resultCallback(ExportResult.FAILED_NOT_RETRYABLE); + return; + } + + this._exportMetrics(metrics) + .then(() => { + resultCallback(ExportResult.SUCCESS); + }) + .catch((error: collectorTypes.ExportServiceError) => { + if (error.message) { + this.logger.error(error.message); + } + if (error.code && error.code < 500) { + resultCallback(ExportResult.FAILED_NOT_RETRYABLE); + } else { + resultCallback(ExportResult.FAILED_RETRYABLE); + } + }); + } + + private _exportMetrics(metrics: MetricRecord[]): Promise { + return new Promise((resolve, reject) => { + try { + this.logger.debug('metrics to be sent', metrics); + // Send metrics to [opentelemetry collector]{@link https://github.com/open-telemetry/opentelemetry-collector} + // it will use the appropriate transport layer automatically depends on platform + this.sendMetrics(metrics, resolve, reject); + } catch (e) { + reject(e); + } + }); + } + + /** + * Shutdown the exporter. + */ + shutdown(): void { + if (this._isShutdown) { + this.logger.debug('shutdown already started'); + return; + } + this._isShutdown = true; + this.logger.debug('shutdown started'); + + // platform dependent + this.onShutdown(); + } + + abstract getDefaultUrl(url: string | undefined): string; + abstract onInit(): void; + abstract onShutdown(): void; + abstract sendMetrics( + metrics: MetricRecord[], + onSuccess: () => void, + onError: (error: collectorTypes.CollectorExporterError) => void + ): void; +} diff --git a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts index 93ab2330db3..4695c439940 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts @@ -18,9 +18,9 @@ import { Attributes, Logger } from '@opentelemetry/api'; import { ExportResult, NoopLogger } from '@opentelemetry/core'; import { ReadableSpan, SpanExporter } from '@opentelemetry/tracing'; import { - opentelemetryProto, CollectorExporterError, CollectorExporterConfigBase, + ExportServiceError, } from './types'; const DEFAULT_SERVICE_NAME = 'collector-exporter'; @@ -34,7 +34,7 @@ export abstract class CollectorTraceExporterBase< public readonly serviceName: string; public readonly url: string; public readonly logger: Logger; - public readonly hostName: string | undefined; + public readonly hostname: string | undefined; public readonly attributes?: Attributes; private _isShutdown: boolean = false; @@ -44,8 +44,8 @@ export abstract class CollectorTraceExporterBase< constructor(config: T = {} as T) { this.serviceName = config.serviceName || DEFAULT_SERVICE_NAME; this.url = this.getDefaultUrl(config.url); - if (typeof config.hostName === 'string') { - this.hostName = config.hostName; + if (typeof config.hostname === 'string') { + this.hostname = config.hostname; } this.attributes = config.attributes; @@ -76,20 +76,16 @@ export abstract class CollectorTraceExporterBase< .then(() => { resultCallback(ExportResult.SUCCESS); }) - .catch( - ( - error: opentelemetryProto.collector.trace.v1.ExportTraceServiceError - ) => { - if (error.message) { - this.logger.error(error.message); - } - if (error.code && error.code < 500) { - resultCallback(ExportResult.FAILED_NOT_RETRYABLE); - } else { - resultCallback(ExportResult.FAILED_RETRYABLE); - } + .catch((error: ExportServiceError) => { + if (error.message) { + this.logger.error(error.message); } - ); + if (error.code && error.code < 500) { + resultCallback(ExportResult.FAILED_NOT_RETRYABLE); + } else { + resultCallback(ExportResult.FAILED_RETRYABLE); + } + }); } private _exportSpans(spans: ReadableSpan[]): Promise { diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts index 38953899810..c5d88ad9b52 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts @@ -112,9 +112,7 @@ export class CollectorTraceExporter extends CollectorTraceExporterBase< this.traceServiceClient.export( exportTraceServiceRequest, this.metadata, - ( - err: collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceError - ) => { + (err: collectorTypes.ExportServiceError) => { if (err) { this.logger.error( 'exportTraceServiceRequest', diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/protos b/packages/opentelemetry-exporter-collector/src/platform/node/protos index e6c3c4a74d5..b5468856918 160000 --- a/packages/opentelemetry-exporter-collector/src/platform/node/protos +++ b/packages/opentelemetry-exporter-collector/src/platform/node/protos @@ -1 +1 @@ -Subproject commit e6c3c4a74d57f870a0d781bada02cb2b2c497d14 +Subproject commit b54688569186e0b862bf7462a983ccf2c50c0547 diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index aca9f6dfb34..627cb736a84 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -32,14 +32,6 @@ export namespace opentelemetryProto { export interface ExportTraceServiceRequest { resourceSpans: opentelemetryProto.trace.v1.ResourceSpans[]; } - - export interface ExportTraceServiceError { - code: number; - details: string; - metadata: { [key: string]: unknown }; - message: string; - stack: string; - } } } @@ -171,11 +163,22 @@ export interface CollectorExporterError { stack?: string; } +/** + * Interface for handling export service errors + */ +export interface ExportServiceError { + code: number; + details: string; + metadata: { [key: string]: unknown }; + message: string; + stack: string; +} + /** * Collector Exporter base config */ export interface CollectorExporterConfigBase { - hostName?: string; + hostname?: string; logger?: Logger; serviceName?: string; attributes?: Attributes; diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts index 58a778547bb..2386b63ab5e 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts @@ -30,7 +30,7 @@ import { } from '../helper'; const sendBeacon = navigator.sendBeacon; -describe('CollectorExporter - web', () => { +describe('CollectorTraceExporter - web', () => { let collectorTraceExporter: CollectorTraceExporter; let collectorExporterConfig: collectorTypes.CollectorExporterConfigBrowser; let spyOpen: any; @@ -56,7 +56,7 @@ describe('CollectorExporter - web', () => { describe('export', () => { beforeEach(() => { collectorExporterConfig = { - hostName: 'foo', + hostname: 'foo', logger: new NoopLogger(), serviceName: 'bar', attributes: {}, @@ -326,7 +326,7 @@ describe('CollectorExporter - web', () => { }); }); -describe('CollectorExporter - browser (getDefaultUrl)', () => { +describe('CollectorTraceExporter - browser (getDefaultUrl)', () => { it('should default to v1/trace', done => { const collectorExporter = new CollectorTraceExporter({}); setTimeout(() => { diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts new file mode 100644 index 00000000000..73e9ddc40eb --- /dev/null +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -0,0 +1,220 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ExportResult, NoopLogger } from '@opentelemetry/core'; +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { CollectorMetricExporterBase } from '../../src/CollectorMetricExporterBase'; +import { CollectorExporterConfigBase } from '../../src/types'; +import { MetricRecord } from '@opentelemetry/metrics'; +import { mockCounter, mockObserver } from '../helper'; + +type CollectorExporterConfig = CollectorExporterConfigBase; +class CollectorMetricExporter extends CollectorMetricExporterBase< + CollectorExporterConfig +> { + onInit() {} + onShutdown() {} + sendMetrics() {} + getDefaultUrl(url: string) { + return url || ''; + } +} + +describe('CollectorMetricExporter - common', () => { + let collectorExporter: CollectorMetricExporter; + let collectorExporterConfig: CollectorExporterConfig; + let metrics: MetricRecord[]; + describe('constructor', () => { + let onInitSpy: any; + + beforeEach(() => { + onInitSpy = sinon.stub(CollectorMetricExporter.prototype, 'onInit'); + collectorExporterConfig = { + hostname: 'foo', + logger: new NoopLogger(), + serviceName: 'bar', + attributes: {}, + url: 'http://foo.bar.com', + }; + collectorExporter = new CollectorMetricExporter(collectorExporterConfig); + metrics = []; + metrics.push(Object.assign({}, mockCounter)); + metrics.push(Object.assign({}, mockObserver)); + }); + + afterEach(() => { + onInitSpy.restore(); + }); + + it('should create an instance', () => { + assert.ok(typeof collectorExporter !== 'undefined'); + }); + + it('should call onInit', () => { + assert.strictEqual(onInitSpy.callCount, 1); + }); + + describe('when config contains certain params', () => { + it('should set hostname', () => { + assert.strictEqual(collectorExporter.hostname, 'foo'); + }); + + it('should set serviceName', () => { + assert.strictEqual(collectorExporter.serviceName, 'bar'); + }); + + it('should set url', () => { + assert.strictEqual(collectorExporter.url, 'http://foo.bar.com'); + }); + + it('should set logger', () => { + assert.ok(collectorExporter.logger === collectorExporterConfig.logger); + }); + }); + + describe('when config is missing certain params', () => { + beforeEach(() => { + collectorExporter = new CollectorMetricExporter(); + }); + + it('should set default serviceName', () => { + assert.strictEqual( + collectorExporter.serviceName, + 'collector-metric-exporter' + ); + }); + + it('should set default logger', () => { + assert.ok(collectorExporter.logger instanceof NoopLogger); + }); + }); + }); + + describe('export', () => { + let spySend: any; + beforeEach(() => { + spySend = sinon.stub(CollectorMetricExporter.prototype, 'sendMetrics'); + collectorExporter = new CollectorMetricExporter(collectorExporterConfig); + }); + afterEach(() => { + spySend.restore(); + }); + + it('should export metrics as collectorTypes.Metrics', done => { + collectorExporter.export(metrics, () => {}); + setTimeout(() => { + const metric1 = spySend.args[0][0][0] as MetricRecord; + assert.deepStrictEqual(metrics[0], metric1); + const metric2 = spySend.args[0][0][1] as MetricRecord; + assert.deepStrictEqual(metrics[1], metric2); + done(); + }); + assert.strictEqual(spySend.callCount, 1); + }); + + describe('when exporter is shutdown', () => { + it('should not export anything but return callback with code "FailedNotRetryable"', () => { + collectorExporter.shutdown(); + spySend.resetHistory(); + + const callbackSpy = sinon.spy(); + collectorExporter.export(metrics, callbackSpy); + const returnCode = callbackSpy.args[0][0]; + assert.strictEqual( + returnCode, + ExportResult.FAILED_NOT_RETRYABLE, + 'return value is wrong' + ); + assert.strictEqual(spySend.callCount, 0, 'should not call send'); + }); + }); + describe('when an error occurs', () => { + it('should return a Not Retryable Error', done => { + spySend.throws({ + code: 100, + details: 'Test error', + metadata: {}, + message: 'Non-retryable', + stack: 'Stack', + }); + const callbackSpy = sinon.spy(); + collectorExporter.export(metrics, callbackSpy); + setTimeout(() => { + const returnCode = callbackSpy.args[0][0]; + assert.strictEqual( + returnCode, + ExportResult.FAILED_NOT_RETRYABLE, + 'return value is wrong' + ); + assert.strictEqual(spySend.callCount, 1, 'should call send'); + done(); + }, 500); + }); + + it('should return a Retryable Error', done => { + spySend.throws({ + code: 600, + details: 'Test error', + metadata: {}, + message: 'Retryable', + stack: 'Stack', + }); + const callbackSpy = sinon.spy(); + collectorExporter.export(metrics, callbackSpy); + setTimeout(() => { + const returnCode = callbackSpy.args[0][0]; + assert.strictEqual( + returnCode, + ExportResult.FAILED_RETRYABLE, + 'return value is wrong' + ); + assert.strictEqual(spySend.callCount, 1, 'should call send'); + done(); + }, 500); + }); + }); + }); + + describe('shutdown', () => { + let onShutdownSpy: any; + beforeEach(() => { + onShutdownSpy = sinon.stub( + CollectorMetricExporter.prototype, + 'onShutdown' + ); + collectorExporterConfig = { + hostname: 'foo', + logger: new NoopLogger(), + serviceName: 'bar', + attributes: {}, + url: 'http://foo.bar.com', + }; + collectorExporter = new CollectorMetricExporter(collectorExporterConfig); + }); + afterEach(() => { + onShutdownSpy.restore(); + }); + + it('should call onShutdown', done => { + collectorExporter.shutdown(); + setTimeout(() => { + assert.equal(onShutdownSpy.callCount, 1); + done(); + }); + }); + }); +}); diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index 71ccee1c1c2..504aff43385 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts @@ -34,7 +34,7 @@ class CollectorTraceExporter extends CollectorTraceExporterBase< } } -describe('CollectorExporter - common', () => { +describe('CollectorTraceExporter - common', () => { let collectorExporter: CollectorTraceExporter; let collectorExporterConfig: CollectorExporterConfig; @@ -44,7 +44,7 @@ describe('CollectorExporter - common', () => { beforeEach(() => { onInitSpy = sinon.stub(CollectorTraceExporter.prototype, 'onInit'); collectorExporterConfig = { - hostName: 'foo', + hostname: 'foo', logger: new NoopLogger(), serviceName: 'bar', attributes: {}, @@ -66,8 +66,8 @@ describe('CollectorExporter - common', () => { }); describe('when config contains certain params', () => { - it('should set hostName', () => { - assert.strictEqual(collectorExporter.hostName, 'foo'); + it('should set hostname', () => { + assert.strictEqual(collectorExporter.hostname, 'foo'); }); it('should set serviceName', () => { @@ -140,6 +140,55 @@ describe('CollectorExporter - common', () => { assert.strictEqual(spySend.callCount, 0, 'should not call send'); }); }); + describe('when an error occurs', () => { + it('should return a Not Retryable Error', done => { + const spans: ReadableSpan[] = []; + spans.push(Object.assign({}, mockedReadableSpan)); + spySend.throws({ + code: 100, + details: 'Test error', + metadata: {}, + message: 'Non-retryable', + stack: 'Stack', + }); + const callbackSpy = sinon.spy(); + collectorExporter.export(spans, callbackSpy); + setTimeout(() => { + const returnCode = callbackSpy.args[0][0]; + assert.strictEqual( + returnCode, + ExportResult.FAILED_NOT_RETRYABLE, + 'return value is wrong' + ); + assert.strictEqual(spySend.callCount, 1, 'should call send'); + done(); + }, 500); + }); + + it('should return a Retryable Error', done => { + const spans: ReadableSpan[] = []; + spans.push(Object.assign({}, mockedReadableSpan)); + spySend.throws({ + code: 600, + details: 'Test error', + metadata: {}, + message: 'Retryable', + stack: 'Stack', + }); + const callbackSpy = sinon.spy(); + collectorExporter.export(spans, callbackSpy); + setTimeout(() => { + const returnCode = callbackSpy.args[0][0]; + assert.strictEqual( + returnCode, + ExportResult.FAILED_RETRYABLE, + 'return value is wrong' + ); + assert.strictEqual(spySend.callCount, 1, 'should call send'); + done(); + }, 500); + }); + }); }); describe('shutdown', () => { @@ -150,7 +199,7 @@ describe('CollectorExporter - common', () => { 'onShutdown' ); collectorExporterConfig = { - hostName: 'foo', + hostname: 'foo', logger: new NoopLogger(), serviceName: 'bar', attributes: {}, diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index 056d95d2a7b..4e57a6edbbf 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { TraceFlags } from '@opentelemetry/api'; +import { TraceFlags, ValueType } from '@opentelemetry/api'; import { ReadableSpan } from '@opentelemetry/tracing'; import { Resource } from '@opentelemetry/resources'; import * as assert from 'assert'; @@ -22,6 +22,12 @@ import { opentelemetryProto } from '../src/types'; import * as collectorTypes from '../src/types'; import { InstrumentationLibrary } from '@opentelemetry/core'; import * as grpc from 'grpc'; +import { + MetricRecord, + MetricKind, + SumAggregator, + LastValueAggregator, +} from '@opentelemetry/metrics'; if (typeof Buffer === 'undefined') { (window as any).Buffer = { @@ -52,6 +58,42 @@ const traceIdArr = [ const spanIdArr = [94, 16, 114, 97, 246, 79, 165, 62]; const parentIdArr = [120, 168, 145, 80, 152, 134, 67, 136]; +export const mockCounter: MetricRecord = { + descriptor: { + name: 'test-counter', + description: 'sample counter description', + unit: '1', + metricKind: MetricKind.COUNTER, + valueType: ValueType.INT, + }, + labels: {}, + aggregator: new SumAggregator(), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, +}; + +export const mockObserver: MetricRecord = { + descriptor: { + name: 'test-observer', + description: 'sample observer description', + unit: '2', + metricKind: MetricKind.VALUE_OBSERVER, + valueType: ValueType.DOUBLE, + }, + labels: {}, + aggregator: new LastValueAggregator(), + resource: new Resource({ + service: 'ui', + version: 1, + cost: 112.12, + }), + instrumentationLibrary: { name: 'default', version: '0.0.1' }, +}; + const traceIdBase64 = 'HxAI3I4nDoXECg18OTmyeA=='; const spanIdBase64 = 'XhByYfZPpT4='; const parentIdBase64 = 'eKiRUJiGQ4g='; diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts index 537bd8eee49..514125e8822 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts @@ -50,7 +50,7 @@ const metadata = new grpc.Metadata(); metadata.set('k', 'v'); const testCollectorExporter = (params: TestParams) => - describe(`CollectorExporter - node ${ + describe(`CollectorTraceExporter - node ${ params.useTLS ? 'with' : 'without' } TLS, ${params.metadata ? 'with' : 'without'} metadata`, () => { let collectorExporter: CollectorTraceExporter; @@ -172,7 +172,7 @@ const testCollectorExporter = (params: TestParams) => }); }); -describe('CollectorExporter - node (getDefaultUrl)', () => { +describe('CollectorTraceExporter - node (getDefaultUrl)', () => { it('should default to localhost', done => { const collectorExporter = new CollectorTraceExporter({}); setTimeout(() => { From 413edb5369536b9be960049edac3b2be96953649 Mon Sep 17 00:00:00 2001 From: David W Date: Mon, 6 Jul 2020 14:33:09 -0400 Subject: [PATCH 2/7] feat: Add types for Collector Metric Exporter (#1271) --- .../browser/CollectorTraceExporter.ts | 2 +- .../src/platform/browser/types.ts | 25 ++++ .../platform/node/CollectorTraceExporter.ts | 9 +- .../src/platform/node/types.ts | 14 ++- .../src/types.ts | 112 +++++++++++++++--- .../browser/CollectorTraceExporter.test.ts | 3 +- 6 files changed, 140 insertions(+), 25 deletions(-) create mode 100644 packages/opentelemetry-exporter-collector/src/platform/browser/types.ts diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts index b6d0e66f841..39da94d1d0f 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts @@ -17,7 +17,7 @@ import { CollectorTraceExporterBase } from '../../CollectorTraceExporterBase'; import { ReadableSpan } from '@opentelemetry/tracing'; import { toCollectorExportTraceServiceRequest } from '../../transform'; -import { CollectorExporterConfigBrowser } from '../../types'; +import { CollectorExporterConfigBrowser } from './types'; import * as collectorTypes from '../../types'; const DEFAULT_COLLECTOR_URL = 'http://localhost:55680/v1/trace'; diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/types.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/types.ts new file mode 100644 index 00000000000..dffdf01c26b --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/types.ts @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { CollectorExporterConfigBase } from '../../types'; + +/** + * Collector Exporter Config for Web + */ +export interface CollectorExporterConfigBrowser + extends CollectorExporterConfigBase { + headers?: { [key: string]: string }; +} diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts index c5d88ad9b52..8750315de5d 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts @@ -21,12 +21,13 @@ import * as collectorTypes from '../../types'; import { ReadableSpan } from '@opentelemetry/tracing'; import { CollectorTraceExporterBase } from '../../CollectorTraceExporterBase'; +import { CollectorExporterError } from '../../types'; +import { toCollectorExportTraceServiceRequest } from '../../transform'; import { - CollectorExporterError, + GRPCSpanQueueItem, + ServiceClient, CollectorExporterConfigNode, -} from '../../types'; -import { toCollectorExportTraceServiceRequest } from '../../transform'; -import { GRPCSpanQueueItem, ServiceClient } from './types'; +} from './types'; import { removeProtocol } from './util'; const DEFAULT_COLLECTOR_URL = 'localhost:55680'; diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/types.ts b/packages/opentelemetry-exporter-collector/src/platform/node/types.ts index 01b1a9a33f2..507f478f44a 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/types.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/types.ts @@ -16,7 +16,10 @@ import * as grpc from 'grpc'; import { ReadableSpan } from '@opentelemetry/tracing'; -import { CollectorExporterError } from '../../types'; +import { + CollectorExporterError, + CollectorExporterConfigBase, +} from '../../types'; /** * Queue item to be used to save temporary spans in case the GRPC service @@ -38,3 +41,12 @@ export interface ServiceClient extends grpc.Client { callback: Function ) => {}; } + +/** + * Collector Exporter Config for Node + */ +export interface CollectorExporterConfigNode + extends CollectorExporterConfigBase { + credentials?: grpc.ChannelCredentials; + metadata?: grpc.Metadata; +} diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index 627cb736a84..68aa4fbf572 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -16,7 +16,6 @@ import { SpanKind, Logger, Attributes } from '@opentelemetry/api'; import * as api from '@opentelemetry/api'; -import * as grpc from 'grpc'; // header to prevent instrumentation on request export const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request'; @@ -42,6 +41,100 @@ export namespace opentelemetryProto { } } + export namespace metrics.v1 { + export interface Metric { + metricDescriptor: opentelemetryProto.metrics.v1.MetricDescriptor; + int64DataPoints?: opentelemetryProto.metrics.v1.Int64DataPoint[]; + doubleDataPoints?: opentelemetryProto.metrics.v1.DoubleDataPoint[]; + histogramDataPoints?: opentelemetryProto.metrics.v1.HistogramDataPoint[]; + summaryDataPoints?: opentelemetryProto.metrics.v1.SummaryDataPoint[]; + } + + export interface Int64DataPoint { + labels: opentelemetryProto.common.v1.StringKeyValue[]; + startTimeUnixNano: number; + timeUnixNano: number; + value: number; + } + + export interface DoubleDataPoint { + labels: opentelemetryProto.common.v1.StringKeyValue[]; + startTimeUnixNano: number; + timeUnixNano: number; + value: number; + } + + export interface HistogramDataPoint { + labels: opentelemetryProto.common.v1.StringKeyValue[]; + startTimeUnixNano: number; + timeUnixNano: number; + count: number; + sum: number; + buckets?: opentelemetryProto.metrics.v1.HistogramDataPointBucket[]; + explicitBounds?: number[]; + } + + export interface HistogramDataPointBucket { + count: number; + exemplar?: opentelemetryProto.metrics.v1.HistogramExemplar; + } + + export interface HistogramExemplar { + value: number; + timeUnixNano: number; + attachments: opentelemetryProto.common.v1.StringKeyValue[]; + } + + export interface SummaryDataPoint { + labels: opentelemetryProto.common.v1.StringKeyValue[]; + startTimeUnixNano: number; + timeUnixNano: number; + count?: number; + sum?: number; + percentileValues: opentelemetryProto.metrics.v1.SummaryDataPointValueAtPercentile[]; + } + + export interface SummaryDataPointValueAtPercentile { + percentile: number; + value: number; + } + + export interface MetricDescriptor { + name: string; + description: string; + unit: string; + type: opentelemetryProto.metrics.v1.MetricDescriptorType; + temporality: opentelemetryProto.metrics.v1.MetricDescriptorTemporality; + } + + export interface InstrumentationLibraryMetrics { + instrumentationLibrary?: opentelemetryProto.common.v1.InstrumentationLibrary; + metrics: opentelemetryProto.metrics.v1.Metric[]; + } + + export interface ResourceMetrics { + resource?: opentelemetryProto.resource.v1.Resource; + instrumentationLibraryMetrics: opentelemetryProto.metrics.v1.InstrumentationLibraryMetrics[]; + } + + export enum MetricDescriptorType { + INVALID_TYPE, + INT64, + MONOTONIC_INT64, + DOUBLE, + MONOTONIC_DOUBLE, + HISTOGRAM, + SUMMARY, + } + + export enum MetricDescriptorTemporality { + INVALID_TEMPORALITY, + INSTANTANEOUS, + DELTA, + CUMULATIVE, + } + } + export namespace trace.v1 { export namespace ConstantSampler { export enum ConstantDecision { @@ -185,23 +278,6 @@ export interface CollectorExporterConfigBase { url?: string; } -/** - * Collector Exporter Config for Web - */ -export interface CollectorExporterConfigBrowser - extends CollectorExporterConfigBase { - headers?: { [key: string]: string }; -} - -/** - * Collector Exporter Config for Node - */ -export interface CollectorExporterConfigNode - extends CollectorExporterConfigBase { - credentials?: grpc.ChannelCredentials; - metadata?: grpc.Metadata; -} - /** * Mapping between api SpanKind and proto SpanKind */ diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts index 2386b63ab5e..2e11e341dff 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts @@ -20,6 +20,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { CollectorTraceExporter } from '../../src/platform/browser/index'; import * as collectorTypes from '../../src/types'; +import { CollectorExporterConfigBrowser } from '../../src/platform/browser/types'; import { ensureSpanIsCorrect, @@ -32,7 +33,7 @@ const sendBeacon = navigator.sendBeacon; describe('CollectorTraceExporter - web', () => { let collectorTraceExporter: CollectorTraceExporter; - let collectorExporterConfig: collectorTypes.CollectorExporterConfigBrowser; + let collectorExporterConfig: CollectorExporterConfigBrowser; let spyOpen: any; let spySend: any; let spyBeacon: any; From f0a3dbabda9a212753e9caf9d8df925be8e9e7e9 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 6 Jul 2020 21:36:56 +0200 Subject: [PATCH 3/7] feat: adding json over http for collector exporter (#1247) * feat: adding json over http for collector exporter * feat: updating readme and adding headers options in config for json over http * chore: reviews and few small cleanups * chore: aligning type for headers * chore: fixing doc * chore: unifying types for headers * chore: reviews * chore: adding validation for headers, and making the types correct this time * chore: linting * chore: linting * chore: fixes after merge * chore: reviews * chore: merge branch 'master' into collector_json --- examples/collector-exporter-node/package.json | 1 + examples/collector-exporter-node/start.js | 8 +- .../README.md | 25 ++- .../src/CollectorTraceExporterBase.ts | 4 +- .../src/enums.ts | 24 +++ .../src/index.ts | 1 + .../browser/CollectorTraceExporter.ts | 12 +- .../platform/node/CollectorTraceExporter.ts | 121 +++++------- .../src/platform/node/types.ts | 2 + .../src/platform/node/utilWithGrpc.ts | 101 ++++++++++ .../src/platform/node/utilWithJson.ts | 84 ++++++++ .../src/transform.ts | 6 +- .../src/types.ts | 1 + .../src/util.ts | 38 ++++ .../browser/CollectorTraceExporter.test.ts | 6 +- .../common/CollectorTraceExporter.test.ts | 4 +- .../test/common/utils.test.ts | 48 +++++ .../node/CollectorExporterWithJson.test.ts | 187 ++++++++++++++++++ .../test/node/CollectorTraceExporter.test.ts | 42 +++- 19 files changed, 622 insertions(+), 93 deletions(-) create mode 100644 packages/opentelemetry-exporter-collector/src/enums.ts create mode 100644 packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts create mode 100644 packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts create mode 100644 packages/opentelemetry-exporter-collector/src/util.ts create mode 100644 packages/opentelemetry-exporter-collector/test/common/utils.test.ts create mode 100644 packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts diff --git a/examples/collector-exporter-node/package.json b/examples/collector-exporter-node/package.json index a30ad9a0b4d..5f5d7b39e68 100644 --- a/examples/collector-exporter-node/package.json +++ b/examples/collector-exporter-node/package.json @@ -27,6 +27,7 @@ }, "dependencies": { "@opentelemetry/api": "^0.9.0", + "@opentelemetry/core": "^0.9.0", "@opentelemetry/exporter-collector": "^0.9.0", "@opentelemetry/tracing": "^0.9.0" }, diff --git a/examples/collector-exporter-node/start.js b/examples/collector-exporter-node/start.js index 637a489cf6a..3f8f939653b 100644 --- a/examples/collector-exporter-node/start.js +++ b/examples/collector-exporter-node/start.js @@ -2,12 +2,14 @@ const opentelemetry = require('@opentelemetry/api'); const { BasicTracerProvider, ConsoleSpanExporter, SimpleSpanProcessor } = require('@opentelemetry/tracing'); -const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorTraceExporter, CollectorProtocolNode } = require('@opentelemetry/exporter-collector'); -const address = '127.0.0.1:55680'; const exporter = new CollectorTraceExporter({ serviceName: 'basic-service', - url: address, + // headers: { + // foo: 'bar' + // }, + protocolNode: CollectorProtocolNode.HTTP_JSON, }); const provider = new BasicTracerProvider(); diff --git a/packages/opentelemetry-exporter-collector/README.md b/packages/opentelemetry-exporter-collector/README.md index 7d198e23920..8cfda2f5a2d 100644 --- a/packages/opentelemetry-exporter-collector/README.md +++ b/packages/opentelemetry-exporter-collector/README.md @@ -36,7 +36,7 @@ provider.register(); ``` -## Usage in Node +## Usage in Node - GRPC The CollectorTraceExporter in Node expects the URL to only be the hostname. It will not work with `/v1/trace`. @@ -109,6 +109,29 @@ provider.register(); Note, that this will only work if TLS is also configured on the server. +## Usage in Node - JSON over http + +```js +const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); +const { CollectorExporter, CollectorTransportNode } = require('@opentelemetry/exporter-collector'); + +const collectorOptions = { + protocolNode: CollectorTransportNode.HTTP_JSON, + serviceName: 'basic-service', + url: '', // url is optional and can be omitted - default is http://localhost:55680/v1/trace + headers: { + foo: 'bar' + }, //an optional object containing custom headers to be sent with each request will only work with json over http +}; + +const provider = new BasicTracerProvider(); +const exporter = new CollectorExporter(collectorOptions); +provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); + +provider.register(); + +``` + ## Running opentelemetry-collector locally to see the traces 1. Go to examples/basic-tracer-node diff --git a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts index 4695c439940..f1508ec0beb 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts @@ -43,7 +43,7 @@ export abstract class CollectorTraceExporterBase< */ constructor(config: T = {} as T) { this.serviceName = config.serviceName || DEFAULT_SERVICE_NAME; - this.url = this.getDefaultUrl(config.url); + this.url = this.getDefaultUrl(config); if (typeof config.hostname === 'string') { this.hostname = config.hostname; } @@ -123,5 +123,5 @@ export abstract class CollectorTraceExporterBase< onSuccess: () => void, onError: (error: CollectorExporterError) => void ): void; - abstract getDefaultUrl(url: string | undefined): string; + abstract getDefaultUrl(config: T): string; } diff --git a/packages/opentelemetry-exporter-collector/src/enums.ts b/packages/opentelemetry-exporter-collector/src/enums.ts new file mode 100644 index 00000000000..08c2fd9f2b7 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/enums.ts @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Collector transport protocol node options + * Default is GRPC + */ +export enum CollectorProtocolNode { + GRPC, + HTTP_JSON, +} diff --git a/packages/opentelemetry-exporter-collector/src/index.ts b/packages/opentelemetry-exporter-collector/src/index.ts index 52ec5f71f5e..79ddd59b38a 100644 --- a/packages/opentelemetry-exporter-collector/src/index.ts +++ b/packages/opentelemetry-exporter-collector/src/index.ts @@ -15,3 +15,4 @@ */ export * from './platform'; +export * from './enums'; diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts index 39da94d1d0f..661f1e9b872 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts @@ -19,6 +19,7 @@ import { ReadableSpan } from '@opentelemetry/tracing'; import { toCollectorExportTraceServiceRequest } from '../../transform'; import { CollectorExporterConfigBrowser } from './types'; import * as collectorTypes from '../../types'; +import { parseHeaders } from '../../util'; const DEFAULT_COLLECTOR_URL = 'http://localhost:55680/v1/trace'; @@ -28,10 +29,10 @@ const DEFAULT_COLLECTOR_URL = 'http://localhost:55680/v1/trace'; export class CollectorTraceExporter extends CollectorTraceExporterBase< CollectorExporterConfigBrowser > { - DEFAULT_HEADERS: { [key: string]: string } = { + DEFAULT_HEADERS: Record = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; - private _headers: { [key: string]: string }; + private _headers: Record; private _useXHR: boolean = false; /** @@ -39,7 +40,8 @@ export class CollectorTraceExporter extends CollectorTraceExporterBase< */ constructor(config: CollectorExporterConfigBrowser = {}) { super(config); - this._headers = config.headers || this.DEFAULT_HEADERS; + this._headers = + parseHeaders(config.headers, this.logger) || this.DEFAULT_HEADERS; this._useXHR = !!config.headers || typeof navigator.sendBeacon !== 'function'; } @@ -52,8 +54,8 @@ export class CollectorTraceExporter extends CollectorTraceExporterBase< window.removeEventListener('unload', this.shutdown); } - getDefaultUrl(url: string | undefined) { - return url || DEFAULT_COLLECTOR_URL; + getDefaultUrl(config: CollectorExporterConfigBrowser) { + return config.url || DEFAULT_COLLECTOR_URL; } sendSpans( diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts index 8750315de5d..4cdaf9e41de 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts @@ -14,23 +14,29 @@ * limitations under the License. */ -import * as protoLoader from '@grpc/proto-loader'; +import { ReadableSpan } from '@opentelemetry/tracing'; import * as grpc from 'grpc'; -import * as path from 'path'; +import { CollectorTraceExporterBase } from '../../CollectorTraceExporterBase'; import * as collectorTypes from '../../types'; -import { ReadableSpan } from '@opentelemetry/tracing'; -import { CollectorTraceExporterBase } from '../../CollectorTraceExporterBase'; -import { CollectorExporterError } from '../../types'; -import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { CollectorProtocolNode } from '../../enums'; +import { parseHeaders } from '../../util'; import { GRPCSpanQueueItem, ServiceClient, CollectorExporterConfigNode, } from './types'; -import { removeProtocol } from './util'; -const DEFAULT_COLLECTOR_URL = 'localhost:55680'; +import { + DEFAULT_COLLECTOR_URL_GRPC, + onInitWithGrpc, + sendSpansUsingGrpc, +} from './utilWithGrpc'; +import { + DEFAULT_COLLECTOR_URL_JSON, + onInitWithJson, + sendSpansUsingJson, +} from './utilWithJson'; /** * Collector Trace Exporter for Node @@ -38,17 +44,39 @@ const DEFAULT_COLLECTOR_URL = 'localhost:55680'; export class CollectorTraceExporter extends CollectorTraceExporterBase< CollectorExporterConfigNode > { + DEFAULT_HEADERS: Record = { + [collectorTypes.OT_REQUEST_HEADER]: '1', + }; isShutDown: boolean = false; traceServiceClient?: ServiceClient = undefined; grpcSpansQueue: GRPCSpanQueueItem[] = []; metadata?: grpc.Metadata; + headers: Record; + private readonly _protocol: CollectorProtocolNode; /** * @param config */ constructor(config: CollectorExporterConfigNode = {}) { super(config); + this._protocol = + typeof config.protocolNode !== 'undefined' + ? config.protocolNode + : CollectorProtocolNode.GRPC; + if (this._protocol === CollectorProtocolNode.HTTP_JSON) { + this.logger.debug('CollectorExporter - using json over http'); + if (config.metadata) { + this.logger.warn('Metadata cannot be set when using json'); + } + } else { + this.logger.debug('CollectorExporter - using grpc'); + if (config.headers) { + this.logger.warn('Headers cannot be set when using grpc'); + } + } this.metadata = config.metadata; + this.headers = + parseHeaders(config.headers, this.logger) || this.DEFAULT_HEADERS; } onShutdown(): void { @@ -60,81 +88,36 @@ export class CollectorTraceExporter extends CollectorTraceExporterBase< onInit(config: CollectorExporterConfigNode): void { this.isShutDown = false; - this.grpcSpansQueue = []; - const serverAddress = removeProtocol(this.url); - const credentials: grpc.ChannelCredentials = - config.credentials || grpc.credentials.createInsecure(); - - const traceServiceProtoPath = - 'opentelemetry/proto/collector/trace/v1/trace_service.proto'; - const includeDirs = [path.resolve(__dirname, 'protos')]; - protoLoader - .load(traceServiceProtoPath, { - keepCase: false, - longs: String, - enums: String, - defaults: true, - oneofs: true, - includeDirs, - }) - .then(packageDefinition => { - const packageObject: any = grpc.loadPackageDefinition( - packageDefinition - ); - this.traceServiceClient = new packageObject.opentelemetry.proto.collector.trace.v1.TraceService( - serverAddress, - credentials - ); - if (this.grpcSpansQueue.length > 0) { - const queue = this.grpcSpansQueue.splice(0); - queue.forEach((item: GRPCSpanQueueItem) => { - this.sendSpans(item.spans, item.onSuccess, item.onError); - }); - } - }); + if (config.protocolNode === CollectorProtocolNode.HTTP_JSON) { + onInitWithJson(this, config); + } else { + onInitWithGrpc(this, config); + } } sendSpans( spans: ReadableSpan[], onSuccess: () => void, - onError: (error: CollectorExporterError) => void + onError: (error: collectorTypes.CollectorExporterError) => void ): void { if (this.isShutDown) { this.logger.debug('Shutdown already started. Cannot send spans'); return; } - if (this.traceServiceClient) { - const exportTraceServiceRequest = toCollectorExportTraceServiceRequest( - spans, - this - ); - - this.traceServiceClient.export( - exportTraceServiceRequest, - this.metadata, - (err: collectorTypes.ExportServiceError) => { - if (err) { - this.logger.error( - 'exportTraceServiceRequest', - exportTraceServiceRequest - ); - onError(err); - } else { - onSuccess(); - } - } - ); + if (this._protocol === CollectorProtocolNode.HTTP_JSON) { + sendSpansUsingJson(this, spans, onSuccess, onError); } else { - this.grpcSpansQueue.push({ - spans, - onSuccess, - onError, - }); + sendSpansUsingGrpc(this, spans, onSuccess, onError); } } - getDefaultUrl(url: string | undefined): string { - return url || DEFAULT_COLLECTOR_URL; + getDefaultUrl(config: CollectorExporterConfigNode): string { + if (!config.url) { + return config.protocolNode === CollectorProtocolNode.HTTP_JSON + ? DEFAULT_COLLECTOR_URL_JSON + : DEFAULT_COLLECTOR_URL_GRPC; + } + return config.url; } } diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/types.ts b/packages/opentelemetry-exporter-collector/src/platform/node/types.ts index 507f478f44a..a7eb8c74b0d 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/types.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/types.ts @@ -16,6 +16,7 @@ import * as grpc from 'grpc'; import { ReadableSpan } from '@opentelemetry/tracing'; +import { CollectorProtocolNode } from '../../enums'; import { CollectorExporterError, CollectorExporterConfigBase, @@ -49,4 +50,5 @@ export interface CollectorExporterConfigNode extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; metadata?: grpc.Metadata; + protocolNode?: CollectorProtocolNode; } diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts new file mode 100644 index 00000000000..49ca20fc872 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts @@ -0,0 +1,101 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as protoLoader from '@grpc/proto-loader'; +import * as grpc from 'grpc'; +import * as path from 'path'; +import * as collectorTypes from '../../types'; + +import { ReadableSpan } from '@opentelemetry/tracing'; +import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { CollectorTraceExporter } from './CollectorTraceExporter'; +import { CollectorExporterConfigNode, GRPCSpanQueueItem } from './types'; +import { removeProtocol } from './util'; + +export const DEFAULT_COLLECTOR_URL_GRPC = 'localhost:55680'; + +export function onInitWithGrpc( + collector: CollectorTraceExporter, + config: CollectorExporterConfigNode +): void { + collector.grpcSpansQueue = []; + const serverAddress = removeProtocol(collector.url); + const credentials: grpc.ChannelCredentials = + config.credentials || grpc.credentials.createInsecure(); + + const traceServiceProtoPath = + 'opentelemetry/proto/collector/trace/v1/trace_service.proto'; + const includeDirs = [path.resolve(__dirname, 'protos')]; + + protoLoader + .load(traceServiceProtoPath, { + keepCase: false, + longs: String, + enums: String, + defaults: true, + oneofs: true, + includeDirs, + }) + .then(packageDefinition => { + const packageObject: any = grpc.loadPackageDefinition(packageDefinition); + collector.traceServiceClient = new packageObject.opentelemetry.proto.collector.trace.v1.TraceService( + serverAddress, + credentials + ); + if (collector.grpcSpansQueue.length > 0) { + const queue = collector.grpcSpansQueue.splice(0); + queue.forEach((item: GRPCSpanQueueItem) => { + collector.sendSpans(item.spans, item.onSuccess, item.onError); + }); + } + }); +} + +export function sendSpansUsingGrpc( + collector: CollectorTraceExporter, + spans: ReadableSpan[], + onSuccess: () => void, + onError: (error: collectorTypes.CollectorExporterError) => void +): void { + if (collector.traceServiceClient) { + const exportTraceServiceRequest = toCollectorExportTraceServiceRequest( + spans, + collector + ); + collector.traceServiceClient.export( + exportTraceServiceRequest, + collector.metadata, + (err: collectorTypes.ExportServiceError) => { + if (err) { + collector.logger.error( + 'exportTraceServiceRequest', + exportTraceServiceRequest + ); + onError(err); + } else { + collector.logger.debug('spans sent'); + onSuccess(); + } + } + ); + } else { + collector.grpcSpansQueue.push({ + spans, + onSuccess, + onError, + }); + } +} diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts new file mode 100644 index 00000000000..a37638ac120 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts @@ -0,0 +1,84 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as url from 'url'; +import * as http from 'http'; +import * as https from 'https'; + +import { ReadableSpan } from '@opentelemetry/tracing'; +import * as collectorTypes from '../../types'; +import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { CollectorTraceExporter } from './CollectorTraceExporter'; +import { CollectorExporterConfigNode } from './types'; + +export const DEFAULT_COLLECTOR_URL_JSON = 'http://localhost:55680/v1/trace'; + +export function onInitWithJson( + _collector: CollectorTraceExporter, + _config: CollectorExporterConfigNode +): void { + // nothing to be done for json yet +} + +export function sendSpansUsingJson( + collector: CollectorTraceExporter, + spans: ReadableSpan[], + onSuccess: () => void, + onError: (error: collectorTypes.CollectorExporterError) => void +): void { + const exportTraceServiceRequest = toCollectorExportTraceServiceRequest( + spans, + collector + ); + + const body = JSON.stringify(exportTraceServiceRequest); + const parsedUrl = new url.URL(collector.url); + + const options = { + hostname: parsedUrl.hostname, + port: parsedUrl.port, + path: parsedUrl.pathname, + method: 'POST', + headers: { + 'Content-Length': Buffer.byteLength(body), + 'Content-Type': 'application/json', + ...collector.headers, + }, + }; + + const request = parsedUrl.protocol === 'http:' ? http.request : https.request; + const req = request(options, (res: http.IncomingMessage) => { + if (res.statusCode && res.statusCode < 299) { + collector.logger.debug(`statusCode: ${res.statusCode}`); + onSuccess(); + } else { + collector.logger.error(`statusCode: ${res.statusCode}`); + onError({ + code: res.statusCode, + message: res.statusMessage, + }); + } + }); + + req.on('error', (error: Error) => { + collector.logger.error('error', error.message); + onError({ + message: error.message, + }); + }); + req.write(body); + req.end(); +} diff --git a/packages/opentelemetry-exporter-collector/src/transform.ts b/packages/opentelemetry-exporter-collector/src/transform.ts index 626169d0777..0e5d0c1f96a 100644 --- a/packages/opentelemetry-exporter-collector/src/transform.ts +++ b/packages/opentelemetry-exporter-collector/src/transform.ts @@ -152,7 +152,7 @@ export function toCollectorSpan( */ export function toCollectorResource( resource?: Resource, - additionalAttributes: { [key: string]: any } = {} + additionalAttributes: { [key: string]: unknown } = {} ): opentelemetryProto.resource.v1.Resource { const attr = Object.assign( {}, @@ -195,14 +195,12 @@ export function toCollectorTraceState( * Prepares trace service request to be sent to collector * @param spans spans * @param collectorExporterBase - * @param [name] Instrumentation Library Name */ export function toCollectorExportTraceServiceRequest< T extends CollectorExporterConfigBase >( spans: ReadableSpan[], - collectorExporterBase: CollectorTraceExporterBase, - name = '' + collectorExporterBase: CollectorTraceExporterBase ): opentelemetryProto.collector.trace.v1.ExportTraceServiceRequest { const groupedSpans: Map< Resource, diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index 68aa4fbf572..907ba602761 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -271,6 +271,7 @@ export interface ExportServiceError { * Collector Exporter base config */ export interface CollectorExporterConfigBase { + headers?: Partial>; hostname?: string; logger?: Logger; serviceName?: string; diff --git a/packages/opentelemetry-exporter-collector/src/util.ts b/packages/opentelemetry-exporter-collector/src/util.ts new file mode 100644 index 00000000000..1cb1b18aae4 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/util.ts @@ -0,0 +1,38 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Logger } from '@opentelemetry/api'; +import { NoopLogger } from '@opentelemetry/core'; + +/** + * Parses headers from config leaving only those that have defined values + * @param partialHeaders + * @param logger + */ +export function parseHeaders( + partialHeaders: Partial> = {}, + logger: Logger = new NoopLogger() +): Record { + const headers: Record = {}; + Object.entries(partialHeaders).forEach(([key, value]) => { + if (typeof value !== 'undefined') { + headers[key] = String(value); + } else { + logger.warn(`Header "${key}" has wrong value and will be ignored`); + } + }); + return headers; +} diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts index 2e11e341dff..4893ddcb98f 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts @@ -34,9 +34,9 @@ const sendBeacon = navigator.sendBeacon; describe('CollectorTraceExporter - web', () => { let collectorTraceExporter: CollectorTraceExporter; let collectorExporterConfig: CollectorExporterConfigBrowser; - let spyOpen: any; - let spySend: any; - let spyBeacon: any; + let spyOpen: sinon.SinonSpy; + let spySend: sinon.SinonSpy; + let spyBeacon: sinon.SinonSpy; let spans: ReadableSpan[]; beforeEach(() => { diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index 504aff43385..69c4a15de31 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts @@ -29,8 +29,8 @@ class CollectorTraceExporter extends CollectorTraceExporterBase< onInit() {} onShutdown() {} sendSpans() {} - getDefaultUrl(url: string | undefined) { - return url || ''; + getDefaultUrl(config: CollectorExporterConfig) { + return config.url || ''; } } diff --git a/packages/opentelemetry-exporter-collector/test/common/utils.test.ts b/packages/opentelemetry-exporter-collector/test/common/utils.test.ts new file mode 100644 index 00000000000..b5fb8d3507e --- /dev/null +++ b/packages/opentelemetry-exporter-collector/test/common/utils.test.ts @@ -0,0 +1,48 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { NoopLogger } from '@opentelemetry/core'; +import { parseHeaders } from '../../src/util'; + +describe('utils', () => { + describe('parseHeaders', () => { + it('should ignore undefined headers', () => { + const logger = new NoopLogger(); + const spyWarn = sinon.stub(logger, 'warn'); + const headers: Partial> = { + foo1: undefined, + foo2: 'bar', + foo3: 1, + }; + const result = parseHeaders(headers, logger); + assert.deepStrictEqual(result, { + foo2: 'bar', + foo3: '1', + }); + const args = spyWarn.args[0]; + assert.strictEqual( + args[0], + 'Header "foo1" has wrong value and will be ignored' + ); + }); + it('should parse undefined', () => { + const result = parseHeaders(undefined); + assert.deepStrictEqual(result, {}); + }); + }); +}); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts new file mode 100644 index 00000000000..3b21ea2c638 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts @@ -0,0 +1,187 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as core from '@opentelemetry/core'; +import { ReadableSpan } from '@opentelemetry/tracing'; +import * as http from 'http'; +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { CollectorProtocolNode } from '../../src/enums'; +import { CollectorTraceExporter } from '../../src/platform/node'; +import { CollectorExporterConfigNode } from '../../src/platform/node/types'; +import * as collectorTypes from '../../src/types'; + +import { + ensureExportTraceServiceRequestIsSet, + ensureSpanIsCorrect, + mockedReadableSpan, +} from '../helper'; + +const fakeRequest = { + end: function () {}, + on: function () {}, + write: function () {}, +}; + +const mockRes = { + statusCode: 200, +}; + +const mockResError = { + statusCode: 400, +}; + +describe('CollectorExporter - node with json over http', () => { + let collectorExporter: CollectorTraceExporter; + let collectorExporterConfig: CollectorExporterConfigNode; + let spyRequest: sinon.SinonSpy; + let spyWrite: sinon.SinonSpy; + let spans: ReadableSpan[]; + describe('export', () => { + beforeEach(() => { + spyRequest = sinon.stub(http, 'request').returns(fakeRequest as any); + spyWrite = sinon.stub(fakeRequest, 'write'); + collectorExporterConfig = { + headers: { + foo: 'bar', + }, + protocolNode: CollectorProtocolNode.HTTP_JSON, + hostname: 'foo', + logger: new core.NoopLogger(), + serviceName: 'bar', + attributes: {}, + url: 'http://foo.bar.com', + }; + collectorExporter = new CollectorTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + }); + afterEach(() => { + spyRequest.restore(); + spyWrite.restore(); + }); + + it('should open the connection', done => { + collectorExporter.export(spans, () => {}); + + setTimeout(() => { + const args = spyRequest.args[0]; + const options = args[0]; + + assert.strictEqual(options.hostname, 'foo.bar.com'); + assert.strictEqual(options.method, 'POST'); + assert.strictEqual(options.path, '/'); + done(); + }); + }); + + it('should set custom headers', done => { + collectorExporter.export(spans, () => {}); + + setTimeout(() => { + const args = spyRequest.args[0]; + const options = args[0]; + assert.strictEqual(options.headers['foo'], 'bar'); + done(); + }); + }); + + it('should successfully send the spans', done => { + collectorExporter.export(spans, () => {}); + + setTimeout(() => { + const writeArgs = spyWrite.args[0]; + const json = JSON.parse( + writeArgs[0] + ) as collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceRequest; + const span1 = + json.resourceSpans[0].instrumentationLibrarySpans[0].spans[0]; + assert.ok(typeof span1 !== 'undefined', "span doesn't exist"); + if (span1) { + ensureSpanIsCorrect(span1); + } + + ensureExportTraceServiceRequestIsSet(json); + + done(); + }); + }); + + it('should log the successful message', done => { + const spyLoggerDebug = sinon.stub(collectorExporter.logger, 'debug'); + const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + + const responseSpy = sinon.spy(); + collectorExporter.export(spans, responseSpy); + + setTimeout(() => { + const args = spyRequest.args[0]; + const callback = args[1]; + callback(mockRes); + setTimeout(() => { + const response: any = spyLoggerDebug.args[1][0]; + assert.strictEqual(response, 'statusCode: 200'); + assert.strictEqual(spyLoggerError.args.length, 0); + assert.strictEqual(responseSpy.args[0][0], 0); + done(); + }); + }); + }); + + it('should log the error message', done => { + const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + + const responseSpy = sinon.spy(); + collectorExporter.export(spans, responseSpy); + + setTimeout(() => { + const args = spyRequest.args[0]; + const callback = args[1]; + callback(mockResError); + setTimeout(() => { + const response: any = spyLoggerError.args[0][0]; + assert.strictEqual(response, 'statusCode: 400'); + + assert.strictEqual(responseSpy.args[0][0], 1); + done(); + }); + }); + }); + }); + describe('CollectorTraceExporter - node (getDefaultUrl)', () => { + it('should default to localhost', done => { + const collectorExporter = new CollectorTraceExporter({ + protocolNode: CollectorProtocolNode.HTTP_JSON, + }); + setTimeout(() => { + assert.strictEqual( + collectorExporter['url'], + 'http://localhost:55680/v1/trace' + ); + done(); + }); + }); + + it('should keep the URL if included', done => { + const url = 'http://foo.bar.com'; + const collectorExporter = new CollectorTraceExporter({ url }); + setTimeout(() => { + assert.strictEqual(collectorExporter['url'], url); + done(); + }); + }); + }); +}); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts index 514125e8822..91e89fc02fc 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts @@ -15,23 +15,25 @@ */ import * as protoLoader from '@grpc/proto-loader'; -import * as grpc from 'grpc'; -import * as path from 'path'; -import * as fs from 'fs'; +import { ConsoleLogger, LogLevel } from '@opentelemetry/core'; import { BasicTracerProvider, SimpleSpanProcessor, } from '@opentelemetry/tracing'; import * as assert from 'assert'; +import * as fs from 'fs'; +import * as grpc from 'grpc'; +import * as path from 'path'; import * as sinon from 'sinon'; +import { CollectorProtocolNode } from '../../src'; import { CollectorTraceExporter } from '../../src/platform/node'; import * as collectorTypes from '../../src/types'; import { - ensureResourceIsCorrect, ensureExportedSpanIsCorrect, ensureMetadataIsCorrect, + ensureResourceIsCorrect, mockedReadableSpan, } from '../helper'; @@ -138,6 +140,38 @@ const testCollectorExporter = (params: TestParams) => reqMetadata = undefined; }); + describe('instance', () => { + it('should warn about headers when using grpc', () => { + const logger = new ConsoleLogger(LogLevel.DEBUG); + const spyLoggerWarn = sinon.stub(logger, 'warn'); + collectorExporter = new CollectorTraceExporter({ + logger, + serviceName: 'basic-service', + url: address, + headers: { + foo: 'bar', + }, + }); + const args = spyLoggerWarn.args[0]; + assert.strictEqual(args[0], 'Headers cannot be set when using grpc'); + }); + it('should warn about metadata when using json', () => { + const metadata = new grpc.Metadata(); + metadata.set('k', 'v'); + const logger = new ConsoleLogger(LogLevel.DEBUG); + const spyLoggerWarn = sinon.stub(logger, 'warn'); + collectorExporter = new CollectorTraceExporter({ + logger, + serviceName: 'basic-service', + url: address, + metadata, + protocolNode: CollectorProtocolNode.HTTP_JSON, + }); + const args = spyLoggerWarn.args[0]; + assert.strictEqual(args[0], 'Metadata cannot be set when using json'); + }); + }); + describe('export', () => { it('should export spans', done => { const responseSpy = sinon.spy(); From 85c430a3b4232582e069e935d7c0f8ece53cb464 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 6 Jul 2020 16:05:04 -0400 Subject: [PATCH 4/7] feat(opentelemetry-web): capture encodedBodySize / http.response_content_length (#1262) --- .../src/enums/AttributeNames.ts | 1 + .../test/fetch.test.ts | 6 +++++- .../test/xhr.test.ts | 16 ++++++++++------ .../src/trace/http.ts | 1 + packages/opentelemetry-web/package.json | 1 + .../src/enums/PerformanceTimingNames.ts | 1 + packages/opentelemetry-web/src/types.ts | 1 + packages/opentelemetry-web/src/utils.ts | 7 +++++++ packages/opentelemetry-web/test/utils.test.ts | 4 ++++ 9 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-plugin-fetch/src/enums/AttributeNames.ts b/packages/opentelemetry-plugin-fetch/src/enums/AttributeNames.ts index 452a1111dd7..f216cee8bec 100644 --- a/packages/opentelemetry-plugin-fetch/src/enums/AttributeNames.ts +++ b/packages/opentelemetry-plugin-fetch/src/enums/AttributeNames.ts @@ -28,4 +28,5 @@ export enum AttributeNames { HTTP_URL = 'http.url', HTTP_TARGET = 'http.target', HTTP_USER_AGENT = 'http.user_agent', + HTTP_RESPONSE_CONTENT_LENGTH = 'http.response_content_length', } diff --git a/packages/opentelemetry-plugin-fetch/test/fetch.test.ts b/packages/opentelemetry-plugin-fetch/test/fetch.test.ts index cf27f84f40d..0ce12cb82df 100644 --- a/packages/opentelemetry-plugin-fetch/test/fetch.test.ts +++ b/packages/opentelemetry-plugin-fetch/test/fetch.test.ts @@ -300,8 +300,12 @@ describe('fetch', () => { attributes[keys[7]] !== '', `attributes ${AttributeNames.HTTP_USER_AGENT} is not defined` ); + assert.ok( + (attributes[keys[8]] as number) > 0, + `attributes ${AttributeNames.HTTP_RESPONSE_CONTENT_LENGTH} is <= 0` + ); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 9, 'number of attributes is wrong'); }); it('span should have correct events', () => { diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 7887339acb6..4491cb0fb9e 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -290,31 +290,35 @@ describe('xhr', () => { url, `attributes ${HttpAttribute.HTTP_URL} is wrong` ); + assert.ok( + (attributes[keys[2]] as number) > 0, + 'attributes ${HttpAttributes.HTTP_RESPONSE_CONTENT_SIZE} <= 0' + ); assert.strictEqual( - attributes[keys[2]], + attributes[keys[3]], 200, `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( - attributes[keys[3]], + attributes[keys[4]], 'OK', `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` ); assert.strictEqual( - attributes[keys[4]], + attributes[keys[5]], parseUrl(url).host, `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( - attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', + attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` ); assert.ok( - attributes[keys[6]] !== '', + attributes[keys[7]] !== '', `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); - assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); }); it('span should have correct events', () => { diff --git a/packages/opentelemetry-semantic-conventions/src/trace/http.ts b/packages/opentelemetry-semantic-conventions/src/trace/http.ts index d194db6a412..d54166c5453 100644 --- a/packages/opentelemetry-semantic-conventions/src/trace/http.ts +++ b/packages/opentelemetry-semantic-conventions/src/trace/http.ts @@ -25,6 +25,7 @@ export const HttpAttribute = { HTTP_SERVER_NAME: 'http.server_name', HTTP_CLIENT_IP: 'http.client_ip', HTTP_SCHEME: 'http.scheme', + HTTP_RESPONSE_CONTENT_LENGTH: 'http.response_content_length', // NOT ON OFFICIAL SPEC HTTP_ERROR_NAME: 'http.error_name', diff --git a/packages/opentelemetry-web/package.json b/packages/opentelemetry-web/package.json index 60f966c82fa..a584ebb6ea3 100644 --- a/packages/opentelemetry-web/package.json +++ b/packages/opentelemetry-web/package.json @@ -78,6 +78,7 @@ "@opentelemetry/api": "^0.9.0", "@opentelemetry/context-base": "^0.9.0", "@opentelemetry/core": "^0.9.0", + "@opentelemetry/semantic-conventions": "^0.9.0", "@opentelemetry/tracing": "^0.9.0" } } diff --git a/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts b/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts index 846a9ba967e..cbcfc4fe36d 100644 --- a/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts +++ b/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts @@ -23,6 +23,7 @@ export enum PerformanceTimingNames { DOM_INTERACTIVE = 'domInteractive', DOMAIN_LOOKUP_END = 'domainLookupEnd', DOMAIN_LOOKUP_START = 'domainLookupStart', + ENCODED_BODY_SIZE = 'encodedBodySize', FETCH_START = 'fetchStart', LOAD_EVENT_END = 'loadEventEnd', LOAD_EVENT_START = 'loadEventStart', diff --git a/packages/opentelemetry-web/src/types.ts b/packages/opentelemetry-web/src/types.ts index 10ee04f14ec..923ec0e13cc 100644 --- a/packages/opentelemetry-web/src/types.ts +++ b/packages/opentelemetry-web/src/types.ts @@ -24,6 +24,7 @@ export type PerformanceEntries = { [PerformanceTimingNames.DOM_INTERACTIVE]?: number; [PerformanceTimingNames.DOMAIN_LOOKUP_END]?: number; [PerformanceTimingNames.DOMAIN_LOOKUP_START]?: number; + [PerformanceTimingNames.ENCODED_BODY_SIZE]?: number; [PerformanceTimingNames.FETCH_START]?: number; [PerformanceTimingNames.LOAD_EVENT_END]?: number; [PerformanceTimingNames.LOAD_EVENT_START]?: number; diff --git a/packages/opentelemetry-web/src/utils.ts b/packages/opentelemetry-web/src/utils.ts index aa3dd320b69..a1d539987a4 100644 --- a/packages/opentelemetry-web/src/utils.ts +++ b/packages/opentelemetry-web/src/utils.ts @@ -26,6 +26,7 @@ import { timeInputToHrTime, urlMatches, } from '@opentelemetry/core'; +import { HttpAttribute } from '@opentelemetry/semantic-conventions'; /** * Helper function to be able to use enum as typed key in type and in interface when using forEach @@ -81,6 +82,12 @@ export function addSpanNetworkEvents( addSpanNetworkEvent(span, PTN.REQUEST_START, resource); addSpanNetworkEvent(span, PTN.RESPONSE_START, resource); addSpanNetworkEvent(span, PTN.RESPONSE_END, resource); + if (resource[PTN.ENCODED_BODY_SIZE]) { + span.setAttribute( + HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH, + resource[PTN.ENCODED_BODY_SIZE] + ); + } } /** diff --git a/packages/opentelemetry-web/test/utils.test.ts b/packages/opentelemetry-web/test/utils.test.ts index 805986de71e..046589793b2 100644 --- a/packages/opentelemetry-web/test/utils.test.ts +++ b/packages/opentelemetry-web/test/utils.test.ts @@ -137,8 +137,10 @@ describe('utils', () => { describe('addSpanNetworkEvents', () => { it('should add all network events to span', () => { const addEventSpy = sinon.spy(); + const setAttributeSpy = sinon.spy(); const span = ({ addEvent: addEventSpy, + setAttribute: setAttributeSpy, } as unknown) as tracing.Span; const entries = { [PTN.FETCH_START]: 123, @@ -150,6 +152,7 @@ describe('utils', () => { [PTN.REQUEST_START]: 123, [PTN.RESPONSE_START]: 123, [PTN.RESPONSE_END]: 123, + [PTN.ENCODED_BODY_SIZE]: 123, } as PerformanceEntries; assert.strictEqual(addEventSpy.callCount, 0); @@ -157,6 +160,7 @@ describe('utils', () => { addSpanNetworkEvents(span, entries); assert.strictEqual(addEventSpy.callCount, 9); + assert.strictEqual(setAttributeSpy.callCount, 1); }); }); describe('addSpanNetworkEvent', () => { From d4544920d70c40f3accfae9d232f05a2d7ba8422 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 6 Jul 2020 22:36:04 +0200 Subject: [PATCH 5/7] chore: updating aggregator MinMaxLastSumCount and use it for value observer and value recorder (#1276) * chore: updating aggregator MinMaxLastSumCount and use it for value observer and value recorder * chore: fix after merge --- .../test/helper.ts | 4 +- .../src/prometheus.ts | 4 +- .../src/export/Batcher.ts | 4 +- .../src/export/aggregators/LastValue.ts | 37 ----- ...inMaxSumCount.ts => MinMaxLastSumCount.ts} | 8 +- .../src/export/aggregators/index.ts | 3 +- .../opentelemetry-metrics/src/export/types.ts | 1 + .../opentelemetry-metrics/test/Meter.test.ts | 137 +++++++++++++++--- 8 files changed, 127 insertions(+), 71 deletions(-) delete mode 100644 packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts rename packages/opentelemetry-metrics/src/export/aggregators/{MinMaxSumCount.ts => MinMaxLastSumCount.ts} (87%) diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index 4e57a6edbbf..7607beab93a 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -26,7 +26,7 @@ import { MetricRecord, MetricKind, SumAggregator, - LastValueAggregator, + MinMaxLastSumCountAggregator, } from '@opentelemetry/metrics'; if (typeof Buffer === 'undefined') { @@ -85,7 +85,7 @@ export const mockObserver: MetricRecord = { valueType: ValueType.DOUBLE, }, labels: {}, - aggregator: new LastValueAggregator(), + aggregator: new MinMaxLastSumCountAggregator(), resource: new Resource({ service: 'ui', version: 1, diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 2951ddeeeed..af0c551dbab 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -161,10 +161,10 @@ export class PrometheusExporter implements MetricExporter { (point.value as Histogram).sum, hrTimeToMilliseconds(point.timestamp) ); - } else if (typeof (point.value as Distribution).max === 'number') { + } else if (typeof (point.value as Distribution).last === 'number') { metric.set( labels, - (point.value as Distribution).sum, + (point.value as Distribution).last, hrTimeToMilliseconds(point.timestamp) ); } diff --git a/packages/opentelemetry-metrics/src/export/Batcher.ts b/packages/opentelemetry-metrics/src/export/Batcher.ts index 9bcf574d020..18e871aa63e 100644 --- a/packages/opentelemetry-metrics/src/export/Batcher.ts +++ b/packages/opentelemetry-metrics/src/export/Batcher.ts @@ -57,9 +57,9 @@ export class UngroupedBatcher extends Batcher { return new aggregators.SumAggregator(); case MetricKind.VALUE_RECORDER: case MetricKind.VALUE_OBSERVER: - return new aggregators.LastValueAggregator(); + return new aggregators.MinMaxLastSumCountAggregator(); default: - return new aggregators.MinMaxSumCountAggregator(); + return new aggregators.MinMaxLastSumCountAggregator(); } } diff --git a/packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts b/packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts deleted file mode 100644 index 20c049c842a..00000000000 --- a/packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Aggregator, Point } from '../types'; -import { HrTime } from '@opentelemetry/api'; -import { hrTime } from '@opentelemetry/core'; - -/** Basic aggregator for LastValue which keeps the last recorded value. */ -export class LastValueAggregator implements Aggregator { - private _current: number = 0; - private _lastUpdateTime: HrTime = [0, 0]; - - update(value: number): void { - this._current = value; - this._lastUpdateTime = hrTime(); - } - - toPoint(): Point { - return { - value: this._current, - timestamp: this._lastUpdateTime, - }; - } -} diff --git a/packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts b/packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts similarity index 87% rename from packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts rename to packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts index 9be0fe8350f..7f5a9df431f 100644 --- a/packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts +++ b/packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts @@ -19,8 +19,10 @@ import { HrTime } from '@opentelemetry/api'; import { hrTime } from '@opentelemetry/core'; import { Distribution } from '../types'; -/** Basic aggregator keeping all raw values (events, sum, max and min). */ -export class MinMaxSumCountAggregator implements Aggregator { +/** + * Basic aggregator keeping all raw values (events, sum, max, last and min). + */ +export class MinMaxLastSumCountAggregator implements Aggregator { private _distribution: Distribution; private _lastUpdateTime: HrTime = [0, 0]; @@ -28,6 +30,7 @@ export class MinMaxSumCountAggregator implements Aggregator { this._distribution = { min: Infinity, max: -Infinity, + last: 0, sum: 0, count: 0, }; @@ -36,6 +39,7 @@ export class MinMaxSumCountAggregator implements Aggregator { update(value: number): void { this._distribution.count++; this._distribution.sum += value; + this._distribution.last = value; this._distribution.min = Math.min(this._distribution.min, value); this._distribution.max = Math.max(this._distribution.max, value); this._lastUpdateTime = hrTime(); diff --git a/packages/opentelemetry-metrics/src/export/aggregators/index.ts b/packages/opentelemetry-metrics/src/export/aggregators/index.ts index 0d3f8155b7c..93001f73d7c 100644 --- a/packages/opentelemetry-metrics/src/export/aggregators/index.ts +++ b/packages/opentelemetry-metrics/src/export/aggregators/index.ts @@ -15,6 +15,5 @@ */ export * from './histogram'; -export * from './MinMaxSumCount'; -export * from './LastValue'; +export * from './MinMaxLastSumCount'; export * from './Sum'; diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index 192cabd414d..66c1262e379 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -37,6 +37,7 @@ export type LastValue = number; export interface Distribution { min: number; max: number; + last: number; count: number; sum: number; } diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts index 2cd8c429377..95f985a9035 100644 --- a/packages/opentelemetry-metrics/test/Meter.test.ts +++ b/packages/opentelemetry-metrics/test/Meter.test.ts @@ -27,8 +27,9 @@ import { MetricRecord, Aggregator, MetricDescriptor, - LastValueAggregator, UpDownCounterMetric, + Distribution, + MinMaxLastSumCountAggregator, } from '../src'; import * as api from '@opentelemetry/api'; import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core'; @@ -562,7 +563,16 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 0, + last: 0, + max: -Infinity, + min: Infinity, + sum: 0, + } + ); }); it('should not set the instrument data when disabled', async () => { @@ -574,7 +584,16 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); - assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0); + assert.deepStrictEqual( + record1.aggregator.toPoint().value as Distribution, + { + count: 0, + last: 0, + max: -Infinity, + min: Infinity, + sum: 0, + } + ); }); it( @@ -591,8 +610,14 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); assert.deepStrictEqual( - record1.aggregator.toPoint().value as number, - 50 + record1.aggregator.toPoint().value as Distribution, + { + count: 2, + last: 50, + max: 50, + min: -10, + sum: 40, + } ); assert.ok( hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) > @@ -612,8 +637,14 @@ describe('Meter', () => { await meter.collect(); const [record1] = meter.getBatcher().checkPointSet(); assert.deepStrictEqual( - record1.aggregator.toPoint().value as number, - 100 + record1.aggregator.toPoint().value as Distribution, + { + count: 2, + last: 100, + max: 100, + min: 10, + sum: 110, + } ); assert.strictEqual(boundValueRecorder1, boundValueRecorder2); }); @@ -809,10 +840,34 @@ describe('Meter', () => { assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1'); assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2'); - ensureMetric(metric1, 'cpu_temp_per_app', 67); - ensureMetric(metric2, 'cpu_temp_per_app', 69); - ensureMetric(metric3, 'cpu_temp_per_app', 67); - ensureMetric(metric4, 'cpu_temp_per_app', 69); + ensureMetric(metric1, 'cpu_temp_per_app', { + count: 1, + last: 67, + max: 67, + min: 67, + sum: 67, + }); + ensureMetric(metric2, 'cpu_temp_per_app', { + count: 1, + last: 69, + max: 69, + min: 69, + sum: 69, + }); + ensureMetric(metric3, 'cpu_temp_per_app', { + count: 1, + last: 67, + max: 67, + min: 67, + sum: 67, + }); + ensureMetric(metric4, 'cpu_temp_per_app', { + count: 1, + last: 69, + max: 69, + min: 69, + sum: 69, + }); const metric5 = cpuUsageMetricRecords[0]; const metric6 = cpuUsageMetricRecords[1]; @@ -823,10 +878,34 @@ describe('Meter', () => { assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1'); assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2'); - ensureMetric(metric5, 'cpu_usage_per_app', 2.1); - ensureMetric(metric6, 'cpu_usage_per_app', 3.1); - ensureMetric(metric7, 'cpu_usage_per_app', 1.2); - ensureMetric(metric8, 'cpu_usage_per_app', 4.5); + ensureMetric(metric5, 'cpu_usage_per_app', { + count: 1, + last: 2.1, + max: 2.1, + min: 2.1, + sum: 2.1, + }); + ensureMetric(metric6, 'cpu_usage_per_app', { + count: 1, + last: 3.1, + max: 3.1, + min: 3.1, + sum: 3.1, + }); + ensureMetric(metric7, 'cpu_usage_per_app', { + count: 1, + last: 1.2, + max: 1.2, + min: 1.2, + sum: 1.2, + }); + ensureMetric(metric8, 'cpu_usage_per_app', { + count: 1, + last: 4.5, + max: 4.5, + min: 4.5, + sum: 4.5, + }); }); it('should not observe values when timeout', done => { @@ -856,9 +935,15 @@ describe('Meter', () => { const value = cpuUsageMetric .bind({ foo: 'bar' }) .getAggregator() - .toPoint().value as number; - - assert.strictEqual(value, 0); + .toPoint().value as Distribution; + + assert.deepStrictEqual(value, { + count: 0, + last: 0, + max: -Infinity, + min: Infinity, + sum: 0, + }); assert.strictEqual(cpuUsageMetricRecords.length, 0); done(); }); @@ -961,13 +1046,17 @@ class CustomBatcher extends Batcher { } } -function ensureMetric(metric: MetricRecord, name?: string, value?: number) { - assert.ok(metric.aggregator instanceof LastValueAggregator); - const lastValue = metric.aggregator.toPoint().value; - if (typeof value === 'number') { - assert.strictEqual(lastValue, value); +function ensureMetric( + metric: MetricRecord, + name?: string, + value?: Distribution +) { + assert.ok(metric.aggregator instanceof MinMaxLastSumCountAggregator); + const distribution = metric.aggregator.toPoint().value as Distribution; + if (value) { + assert.deepStrictEqual(distribution, value); } else { - assert.ok(lastValue >= 0 && lastValue <= 1); + assert.ok(distribution.last >= 0 && distribution.last <= 1); } const descriptor = metric.descriptor; assert.strictEqual(descriptor.name, name || 'name'); From 7cce64fcd472a2c55f83ab19a8c5eb0a69257bde Mon Sep 17 00:00:00 2001 From: legendecas Date: Wed, 8 Jul 2020 01:14:19 +0800 Subject: [PATCH 6/7] refactor: remove unnecessary new Promise operation (#1277) --- packages/opentelemetry-metrics/src/Meter.ts | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts index 0842eefabd3..08dd71fb55f 100644 --- a/packages/opentelemetry-metrics/src/Meter.ts +++ b/packages/opentelemetry-metrics/src/Meter.ts @@ -21,7 +21,6 @@ import { BatchObserverMetric } from './BatchObserverMetric'; import { BaseBoundInstrument } from './BoundInstrument'; import { UpDownCounterMetric } from './UpDownCounterMetric'; import { CounterMetric } from './CounterMetric'; -import { MetricRecord } from './export/types'; import { ValueRecorderMetric } from './ValueRecorderMetric'; import { Metric } from './Metric'; import { ValueObserverMetric } from './ValueObserverMetric'; @@ -231,20 +230,14 @@ export class Meter implements api.Meter { * each aggregator belonging to the metrics that were created with this * meter instance. */ - collect(): Promise { - return new Promise((resolve, reject) => { - const metrics: Promise[] = []; - Array.from(this._metrics.values()).forEach(metric => { - metrics.push(metric.getMetricRecord()); + async collect(): Promise { + const metrics = Array.from(this._metrics.values()).map(metric => { + return metric.getMetricRecord(); + }); + await Promise.all(metrics).then(records => { + records.forEach(metrics => { + metrics.forEach(metric => this._batcher.process(metric)); }); - Promise.all(metrics) - .then(records => { - records.forEach(metrics => { - metrics.forEach(metric => this._batcher.process(metric)); - }); - resolve(); - }) - .catch(reject); }); } From 264fb36a8eb28e55fdc162384c428ac1cd1293c3 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 8 Jul 2020 00:36:34 -0400 Subject: [PATCH 7/7] fix: do not crash on fetch(new Request(url)) (#1274) * fix: do not crash on fetch(new Request(url)) * chore: fix headers type --- .../opentelemetry-plugin-fetch/src/fetch.ts | 18 ++++++++++++------ .../test/fetch.test.ts | 8 +++++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-plugin-fetch/src/fetch.ts b/packages/opentelemetry-plugin-fetch/src/fetch.ts index cccc7d43f93..d3865ac0d4c 100644 --- a/packages/opentelemetry-plugin-fetch/src/fetch.ts +++ b/packages/opentelemetry-plugin-fetch/src/fetch.ts @@ -98,7 +98,7 @@ export class FetchPlugin extends core.BasePlugin> { * @param options * @param spanUrl */ - private _addHeaders(options: RequestInit, spanUrl: string): void { + private _addHeaders(options: Request | RequestInit, spanUrl: string): void { if ( !web.shouldPropagateTraceHeaders( spanUrl, @@ -107,9 +107,16 @@ export class FetchPlugin extends core.BasePlugin> { ) { return; } - const headers: { [key: string]: unknown } = {}; - api.propagation.inject(headers); - options.headers = Object.assign({}, headers, options.headers || {}); + + if (options instanceof Request) { + api.propagation.inject(options.headers, (h, k, v) => + h.set(k, typeof v === 'string' ? v : String(v)) + ); + } else { + const headers: Partial> = {}; + api.propagation.inject(headers); + options.headers = Object.assign({}, headers, options.headers || {}); + } } /** @@ -242,8 +249,7 @@ export class FetchPlugin extends core.BasePlugin> { init?: RequestInit ): Promise { const url = input instanceof Request ? input.url : input; - const options: RequestInit = - input instanceof Request ? input : init || {}; + const options = input instanceof Request ? input : init || {}; const span = plugin._createSpan(url, options); if (!span) { diff --git a/packages/opentelemetry-plugin-fetch/test/fetch.test.ts b/packages/opentelemetry-plugin-fetch/test/fetch.test.ts index 0ce12cb82df..9f70969be03 100644 --- a/packages/opentelemetry-plugin-fetch/test/fetch.test.ts +++ b/packages/opentelemetry-plugin-fetch/test/fetch.test.ts @@ -117,7 +117,7 @@ describe('fetch', () => { sandbox.stub(core.otperformance, 'timeOrigin').value(0); sandbox.stub(core.otperformance, 'now').callsFake(() => fakeNow); - function fakeFetch(input: RequestInfo, init: RequestInit = {}) { + function fakeFetch(input: RequestInfo | Request, init: RequestInit = {}) { return new Promise((resolve, reject) => { const response: any = { args: {}, @@ -459,6 +459,12 @@ describe('fetch', () => { ); }); + it('should set trace headers with a request object', () => { + const r = new Request('url'); + window.fetch(r); + assert.ok(typeof r.headers.get(core.X_B3_TRACE_ID) === 'string'); + }); + it('should NOT clear the resources', () => { assert.strictEqual( clearResourceTimingsSpy.args.length,