From 18b8644b36a9c805bf1f2a160aef8e2786be2a06 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 18:55:24 +0000 Subject: [PATCH 01/13] Renamed --- .../{CollectorExporterBase.ts => CollectorTraceExporterBase.ts} | 0 .../browser/{CollectorExporter.ts => CollectorTraceExporter.ts} | 2 +- .../src/platform/browser/index.ts | 2 +- .../node/{CollectorExporter.ts => CollectorTraceExporter.ts} | 2 +- .../opentelemetry-exporter-collector/src/platform/node/index.ts | 2 +- .../opentelemetry-exporter-collector/src/platform/node/protos | 2 +- packages/opentelemetry-exporter-collector/src/transform.ts | 2 +- ...CollectorExporter.test.ts => CollectorTraceExporter.test.ts} | 0 ...CollectorExporter.test.ts => CollectorTraceExporter.test.ts} | 2 +- ...CollectorExporter.test.ts => CollectorTraceExporter.test.ts} | 0 10 files changed, 7 insertions(+), 7 deletions(-) rename packages/opentelemetry-exporter-collector/src/{CollectorExporterBase.ts => CollectorTraceExporterBase.ts} (100%) rename packages/opentelemetry-exporter-collector/src/platform/browser/{CollectorExporter.ts => CollectorTraceExporter.ts} (98%) rename packages/opentelemetry-exporter-collector/src/platform/node/{CollectorExporter.ts => CollectorTraceExporter.ts} (98%) rename packages/opentelemetry-exporter-collector/test/browser/{CollectorExporter.test.ts => CollectorTraceExporter.test.ts} (100%) rename packages/opentelemetry-exporter-collector/test/common/{CollectorExporter.test.ts => CollectorTraceExporter.test.ts} (99%) rename packages/opentelemetry-exporter-collector/test/node/{CollectorExporter.test.ts => CollectorTraceExporter.test.ts} (100%) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts similarity index 100% rename from packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts rename to packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts similarity index 98% rename from packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts rename to packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts index 5e7f121ac62..b9afe7777b8 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts @@ -17,7 +17,7 @@ import { CollectorExporterBase, CollectorExporterConfigBase, -} from '../../CollectorExporterBase'; +} from '../../CollectorTraceExporterBase'; import { ReadableSpan } from '@opentelemetry/tracing'; import { toCollectorExportTraceServiceRequest } from '../../transform'; import * as collectorTypes from '../../types'; diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/index.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/index.ts index f38bc0489f6..1c17973de2e 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/index.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export * from './CollectorExporter'; +export * from './CollectorTraceExporter'; diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts similarity index 98% rename from packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts rename to packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts index e5c767ecf3a..c440c06105f 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts @@ -23,7 +23,7 @@ import { ReadableSpan } from '@opentelemetry/tracing'; import { CollectorExporterBase, CollectorExporterConfigBase, -} from '../../CollectorExporterBase'; +} from '../../CollectorTraceExporterBase'; import { CollectorExporterError } from '../../types'; import { toCollectorExportTraceServiceRequest } from '../../transform'; import { GRPCQueueItem, TraceServiceClient } from './types'; diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/index.ts b/packages/opentelemetry-exporter-collector/src/platform/node/index.ts index f38bc0489f6..1c17973de2e 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/index.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export * from './CollectorExporter'; +export * from './CollectorTraceExporter'; 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/transform.ts b/packages/opentelemetry-exporter-collector/src/transform.ts index 09af80dbf0a..2eb0ffe8fbf 100644 --- a/packages/opentelemetry-exporter-collector/src/transform.ts +++ b/packages/opentelemetry-exporter-collector/src/transform.ts @@ -27,7 +27,7 @@ import { ReadableSpan } from '@opentelemetry/tracing'; import { CollectorExporterBase, CollectorExporterConfigBase, -} from './CollectorExporterBase'; +} from './CollectorTraceExporterBase'; import { COLLECTOR_SPAN_KIND_MAPPING, opentelemetryProto } from './types'; import ValueType = opentelemetryProto.common.v1.ValueType; import { InstrumentationLibrary } from '@opentelemetry/core'; diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts similarity index 100% rename from packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts rename to packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts similarity index 99% rename from packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts rename to packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index e5d67d67db0..7bd74fc2e2b 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts @@ -21,7 +21,7 @@ import * as sinon from 'sinon'; import { CollectorExporterBase, CollectorExporterConfigBase, -} from '../../src/CollectorExporterBase'; +} from '../../src/CollectorTraceExporterBase'; import { mockedReadableSpan } from '../helper'; diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts similarity index 100% rename from packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts rename to packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts From b5bd5ca64c9f1a36f9c5f79797e3f576de3f0137 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 19:10:07 +0000 Subject: [PATCH 02/13] Completed renaming --- .../src/CollectorTraceExporterBase.ts | 21 ++--- .../browser/CollectorTraceExporter.ts | 19 ++--- .../platform/node/CollectorTraceExporter.ts | 35 ++++---- .../src/platform/node/types.ts | 6 +- .../src/transform.ts | 9 +- .../src/types.ts | 31 ++++++- .../browser/CollectorTraceExporter.test.ts | 82 ++++++++++++------- .../common/CollectorTraceExporter.test.ts | 30 +++---- .../test/node/CollectorTraceExporter.test.ts | 10 +-- 9 files changed, 139 insertions(+), 104 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts index 0e6e052e9d4..93ab2330db3 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts @@ -17,25 +17,18 @@ import { Attributes, Logger } from '@opentelemetry/api'; import { ExportResult, NoopLogger } from '@opentelemetry/core'; import { ReadableSpan, SpanExporter } from '@opentelemetry/tracing'; -import { opentelemetryProto, CollectorExporterError } from './types'; - -/** - * Collector Exporter base config - */ -export interface CollectorExporterConfigBase { - hostName?: string; - logger?: Logger; - serviceName?: string; - attributes?: Attributes; - url?: string; -} +import { + opentelemetryProto, + CollectorExporterError, + CollectorExporterConfigBase, +} from './types'; const DEFAULT_SERVICE_NAME = 'collector-exporter'; /** - * Collector Exporter abstract base class + * Collector Trace Exporter abstract base class */ -export abstract class CollectorExporterBase< +export abstract class CollectorTraceExporterBase< T extends CollectorExporterConfigBase > implements SpanExporter { public readonly serviceName: string; diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts index b9afe7777b8..2d0a337f2ba 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts @@ -14,28 +14,19 @@ * limitations under the License. */ -import { - CollectorExporterBase, - CollectorExporterConfigBase, -} from '../../CollectorTraceExporterBase'; +import { CollectorTraceExporterBase } from '../../CollectorTraceExporterBase'; import { ReadableSpan } from '@opentelemetry/tracing'; import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { CollectorExporterConfigBrowser } from '../../types'; import * as collectorTypes from '../../types'; -/** - * Collector Exporter Config for Web - */ -export interface CollectorExporterConfig extends CollectorExporterConfigBase { - headers?: { [key: string]: string }; -} - const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace'; /** * Collector Exporter for Web */ -export class CollectorExporter extends CollectorExporterBase< - CollectorExporterConfig +export class CollectorTraceExporter extends CollectorTraceExporterBase< + CollectorExporterConfigBrowser > { DEFAULT_HEADERS: { [key: string]: string } = { [collectorTypes.OT_REQUEST_HEADER]: '1', @@ -46,7 +37,7 @@ export class CollectorExporter extends CollectorExporterBase< /** * @param config */ - constructor(config: CollectorExporterConfig = {}) { + constructor(config: CollectorExporterConfigBrowser = {}) { super(config); this._headers = config.headers || this.DEFAULT_HEADERS; this._useXHR = diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts index c440c06105f..8659a97b718 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts @@ -20,40 +20,32 @@ import * as path from 'path'; import * as collectorTypes from '../../types'; import { ReadableSpan } from '@opentelemetry/tracing'; +import { CollectorTraceExporterBase } from '../../CollectorTraceExporterBase'; import { - CollectorExporterBase, - CollectorExporterConfigBase, -} from '../../CollectorTraceExporterBase'; -import { CollectorExporterError } from '../../types'; + CollectorExporterError, + CollectorExporterConfigNode, +} from '../../types'; import { toCollectorExportTraceServiceRequest } from '../../transform'; -import { GRPCQueueItem, TraceServiceClient } from './types'; +import { GRPCSpanQueueItem, ServiceClient } from './types'; import { removeProtocol } from './util'; const DEFAULT_COLLECTOR_URL = 'localhost:55678'; /** - * Collector Exporter Config for Node + * Collector Trace Exporter for Node */ -export interface CollectorExporterConfig extends CollectorExporterConfigBase { - credentials?: grpc.ChannelCredentials; - metadata?: grpc.Metadata; -} - -/** - * Collector Exporter for Node - */ -export class CollectorExporter extends CollectorExporterBase< - CollectorExporterConfig +export class CollectorTraceExporter extends CollectorTraceExporterBase< + CollectorExporterConfigNode > { isShutDown: boolean = false; - traceServiceClient?: TraceServiceClient = undefined; - grpcSpansQueue: GRPCQueueItem[] = []; + traceServiceClient?: ServiceClient = undefined; + grpcSpansQueue: GRPCSpanQueueItem[] = []; metadata?: grpc.Metadata; /** * @param config */ - constructor(config: CollectorExporterConfig = {}) { + constructor(config: CollectorExporterConfigNode = {}) { super(config); this.metadata = config.metadata; } @@ -65,7 +57,7 @@ export class CollectorExporter extends CollectorExporterBase< } } - onInit(config: CollectorExporterConfig): void { + onInit(config: CollectorExporterConfigNode): void { this.isShutDown = false; this.grpcSpansQueue = []; const serverAddress = removeProtocol(this.url); @@ -95,7 +87,7 @@ export class CollectorExporter extends CollectorExporterBase< ); if (this.grpcSpansQueue.length > 0) { const queue = this.grpcSpansQueue.splice(0); - queue.forEach((item: GRPCQueueItem) => { + queue.forEach((item: GRPCSpanQueueItem) => { this.sendSpans(item.spans, item.onSuccess, item.onError); }); } @@ -108,6 +100,7 @@ export class CollectorExporter extends CollectorExporterBase< onError: (error: CollectorExporterError) => void ): void { if (this.isShutDown) { + this.logger.debug('Shutdown already started. Cannot send spans'); return; } if (this.traceServiceClient) { diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/types.ts b/packages/opentelemetry-exporter-collector/src/platform/node/types.ts index 4674fadf908..01b1a9a33f2 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/types.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/types.ts @@ -22,16 +22,16 @@ import { CollectorExporterError } from '../../types'; * Queue item to be used to save temporary spans in case the GRPC service * hasn't been fully initialised yet */ -export interface GRPCQueueItem { +export interface GRPCSpanQueueItem { spans: ReadableSpan[]; onSuccess: () => void; onError: (error: CollectorExporterError) => void; } /** - * Trace Service Client for sending spans + * Service Client for sending spans or metrics */ -export interface TraceServiceClient extends grpc.Client { +export interface ServiceClient extends grpc.Client { export: ( request: any, metadata: grpc.Metadata | undefined, diff --git a/packages/opentelemetry-exporter-collector/src/transform.ts b/packages/opentelemetry-exporter-collector/src/transform.ts index 2eb0ffe8fbf..626169d0777 100644 --- a/packages/opentelemetry-exporter-collector/src/transform.ts +++ b/packages/opentelemetry-exporter-collector/src/transform.ts @@ -24,11 +24,12 @@ import { import * as core from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { ReadableSpan } from '@opentelemetry/tracing'; +import { CollectorTraceExporterBase } from './CollectorTraceExporterBase'; import { - CollectorExporterBase, + COLLECTOR_SPAN_KIND_MAPPING, + opentelemetryProto, CollectorExporterConfigBase, -} from './CollectorTraceExporterBase'; -import { COLLECTOR_SPAN_KIND_MAPPING, opentelemetryProto } from './types'; +} from './types'; import ValueType = opentelemetryProto.common.v1.ValueType; import { InstrumentationLibrary } from '@opentelemetry/core'; @@ -200,7 +201,7 @@ export function toCollectorExportTraceServiceRequest< T extends CollectorExporterConfigBase >( spans: ReadableSpan[], - collectorExporterBase: CollectorExporterBase, + collectorExporterBase: CollectorTraceExporterBase, name = '' ): opentelemetryProto.collector.trace.v1.ExportTraceServiceRequest { const groupedSpans: Map< diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index ea10b0ca4bd..aca9f6dfb34 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -14,8 +14,9 @@ * limitations under the License. */ -import { SpanKind } from '@opentelemetry/api'; +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'; @@ -170,6 +171,34 @@ export interface CollectorExporterError { stack?: string; } +/** + * Collector Exporter base config + */ +export interface CollectorExporterConfigBase { + hostName?: string; + logger?: Logger; + serviceName?: string; + attributes?: Attributes; + 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 265bf441366..b5621a3ee55 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts @@ -18,10 +18,7 @@ import { NoopLogger } from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/tracing'; import * as assert from 'assert'; import * as sinon from 'sinon'; -import { - CollectorExporter, - CollectorExporterConfig, -} from '../../src/platform/browser/index'; +import { CollectorTraceExporter } from '../../src/platform/browser/index'; import * as collectorTypes from '../../src/types'; import { @@ -34,8 +31,8 @@ import { const sendBeacon = navigator.sendBeacon; describe('CollectorExporter - web', () => { - let collectorExporter: CollectorExporter; - let collectorExporterConfig: CollectorExporterConfig; + let collectorTraceExporter: CollectorTraceExporter; + let collectorExporterConfig: collectorTypes.CollectorExporterConfigBrowser; let spyOpen: any; let spySend: any; let spyBeacon: any; @@ -69,11 +66,13 @@ describe('CollectorExporter - web', () => { describe('when "sendBeacon" is available', () => { beforeEach(() => { - collectorExporter = new CollectorExporter(collectorExporterConfig); + collectorTraceExporter = new CollectorTraceExporter( + collectorExporterConfig + ); }); it('should successfully send the spans using sendBeacon', done => { - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const args = spyBeacon.args[0]; @@ -108,12 +107,18 @@ describe('CollectorExporter - web', () => { }); it('should log the successful message', done => { - const spyLoggerDebug = sinon.stub(collectorExporter.logger, 'debug'); - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerDebug = sinon.stub( + collectorTraceExporter.logger, + 'debug' + ); + const spyLoggerError = sinon.stub( + collectorTraceExporter.logger, + 'error' + ); spyBeacon.restore(); spyBeacon = sinon.stub(window.navigator, 'sendBeacon').returns(true); - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const response: any = spyLoggerDebug.args[1][0]; @@ -125,12 +130,18 @@ describe('CollectorExporter - web', () => { }); it('should log the error message', done => { - const spyLoggerDebug = sinon.stub(collectorExporter.logger, 'debug'); - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerDebug = sinon.stub( + collectorTraceExporter.logger, + 'debug' + ); + const spyLoggerError = sinon.stub( + collectorTraceExporter.logger, + 'error' + ); spyBeacon.restore(); spyBeacon = sinon.stub(window.navigator, 'sendBeacon').returns(false); - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const response: any = spyLoggerError.args[0][0]; @@ -146,7 +157,9 @@ describe('CollectorExporter - web', () => { let server: any; beforeEach(() => { (window.navigator as any).sendBeacon = false; - collectorExporter = new CollectorExporter(collectorExporterConfig); + collectorTraceExporter = new CollectorTraceExporter( + collectorExporterConfig + ); server = sinon.fakeServer.create(); }); afterEach(() => { @@ -154,7 +167,7 @@ describe('CollectorExporter - web', () => { }); it('should successfully send the spans using XMLHttpRequest', done => { - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const request = server.requests[0]; @@ -188,10 +201,16 @@ describe('CollectorExporter - web', () => { }); it('should log the successful message', done => { - const spyLoggerDebug = sinon.stub(collectorExporter.logger, 'debug'); - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerDebug = sinon.stub( + collectorTraceExporter.logger, + 'debug' + ); + const spyLoggerError = sinon.stub( + collectorTraceExporter.logger, + 'error' + ); - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const request = server.requests[0]; @@ -207,9 +226,12 @@ describe('CollectorExporter - web', () => { }); it('should log the error message', done => { - const spyLoggerError = sinon.stub(collectorExporter.logger, 'error'); + const spyLoggerError = sinon.stub( + collectorTraceExporter.logger, + 'error' + ); - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const request = server.requests[0]; @@ -226,7 +248,7 @@ describe('CollectorExporter - web', () => { }); it('should send custom headers', done => { - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const request = server.requests[0]; @@ -260,10 +282,12 @@ describe('CollectorExporter - web', () => { describe('when "sendBeacon" is available', () => { beforeEach(() => { - collectorExporter = new CollectorExporter(collectorExporterConfig); + collectorTraceExporter = new CollectorTraceExporter( + collectorExporterConfig + ); }); it('should successfully send custom headers using XMLHTTPRequest', done => { - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const [{ requestHeaders }] = server.requests; @@ -280,11 +304,13 @@ describe('CollectorExporter - web', () => { describe('when "sendBeacon" is NOT available', () => { beforeEach(() => { (window.navigator as any).sendBeacon = false; - collectorExporter = new CollectorExporter(collectorExporterConfig); + collectorTraceExporter = new CollectorTraceExporter( + collectorExporterConfig + ); }); it('should successfully send spans using XMLHttpRequest', done => { - collectorExporter.export(spans, () => {}); + collectorTraceExporter.export(spans, () => {}); setTimeout(() => { const [{ requestHeaders }] = server.requests; @@ -302,7 +328,7 @@ describe('CollectorExporter - web', () => { describe('CollectorExporter - browser (getDefaultUrl)', () => { it('should default to v1/trace', done => { - const collectorExporter = new CollectorExporter({}); + const collectorExporter = new CollectorTraceExporter({}); setTimeout(() => { assert.strictEqual( collectorExporter['url'], @@ -313,7 +339,7 @@ describe('CollectorExporter - browser (getDefaultUrl)', () => { }); it('should keep the URL if included', done => { const url = 'http://foo.bar.com'; - const collectorExporter = new CollectorExporter({ url }); + const collectorExporter = new CollectorTraceExporter({ url }); setTimeout(() => { assert.strictEqual(collectorExporter['url'], url); done(); diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index 7bd74fc2e2b..71ccee1c1c2 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts @@ -18,15 +18,14 @@ import { ExportResult, NoopLogger } from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/tracing'; import * as assert from 'assert'; import * as sinon from 'sinon'; -import { - CollectorExporterBase, - CollectorExporterConfigBase, -} from '../../src/CollectorTraceExporterBase'; - +import { CollectorTraceExporterBase } from '../../src/CollectorTraceExporterBase'; +import { CollectorExporterConfigBase } from '../../src/types'; import { mockedReadableSpan } from '../helper'; type CollectorExporterConfig = CollectorExporterConfigBase; -class CollectorExporter extends CollectorExporterBase { +class CollectorTraceExporter extends CollectorTraceExporterBase< + CollectorExporterConfig +> { onInit() {} onShutdown() {} sendSpans() {} @@ -36,14 +35,14 @@ class CollectorExporter extends CollectorExporterBase { } describe('CollectorExporter - common', () => { - let collectorExporter: CollectorExporter; + let collectorExporter: CollectorTraceExporter; let collectorExporterConfig: CollectorExporterConfig; describe('constructor', () => { let onInitSpy: any; beforeEach(() => { - onInitSpy = sinon.stub(CollectorExporter.prototype, 'onInit'); + onInitSpy = sinon.stub(CollectorTraceExporter.prototype, 'onInit'); collectorExporterConfig = { hostName: 'foo', logger: new NoopLogger(), @@ -51,7 +50,7 @@ describe('CollectorExporter - common', () => { attributes: {}, url: 'http://foo.bar.com', }; - collectorExporter = new CollectorExporter(collectorExporterConfig); + collectorExporter = new CollectorTraceExporter(collectorExporterConfig); }); afterEach(() => { @@ -86,7 +85,7 @@ describe('CollectorExporter - common', () => { describe('when config is missing certain params', () => { beforeEach(() => { - collectorExporter = new CollectorExporter(); + collectorExporter = new CollectorTraceExporter(); }); it('should set default serviceName', () => { @@ -102,8 +101,8 @@ describe('CollectorExporter - common', () => { describe('export', () => { let spySend: any; beforeEach(() => { - spySend = sinon.stub(CollectorExporter.prototype, 'sendSpans'); - collectorExporter = new CollectorExporter(collectorExporterConfig); + spySend = sinon.stub(CollectorTraceExporter.prototype, 'sendSpans'); + collectorExporter = new CollectorTraceExporter(collectorExporterConfig); }); afterEach(() => { spySend.restore(); @@ -146,7 +145,10 @@ describe('CollectorExporter - common', () => { describe('shutdown', () => { let onShutdownSpy: any; beforeEach(() => { - onShutdownSpy = sinon.stub(CollectorExporter.prototype, 'onShutdown'); + onShutdownSpy = sinon.stub( + CollectorTraceExporter.prototype, + 'onShutdown' + ); collectorExporterConfig = { hostName: 'foo', logger: new NoopLogger(), @@ -154,7 +156,7 @@ describe('CollectorExporter - common', () => { attributes: {}, url: 'http://foo.bar.com', }; - collectorExporter = new CollectorExporter(collectorExporterConfig); + collectorExporter = new CollectorTraceExporter(collectorExporterConfig); }); afterEach(() => { onShutdownSpy.restore(); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts index 13da3284a2d..aaa534986ac 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts @@ -25,7 +25,7 @@ import { import * as assert from 'assert'; import * as sinon from 'sinon'; -import { CollectorExporter } from '../../src/platform/node'; +import { CollectorTraceExporter } from '../../src/platform/node'; import * as collectorTypes from '../../src/types'; import { @@ -53,7 +53,7 @@ const testCollectorExporter = (params: TestParams) => describe(`CollectorExporter - node ${ params.useTLS ? 'with' : 'without' } TLS, ${params.metadata ? 'with' : 'without'} metadata`, () => { - let collectorExporter: CollectorExporter; + let collectorExporter: CollectorTraceExporter; let server: grpc.Server; let exportedData: | collectorTypes.opentelemetryProto.trace.v1.ResourceSpans @@ -121,7 +121,7 @@ const testCollectorExporter = (params: TestParams) => fs.readFileSync('./test/certs/client.crt') ) : undefined; - collectorExporter = new CollectorExporter({ + collectorExporter = new CollectorTraceExporter({ serviceName: 'basic-service', url: address, credentials, @@ -174,7 +174,7 @@ const testCollectorExporter = (params: TestParams) => describe('CollectorExporter - node (getDefaultUrl)', () => { it('should default to localhost', done => { - const collectorExporter = new CollectorExporter({}); + const collectorExporter = new CollectorTraceExporter({}); setTimeout(() => { assert.strictEqual(collectorExporter['url'], 'localhost:55678'); done(); @@ -182,7 +182,7 @@ describe('CollectorExporter - node (getDefaultUrl)', () => { }); it('should keep the URL if included', done => { const url = 'http://foo.bar.com'; - const collectorExporter = new CollectorExporter({ url }); + const collectorExporter = new CollectorTraceExporter({ url }); setTimeout(() => { assert.strictEqual(collectorExporter['url'], url); done(); From 955aea2bbe74f42252f9521ffdc7d78acfcee60d Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 20:26:53 +0000 Subject: [PATCH 03/13] Added documentation --- examples/collector-exporter-node/start.js | 4 ++-- .../examples/document-load/index.js | 4 ++-- examples/tracer-web/examples/fetch/index.js | 4 ++-- .../examples/user-interaction/index.js | 4 ++-- .../examples/xml-http-request/index.js | 4 ++-- .../README.md | 20 +++++++++---------- .../src/platform/node/README.md | 2 +- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/examples/collector-exporter-node/start.js b/examples/collector-exporter-node/start.js index 9acbcc06870..5ff85ec1237 100644 --- a/examples/collector-exporter-node/start.js +++ b/examples/collector-exporter-node/start.js @@ -2,10 +2,10 @@ const opentelemetry = require('@opentelemetry/api'); const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); -const { CollectorExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector'); const address = '127.0.0.1:55678'; -const exporter = new CollectorExporter({ +const exporter = new CollectorTraceExporter({ serviceName: 'basic-service', url: address, }); diff --git a/examples/tracer-web/examples/document-load/index.js b/examples/tracer-web/examples/document-load/index.js index 4228e8730b2..fe72746a5ef 100644 --- a/examples/tracer-web/examples/document-load/index.js +++ b/examples/tracer-web/examples/document-load/index.js @@ -2,7 +2,7 @@ import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing import { WebTracerProvider } from '@opentelemetry/web'; import { DocumentLoad } from '@opentelemetry/plugin-document-load'; import { ZoneContextManager } from '@opentelemetry/context-zone'; -import { CollectorExporter } from '@opentelemetry/exporter-collector'; +import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; const provider = new WebTracerProvider({ plugins: [ @@ -10,7 +10,7 @@ const provider = new WebTracerProvider({ ], }); provider.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter())); -provider.addSpanProcessor(new SimpleSpanProcessor(new CollectorExporter())); +provider.addSpanProcessor(new SimpleSpanProcessor(new CollectorTraceExporter())); provider.register({ contextManager: new ZoneContextManager(), diff --git a/examples/tracer-web/examples/fetch/index.js b/examples/tracer-web/examples/fetch/index.js index 5be984bbbe8..1d29b654752 100644 --- a/examples/tracer-web/examples/fetch/index.js +++ b/examples/tracer-web/examples/fetch/index.js @@ -1,7 +1,7 @@ 'use strict'; import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing'; -import { CollectorExporter } from '@opentelemetry/exporter-collector'; +import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; import { WebTracerProvider } from '@opentelemetry/web'; import { FetchPlugin } from '@opentelemetry/plugin-fetch'; import { ZoneContextManager } from '@opentelemetry/context-zone'; @@ -21,7 +21,7 @@ const provider = new WebTracerProvider({ }); provider.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter())); -provider.addSpanProcessor(new SimpleSpanProcessor(new CollectorExporter())); +provider.addSpanProcessor(new SimpleSpanProcessor(new CollectorTraceExporter())); provider.register({ contextManager: new ZoneContextManager(), propagator: new B3Propagator(), diff --git a/examples/tracer-web/examples/user-interaction/index.js b/examples/tracer-web/examples/user-interaction/index.js index a8b67488f69..b15f85a99e4 100644 --- a/examples/tracer-web/examples/user-interaction/index.js +++ b/examples/tracer-web/examples/user-interaction/index.js @@ -3,7 +3,7 @@ import { WebTracerProvider } from '@opentelemetry/web'; import { XMLHttpRequestPlugin } from '@opentelemetry/plugin-xml-http-request'; import { UserInteractionPlugin } from '@opentelemetry/plugin-user-interaction'; import { ZoneContextManager } from '@opentelemetry/context-zone'; -import { CollectorExporter } from '@opentelemetry/exporter-collector'; +import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; import { B3Propagator } from '@opentelemetry/core'; const providerWithZone = new WebTracerProvider({ @@ -19,7 +19,7 @@ const providerWithZone = new WebTracerProvider({ }); providerWithZone.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter())); -providerWithZone.addSpanProcessor(new SimpleSpanProcessor(new CollectorExporter())); +providerWithZone.addSpanProcessor(new SimpleSpanProcessor(new CollectorTraceExporter())); providerWithZone.register({ contextManager: new ZoneContextManager(), diff --git a/examples/tracer-web/examples/xml-http-request/index.js b/examples/tracer-web/examples/xml-http-request/index.js index 7f025ea14bf..ce22cec2c52 100644 --- a/examples/tracer-web/examples/xml-http-request/index.js +++ b/examples/tracer-web/examples/xml-http-request/index.js @@ -2,7 +2,7 @@ import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing import { WebTracerProvider } from '@opentelemetry/web'; import { XMLHttpRequestPlugin } from '@opentelemetry/plugin-xml-http-request'; import { ZoneContextManager } from '@opentelemetry/context-zone'; -import { CollectorExporter } from '@opentelemetry/exporter-collector'; +import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; import { B3Propagator } from '@opentelemetry/core'; const providerWithZone = new WebTracerProvider({ @@ -17,7 +17,7 @@ const providerWithZone = new WebTracerProvider({ }); providerWithZone.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter())); -providerWithZone.addSpanProcessor(new SimpleSpanProcessor(new CollectorExporter())); +providerWithZone.addSpanProcessor(new SimpleSpanProcessor(new CollectorTraceExporter())); providerWithZone.register({ contextManager: new ZoneContextManager(), diff --git a/packages/opentelemetry-exporter-collector/README.md b/packages/opentelemetry-exporter-collector/README.md index 6a8b8280567..a3d0ecd6a39 100644 --- a/packages/opentelemetry-exporter-collector/README.md +++ b/packages/opentelemetry-exporter-collector/README.md @@ -16,12 +16,12 @@ npm install --save @opentelemetry/exporter-collector ## Usage in Web -The CollectorExporter in Web expects the endpoint to end in `/v1/trace`. +The CollectorTraceExporter in Web expects the endpoint to end in `/v1/trace`. ```js import { SimpleSpanProcessor } from '@opentelemetry/tracing'; import { WebTracerProvider } from '@opentelemetry/web'; -import { CollectorExporter } from '@opentelemetry/exporter-collector'; +import { CollectorTraceExporter } from '@opentelemetry/exporter-collector'; const collectorOptions = { url: '', // url is optional and can be omitted - default is http://localhost:55678/v1/trace @@ -29,7 +29,7 @@ const collectorOptions = { }; const provider = new WebTracerProvider(); -const exporter = new CollectorExporter(collectorOptions); +const exporter = new CollectorTraceExporter(collectorOptions); provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); provider.register(); @@ -38,11 +38,11 @@ provider.register(); ## Usage in Node -The CollectorExporter in Node expects the URL to only be the hostname. It will not work with `/v1/trace`. +The CollectorTraceExporter in Node expects the URL to only be the hostname. It will not work with `/v1/trace`. ```js const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); -const { CollectorExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector'); const collectorOptions = { serviceName: 'basic-service', @@ -50,7 +50,7 @@ const collectorOptions = { }; const provider = new BasicTracerProvider(); -const exporter = new CollectorExporter(collectorOptions); +const exporter = new CollectorTraceExporter(collectorOptions); provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); provider.register(); @@ -63,7 +63,7 @@ By default, plaintext connection is used. In order to use TLS in Node.js, provid const fs = require('fs'); const grpc = require('grpc'); const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); -const { CollectorExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector'); const collectorOptions = { serviceName: 'basic-service', @@ -76,7 +76,7 @@ const collectorOptions = { }; const provider = new BasicTracerProvider(); -const exporter = new CollectorExporter(collectorOptions); +const exporter = new CollectorTraceExporter(collectorOptions); provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); provider.register(); @@ -89,7 +89,7 @@ The exporter can be configured to send custom metadata with each request as in t ```js const grpc = require('grpc'); const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); -const { CollectorExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector'); const metadata = new grpc.Metadata(); metadata.set('k', 'v'); @@ -101,7 +101,7 @@ const collectorOptions = { }; const provider = new BasicTracerProvider(); -const exporter = new CollectorExporter(collectorOptions); +const exporter = new CollectorTraceExporter(collectorOptions); provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); provider.register(); diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/README.md b/packages/opentelemetry-exporter-collector/src/platform/node/README.md index edb0ab60a23..76ff2c2c492 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/README.md +++ b/packages/opentelemetry-exporter-collector/src/platform/node/README.md @@ -15,7 +15,7 @@ Knowing this if you want to change the submodule to point to a different version ``` 3. Find the SHA which you want to update to and copy it (the long one) -the latest sha when this guide was written is `e6c3c4a74d57f870a0d781bada02cb2b2c497d14` +the latest sha when this guide was written is `b54688569186e0b862bf7462a983ccf2c50c0547` 4. Enter a submodule directory from this directory From 7ce4a77e9a611ad22c0cdf1f8624b6e3444f243d Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 21:42:43 +0000 Subject: [PATCH 04/13] Metric Base --- .../src/CollectorMetricExporterBase.ts | 122 ++++++++++++ .../common/CollectorMetricExporter.test.ts | 176 ++++++++++++++++++ .../test/helper.ts | 44 ++++- 3 files changed, 341 insertions(+), 1 deletion(-) 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/src/CollectorMetricExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts new file mode 100644 index 00000000000..8be812cf5f9 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts @@ -0,0 +1,122 @@ +/* + * 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.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); + } + } + ); + } + + 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; +} \ No newline at end of file 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..f1c62d8b542 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -0,0 +1,176 @@ +/* + * 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('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(); + }); + }); + }); +}); \ No newline at end of file diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index 056d95d2a7b..f50e7b4c761 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, + CounterSumAggregator, + ObserverAggregator, +} 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 CounterSumAggregator(), + 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.OBSERVER, + valueType: ValueType.DOUBLE, + }, + labels: {}, + aggregator: new ObserverAggregator(), + 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='; From 4f64aa86536486aade112c003010b91791ae5e4d Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 21:45:48 +0000 Subject: [PATCH 05/13] Add newline --- .../src/CollectorMetricExporterBase.ts | 2 +- .../test/common/CollectorMetricExporter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts index 8be812cf5f9..78ae03d1fb8 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts @@ -119,4 +119,4 @@ export abstract class CollectorMetricExporterBase< onSuccess: () => void, onError: (error: collectorTypes.CollectorExporterError) => void ): void; -} \ No newline at end of file +} diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts index f1c62d8b542..fdfa1e76ea7 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -173,4 +173,4 @@ describe('CollectorMetricExporter - common', () => { }); }); }); -}); \ No newline at end of file +}); From c0201a24c96929de364bda0d4fa8225cd6c8e512 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 22:25:34 +0000 Subject: [PATCH 06/13] Add metrics package --- packages/opentelemetry-exporter-collector/package.json | 1 + 1 file changed, 1 insertion(+) 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" From 99ff4a65bbe371b37fd0c3615df8b8412a038d9d Mon Sep 17 00:00:00 2001 From: davidwitten Date: Mon, 29 Jun 2020 22:37:12 +0000 Subject: [PATCH 07/13] Updated proto --- .../opentelemetry-exporter-collector/src/platform/node/protos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From eea2bfc36daf16e129dc618b8578f80909d24e2e Mon Sep 17 00:00:00 2001 From: davidwitten Date: Tue, 30 Jun 2020 16:40:56 +0000 Subject: [PATCH 08/13] Rename type --- .../src/CollectorMetricExporterBase.ts | 22 +++++++---------- .../src/CollectorTraceExporterBase.ts | 24 ++++++++----------- .../platform/node/CollectorTraceExporter.ts | 4 +--- .../src/types.ts | 19 ++++++++------- 4 files changed, 31 insertions(+), 38 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts index 78ae03d1fb8..1c06aea0cd9 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts @@ -67,20 +67,16 @@ export abstract class CollectorMetricExporterBase< .then(() => { resultCallback(ExportResult.SUCCESS); }) - .catch( - ( - error: collectorTypes.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: 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 { diff --git a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts index 93ab2330db3..80961fd485d 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'; @@ -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/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index aca9f6dfb34..f0cd3141609 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,6 +163,17 @@ 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 */ From f70ca7039e2c3dc7209a50e9e71febbd74583753 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Tue, 30 Jun 2020 16:41:49 +0000 Subject: [PATCH 09/13] Fixed nit --- .../src/CollectorMetricExporterBase.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts index 1c06aea0cd9..743c6e3c80b 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts @@ -13,6 +13,7 @@ * 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'; From 0aa85021d4652f462f47d9e88a6b4e533926a7e9 Mon Sep 17 00:00:00 2001 From: davidwitten Date: Wed, 1 Jul 2020 15:54:03 +0000 Subject: [PATCH 10/13] Changed aggregator names --- .../opentelemetry-exporter-collector/test/helper.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index f50e7b4c761..4e57a6edbbf 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -25,8 +25,8 @@ import * as grpc from 'grpc'; import { MetricRecord, MetricKind, - CounterSumAggregator, - ObserverAggregator, + SumAggregator, + LastValueAggregator, } from '@opentelemetry/metrics'; if (typeof Buffer === 'undefined') { @@ -67,7 +67,7 @@ export const mockCounter: MetricRecord = { valueType: ValueType.INT, }, labels: {}, - aggregator: new CounterSumAggregator(), + aggregator: new SumAggregator(), resource: new Resource({ service: 'ui', version: 1, @@ -81,11 +81,11 @@ export const mockObserver: MetricRecord = { name: 'test-observer', description: 'sample observer description', unit: '2', - metricKind: MetricKind.OBSERVER, + metricKind: MetricKind.VALUE_OBSERVER, valueType: ValueType.DOUBLE, }, labels: {}, - aggregator: new ObserverAggregator(), + aggregator: new LastValueAggregator(), resource: new Resource({ service: 'ui', version: 1, From 6f8a80629c8334f5b14642860bcdcf8125c36ccd Mon Sep 17 00:00:00 2001 From: davidwitten Date: Thu, 2 Jul 2020 04:29:35 +0000 Subject: [PATCH 11/13] Changed hostname and added tests --- .../src/CollectorMetricExporterBase.ts | 6 +- .../src/CollectorTraceExporterBase.ts | 6 +- .../src/types.ts | 2 +- .../browser/CollectorTraceExporter.test.ts | 2 +- .../common/CollectorMetricExporter.test.ts | 52 ++++++++++++++++-- .../common/CollectorTraceExporter.test.ts | 55 ++++++++++++++++++- 6 files changed, 108 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts index 743c6e3c80b..7580e5f8a73 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorMetricExporterBase.ts @@ -31,7 +31,7 @@ export abstract class CollectorMetricExporterBase< 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; protected readonly _startTime = new Date().getTime() * 1000000; private _isShutdown: boolean = false; @@ -44,8 +44,8 @@ export abstract class CollectorMetricExporterBase< 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; + if (typeof config.hostname === 'string') { + this.hostname = config.hostname; } this.onInit(); } diff --git a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts index 80961fd485d..4695c439940 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorTraceExporterBase.ts @@ -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; diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index f0cd3141609..627cb736a84 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -178,7 +178,7 @@ export interface ExportServiceError { * 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..1cf7739d3c3 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts @@ -56,7 +56,7 @@ describe('CollectorExporter - web', () => { describe('export', () => { beforeEach(() => { collectorExporterConfig = { - hostName: 'foo', + hostname: 'foo', logger: new NoopLogger(), serviceName: 'bar', attributes: {}, diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts index fdfa1e76ea7..2c18830556d 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -44,7 +44,7 @@ describe('CollectorMetricExporter - common', () => { beforeEach(() => { onInitSpy = sinon.stub(CollectorMetricExporter.prototype, 'onInit'); collectorExporterConfig = { - hostName: 'foo', + hostname: 'foo', logger: new NoopLogger(), serviceName: 'bar', attributes: {}, @@ -70,7 +70,7 @@ describe('CollectorMetricExporter - common', () => { describe('when config contains certain params', () => { it('should set hostName', () => { - assert.strictEqual(collectorExporter.hostName, 'foo'); + assert.strictEqual(collectorExporter.hostname, 'foo'); }); it('should set serviceName', () => { @@ -134,7 +134,6 @@ describe('CollectorMetricExporter - common', () => { const callbackSpy = sinon.spy(); collectorExporter.export(metrics, callbackSpy); const returnCode = callbackSpy.args[0][0]; - assert.strictEqual( returnCode, ExportResult.FAILED_NOT_RETRYABLE, @@ -143,6 +142,51 @@ describe('CollectorMetricExporter - common', () => { 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', () => { @@ -153,7 +197,7 @@ describe('CollectorMetricExporter - common', () => { 'onShutdown' ); collectorExporterConfig = { - hostName: 'foo', + hostname: 'foo', logger: new NoopLogger(), serviceName: 'bar', attributes: {}, diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index 71ccee1c1c2..c20cbed7cfc 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts @@ -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: {}, @@ -67,7 +67,7 @@ describe('CollectorExporter - common', () => { describe('when config contains certain params', () => { it('should set hostName', () => { - assert.strictEqual(collectorExporter.hostName, 'foo'); + 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: {}, From ceda0ae77d7c32d6d406be9821e3bd2944b252cb Mon Sep 17 00:00:00 2001 From: davidwitten Date: Thu, 2 Jul 2020 04:32:13 +0000 Subject: [PATCH 12/13] forgot to replace 2 --- .../test/common/CollectorMetricExporter.test.ts | 2 +- .../test/common/CollectorTraceExporter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts index 2c18830556d..73e9ddc40eb 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorMetricExporter.test.ts @@ -69,7 +69,7 @@ describe('CollectorMetricExporter - common', () => { }); describe('when config contains certain params', () => { - it('should set hostName', () => { + it('should set hostname', () => { assert.strictEqual(collectorExporter.hostname, 'foo'); }); diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index c20cbed7cfc..3e403693297 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts @@ -66,7 +66,7 @@ describe('CollectorExporter - common', () => { }); describe('when config contains certain params', () => { - it('should set hostName', () => { + it('should set hostname', () => { assert.strictEqual(collectorExporter.hostname, 'foo'); }); From 3ce9ebc63576132c3ab93824665cb94cadcda2ad Mon Sep 17 00:00:00 2001 From: davidwitten Date: Fri, 3 Jul 2020 17:45:18 +0000 Subject: [PATCH 13/13] Rename tests --- .../test/browser/CollectorTraceExporter.test.ts | 4 ++-- .../test/common/CollectorTraceExporter.test.ts | 2 +- .../test/node/CollectorTraceExporter.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts index 1cf7739d3c3..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; @@ -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/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorTraceExporter.test.ts index 3e403693297..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; 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(() => {