From 6bcd7c0f2741ce5adef6fa5a258ab6605d3fc5eb Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 25 Jun 2020 23:43:45 +0200 Subject: [PATCH 01/13] feat: adding json over http for collector exporter --- examples/collector-exporter-node/package.json | 1 + examples/collector-exporter-node/start.js | 5 +- .../src/CollectorExporterBase.ts | 4 +- .../src/platform/browser/CollectorExporter.ts | 4 +- .../src/platform/node/CollectorExporter.ts | 105 ++++------- .../src/platform/node/utilWithGrpc.ts | 107 +++++++++++ .../src/platform/node/utilWithJson.ts | 86 +++++++++ .../src/transform.ts | 2 +- .../test/common/CollectorExporter.test.ts | 4 +- .../node/CollectorExporterWithJson.test.ts | 172 ++++++++++++++++++ 10 files changed, 410 insertions(+), 80 deletions(-) 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/test/node/CollectorExporterWithJson.test.ts diff --git a/examples/collector-exporter-node/package.json b/examples/collector-exporter-node/package.json index a10b08b64b..2950b71a3d 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 9acbcc0687..28364905d7 100644 --- a/examples/collector-exporter-node/start.js +++ b/examples/collector-exporter-node/start.js @@ -2,12 +2,13 @@ const opentelemetry = require('@opentelemetry/api'); const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); +const { ConsoleLogger, LogLevel } = require('@opentelemetry/core'); const { CollectorExporter } = require('@opentelemetry/exporter-collector'); -const address = '127.0.0.1:55678'; const exporter = new CollectorExporter({ + logger: new ConsoleLogger(LogLevel.DEBUG), serviceName: 'basic-service', - url: address, + // useJson: true, }); const provider = new BasicTracerProvider(); diff --git a/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts index 0e6e052e9d..332a82b0e3 100644 --- a/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts +++ b/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts @@ -50,7 +50,7 @@ export abstract class CollectorExporterBase< */ 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; } @@ -134,5 +134,5 @@ export abstract class CollectorExporterBase< 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/platform/browser/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts index 5e7f121ac6..afbd742c3f 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts @@ -61,8 +61,8 @@ export class CollectorExporter extends CollectorExporterBase< window.removeEventListener('unload', this.shutdown); } - getDefaultUrl(url: string | undefined) { - return url || DEFAULT_COLLECTOR_URL; + getDefaultUrl(config: CollectorExporterConfig) { + return config.url || DEFAULT_COLLECTOR_URL; } sendSpans( diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index e5c767ecf3..9fc27d2ebf 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -14,22 +14,24 @@ * 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 * as grpc from 'grpc'; import { CollectorExporterBase, CollectorExporterConfigBase, } from '../../CollectorExporterBase'; import { CollectorExporterError } from '../../types'; -import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { + DEFAULT_COLLECTOR_URL_GRPC, + onInitWithGrpc, + sendSpansUsingGrpc, +} from './utilWithGrpc'; +import { + DEFAULT_COLLECTOR_URL_JSON, + onInitWithJson, + sendSpansUsingJson, +} from './utilWithJson'; import { GRPCQueueItem, TraceServiceClient } from './types'; -import { removeProtocol } from './util'; - -const DEFAULT_COLLECTOR_URL = 'localhost:55678'; /** * Collector Exporter Config for Node @@ -37,6 +39,7 @@ const DEFAULT_COLLECTOR_URL = 'localhost:55678'; export interface CollectorExporterConfig extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; metadata?: grpc.Metadata; + useJson?: boolean; } /** @@ -49,12 +52,19 @@ export class CollectorExporter extends CollectorExporterBase< traceServiceClient?: TraceServiceClient = undefined; grpcSpansQueue: GRPCQueueItem[] = []; metadata?: grpc.Metadata; + private readonly _useJson: boolean = false; /** * @param config */ constructor(config: CollectorExporterConfig = {}) { super(config); + this._useJson = !!config.useJson; + if (this._useJson) { + this.logger.debug('CollectorExporter - using json over http'); + } else { + this.logger.debug('CollectorExporter - using grpc'); + } this.metadata = config.metadata; } @@ -67,39 +77,12 @@ export class CollectorExporter extends CollectorExporterBase< onInit(config: CollectorExporterConfig): 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: GRPCQueueItem) => { - this.sendSpans(item.spans, item.onSuccess, item.onError); - }); - } - }); + if (config.useJson) { + onInitWithJson(this, config); + } else { + onInitWithGrpc(this, config); + } } sendSpans( @@ -110,39 +93,19 @@ export class CollectorExporter extends CollectorExporterBase< if (this.isShutDown) { return; } - if (this.traceServiceClient) { - const exportTraceServiceRequest = toCollectorExportTraceServiceRequest( - spans, - this - ); - - this.traceServiceClient.export( - exportTraceServiceRequest, - this.metadata, - ( - err: collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceError - ) => { - if (err) { - this.logger.error( - 'exportTraceServiceRequest', - exportTraceServiceRequest - ); - onError(err); - } else { - onSuccess(); - } - } - ); + if (this._useJson) { + 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: CollectorExporterConfig): string { + if (!config.url) { + return config.useJson + ? DEFAULT_COLLECTOR_URL_JSON + : DEFAULT_COLLECTOR_URL_GRPC; + } + return config.url; } } 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 0000000000..79b150aeab --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts @@ -0,0 +1,107 @@ +/* + * 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 { CollectorExporterError } from '../../types'; +import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { + CollectorExporter, + CollectorExporterConfig, +} from './CollectorExporter'; +import { GRPCQueueItem } from './types'; +import { removeProtocol } from './util'; + +export const DEFAULT_COLLECTOR_URL_GRPC = 'localhost:55678'; + +export function onInitWithGrpc( + collector: CollectorExporter, + config: CollectorExporterConfig +): 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: GRPCQueueItem) => { + collector.sendSpans(item.spans, item.onSuccess, item.onError); + }); + } + }); +} + +export function sendSpansUsingGrpc( + collector: CollectorExporter, + spans: ReadableSpan[], + onSuccess: () => void, + onError: (error: CollectorExporterError) => void +): void { + if (collector.traceServiceClient) { + const exportTraceServiceRequest = toCollectorExportTraceServiceRequest( + spans, + collector + ); + collector.traceServiceClient.export( + exportTraceServiceRequest, + collector.metadata, + ( + err: collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceError + ) => { + 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 0000000000..ba98291d0f --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts @@ -0,0 +1,86 @@ +/* + * 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 * as collectorTypes from '../../types'; + +import { ReadableSpan } from '@opentelemetry/tracing'; +import { CollectorExporterError } from '../../types'; +import { toCollectorExportTraceServiceRequest } from '../../transform'; +import { + CollectorExporter, + CollectorExporterConfig, +} from './CollectorExporter'; + +export const DEFAULT_COLLECTOR_URL_JSON = 'http://localhost:55678/v1/trace'; + +export function onInitWithJson( + _collector: CollectorExporter, + _config: CollectorExporterConfig +): void { + // nothing to be done for json yet +} + +export function sendSpansUsingJson( + collector: CollectorExporter, + spans: ReadableSpan[], + onSuccess: () => void, + onError: (error: 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), + [collectorTypes.OT_REQUEST_HEADER]: 1, + }, + }; + + 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 09af80dbf0..c27e0295fd 100644 --- a/packages/opentelemetry-exporter-collector/src/transform.ts +++ b/packages/opentelemetry-exporter-collector/src/transform.ts @@ -151,7 +151,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( {}, diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts index e5d67d67db..dcbab51b15 100644 --- a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts @@ -30,8 +30,8 @@ class CollectorExporter extends CollectorExporterBase { onInit() {} onShutdown() {} sendSpans() {} - getDefaultUrl(url: string | undefined) { - return url || ''; + getDefaultUrl(config: CollectorExporterConfig) { + return config.url || ''; } } 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 0000000000..500f0e654c --- /dev/null +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts @@ -0,0 +1,172 @@ +/* + * 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 { + CollectorExporter, + CollectorExporterConfig, +} from '../../src/platform/node'; +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: CollectorExporter; + let collectorExporterConfig: CollectorExporterConfig; + let spyRequest: any; + let spyWrite: any; + let spans: ReadableSpan[]; + describe('export', () => { + beforeEach(() => { + spyRequest = sinon.stub(http, 'request').returns(fakeRequest as any); + spyWrite = sinon.stub(fakeRequest, 'write'); + collectorExporterConfig = { + useJson: true, + hostName: 'foo', + logger: new core.NoopLogger(), + serviceName: 'bar', + attributes: {}, + url: 'http://foo.bar.com', + }; + collectorExporter = new CollectorExporter(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 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('CollectorExporter - node (getDefaultUrl)', () => { + it('should default to localhost', done => { + const collectorExporter = new CollectorExporter({ useJson: true }); + setTimeout(() => { + assert.strictEqual( + collectorExporter['url'], + 'http://localhost:55678/v1/trace' + ); + done(); + }); + }); + + it('should keep the URL if included', done => { + const url = 'http://foo.bar.com'; + const collectorExporter = new CollectorExporter({ url }); + setTimeout(() => { + assert.strictEqual(collectorExporter['url'], url); + done(); + }); + }); + }); +}); From c3966405306643e7128b55a3f64847d8dd1b7c5c Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 00:44:07 +0200 Subject: [PATCH 02/13] feat: updating readme and adding headers options in config for json over http --- .../README.md | 25 ++++++++++++++++++- .../src/platform/node/CollectorExporter.ts | 11 ++++++-- .../src/platform/node/utilWithJson.ts | 3 +-- .../node/CollectorExporterWithJson.test.ts | 18 +++++++++++-- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/README.md b/packages/opentelemetry-exporter-collector/README.md index 6a8b828056..0de255d987 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 CollectorExporter 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 } = require('@opentelemetry/exporter-collector'); + +const collectorOptions = { + useJson: true, // this needs to be set to true + serviceName: 'basic-service', + url: '', // url is optional and can be omitted - default is http://localhost:55678/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/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index 9fc27d2ebf..56fb32e5ae 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -20,7 +20,7 @@ import { CollectorExporterBase, CollectorExporterConfigBase, } from '../../CollectorExporterBase'; -import { CollectorExporterError } from '../../types'; +import * as collectorTypes from '../../types'; import { DEFAULT_COLLECTOR_URL_GRPC, onInitWithGrpc, @@ -35,10 +35,12 @@ import { GRPCQueueItem, TraceServiceClient } from './types'; /** * Collector Exporter Config for Node + * headers will only work if useJson is set to true */ export interface CollectorExporterConfig extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; metadata?: grpc.Metadata; + headers?: { [key: string]: string }; useJson?: boolean; } @@ -48,10 +50,14 @@ export interface CollectorExporterConfig extends CollectorExporterConfigBase { export class CollectorExporter extends CollectorExporterBase< CollectorExporterConfig > { + DEFAULT_HEADERS: { [key: string]: string } = { + [collectorTypes.OT_REQUEST_HEADER]: '1', + }; isShutDown: boolean = false; traceServiceClient?: TraceServiceClient = undefined; grpcSpansQueue: GRPCQueueItem[] = []; metadata?: grpc.Metadata; + headers: { [key: string]: string }; private readonly _useJson: boolean = false; /** @@ -66,6 +72,7 @@ export class CollectorExporter extends CollectorExporterBase< this.logger.debug('CollectorExporter - using grpc'); } this.metadata = config.metadata; + this.headers = config.headers || this.DEFAULT_HEADERS; } onShutdown(): void { @@ -88,7 +95,7 @@ export class CollectorExporter extends CollectorExporterBase< sendSpans( spans: ReadableSpan[], onSuccess: () => void, - onError: (error: CollectorExporterError) => void + onError: (error: collectorTypes.CollectorExporterError) => void ): void { if (this.isShutDown) { return; diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts index ba98291d0f..2dd30ddca5 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts @@ -17,7 +17,6 @@ import * as url from 'url'; import * as http from 'http'; import * as https from 'https'; -import * as collectorTypes from '../../types'; import { ReadableSpan } from '@opentelemetry/tracing'; import { CollectorExporterError } from '../../types'; @@ -57,7 +56,7 @@ export function sendSpansUsingJson( method: 'POST', headers: { 'Content-Length': Buffer.byteLength(body), - [collectorTypes.OT_REQUEST_HEADER]: 1, + ...collector.headers, }, }; diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts index 500f0e654c..c642d27de1 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts @@ -48,14 +48,17 @@ const mockResError = { describe('CollectorExporter - node with json over http', () => { let collectorExporter: CollectorExporter; let collectorExporterConfig: CollectorExporterConfig; - let spyRequest: any; - let spyWrite: any; + 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', + }, useJson: true, hostName: 'foo', logger: new core.NoopLogger(), @@ -86,6 +89,17 @@ describe('CollectorExporter - node with json over http', () => { }); }); + 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, () => {}); From 5b9f95d5e4f44db79c4ca07f9e1cc89eb62d6669 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 14:49:32 +0200 Subject: [PATCH 03/13] chore: reviews and few small cleanups --- examples/collector-exporter-node/start.js | 4 ++-- .../README.md | 4 ++-- .../src/enums.ts | 23 +++++++++++++++++++ .../src/index.ts | 1 + .../src/platform/browser/CollectorExporter.ts | 2 +- .../src/platform/node/CollectorExporter.ts | 20 +++++++++------- .../test/browser/CollectorExporter.test.ts | 6 ++--- .../node/CollectorExporterWithJson.test.ts | 7 ++++-- 8 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 packages/opentelemetry-exporter-collector/src/enums.ts diff --git a/examples/collector-exporter-node/start.js b/examples/collector-exporter-node/start.js index 28364905d7..67ddf76713 100644 --- a/examples/collector-exporter-node/start.js +++ b/examples/collector-exporter-node/start.js @@ -3,12 +3,12 @@ const opentelemetry = require('@opentelemetry/api'); const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); const { ConsoleLogger, LogLevel } = require('@opentelemetry/core'); -const { CollectorExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorExporter, CollectorTransportNode } = require('@opentelemetry/exporter-collector'); const exporter = new CollectorExporter({ logger: new ConsoleLogger(LogLevel.DEBUG), serviceName: 'basic-service', - // useJson: true, + protocol: CollectorTransportNode.HTTP_JSON, }); const provider = new BasicTracerProvider(); diff --git a/packages/opentelemetry-exporter-collector/README.md b/packages/opentelemetry-exporter-collector/README.md index 0de255d987..3ad45c5d93 100644 --- a/packages/opentelemetry-exporter-collector/README.md +++ b/packages/opentelemetry-exporter-collector/README.md @@ -113,10 +113,10 @@ Note, that this will only work if TLS is also configured on the server. ```js const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing'); -const { CollectorExporter } = require('@opentelemetry/exporter-collector'); +const { CollectorExporter, CollectorTransportNode } = require('@opentelemetry/exporter-collector'); const collectorOptions = { - useJson: true, // this needs to be set to true + protocolNode: CollectorTransportNode.HTTP_JSON, serviceName: 'basic-service', url: '', // url is optional and can be omitted - default is http://localhost:55678/v1/trace headers: { diff --git a/packages/opentelemetry-exporter-collector/src/enums.ts b/packages/opentelemetry-exporter-collector/src/enums.ts new file mode 100644 index 0000000000..adea96ac68 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/src/enums.ts @@ -0,0 +1,23 @@ +/* + * 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 + */ +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 52ec5f71f5..79ddd59b38 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/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts index afbd742c3f..00db095974 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts @@ -40,7 +40,7 @@ export class CollectorExporter extends CollectorExporterBase< DEFAULT_HEADERS: { [key: string]: string } = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; - private _headers: { [key: string]: string }; + private _headers: Record; private _useXHR: boolean = false; /** diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index 56fb32e5ae..6ba365c493 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -20,6 +20,7 @@ import { CollectorExporterBase, CollectorExporterConfigBase, } from '../../CollectorExporterBase'; +import { CollectorProtocolNode } from '../../enums'; import * as collectorTypes from '../../types'; import { DEFAULT_COLLECTOR_URL_GRPC, @@ -40,8 +41,8 @@ import { GRPCQueueItem, TraceServiceClient } from './types'; export interface CollectorExporterConfig extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; metadata?: grpc.Metadata; - headers?: { [key: string]: string }; - useJson?: boolean; + headers?: Record; + protocolNode?: CollectorProtocolNode; } /** @@ -58,15 +59,18 @@ export class CollectorExporter extends CollectorExporterBase< grpcSpansQueue: GRPCQueueItem[] = []; metadata?: grpc.Metadata; headers: { [key: string]: string }; - private readonly _useJson: boolean = false; + private readonly _protocol: CollectorProtocolNode; /** * @param config */ constructor(config: CollectorExporterConfig = {}) { super(config); - this._useJson = !!config.useJson; - if (this._useJson) { + this._protocol = + typeof config.protocolNode !== 'undefined' + ? config.protocolNode + : CollectorProtocolNode.GRPC; + if (this._protocol === CollectorProtocolNode.HTTP_JSON) { this.logger.debug('CollectorExporter - using json over http'); } else { this.logger.debug('CollectorExporter - using grpc'); @@ -85,7 +89,7 @@ export class CollectorExporter extends CollectorExporterBase< onInit(config: CollectorExporterConfig): void { this.isShutDown = false; - if (config.useJson) { + if (config.protocolNode === CollectorProtocolNode.HTTP_JSON) { onInitWithJson(this, config); } else { onInitWithGrpc(this, config); @@ -100,7 +104,7 @@ export class CollectorExporter extends CollectorExporterBase< if (this.isShutDown) { return; } - if (this._useJson) { + if (this._protocol) { sendSpansUsingJson(this, spans, onSuccess, onError); } else { sendSpansUsingGrpc(this, spans, onSuccess, onError); @@ -109,7 +113,7 @@ export class CollectorExporter extends CollectorExporterBase< getDefaultUrl(config: CollectorExporterConfig): string { if (!config.url) { - return config.useJson + return config.protocolNode === CollectorProtocolNode.HTTP_JSON ? DEFAULT_COLLECTOR_URL_JSON : DEFAULT_COLLECTOR_URL_GRPC; } diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts index 265bf44136..90410827b5 100644 --- a/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts @@ -36,9 +36,9 @@ const sendBeacon = navigator.sendBeacon; describe('CollectorExporter - web', () => { let collectorExporter: CollectorExporter; let collectorExporterConfig: CollectorExporterConfig; - 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/node/CollectorExporterWithJson.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts index c642d27de1..1ed381ad47 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts @@ -19,6 +19,7 @@ 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 { CollectorExporter, CollectorExporterConfig, @@ -59,7 +60,7 @@ describe('CollectorExporter - node with json over http', () => { headers: { foo: 'bar', }, - useJson: true, + protocolNode: CollectorProtocolNode.HTTP_JSON, hostName: 'foo', logger: new core.NoopLogger(), serviceName: 'bar', @@ -164,7 +165,9 @@ describe('CollectorExporter - node with json over http', () => { }); describe('CollectorExporter - node (getDefaultUrl)', () => { it('should default to localhost', done => { - const collectorExporter = new CollectorExporter({ useJson: true }); + const collectorExporter = new CollectorExporter({ + protocolNode: CollectorProtocolNode.HTTP_JSON, + }); setTimeout(() => { assert.strictEqual( collectorExporter['url'], From c8975254fe20e04ed05e76fd193f2a69af95ef8e Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 14:54:49 +0200 Subject: [PATCH 04/13] chore: aligning type for headers --- .../src/platform/browser/CollectorExporter.ts | 2 +- .../src/platform/node/CollectorExporter.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts index 00db095974..186b40b81b 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts @@ -37,7 +37,7 @@ const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace'; export class CollectorExporter extends CollectorExporterBase< CollectorExporterConfig > { - DEFAULT_HEADERS: { [key: string]: string } = { + DEFAULT_HEADERS: Record = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; private _headers: Record; diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index 6ba365c493..e6d72ba078 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -58,7 +58,7 @@ export class CollectorExporter extends CollectorExporterBase< traceServiceClient?: TraceServiceClient = undefined; grpcSpansQueue: GRPCQueueItem[] = []; metadata?: grpc.Metadata; - headers: { [key: string]: string }; + headers: Record; private readonly _protocol: CollectorProtocolNode; /** From 9134664aa625ff0090def616df528240f11307fc Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 16:50:35 +0200 Subject: [PATCH 05/13] chore: fixing doc --- .../src/platform/node/CollectorExporter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index e6d72ba078..de7c5096b9 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -36,7 +36,7 @@ import { GRPCQueueItem, TraceServiceClient } from './types'; /** * Collector Exporter Config for Node - * headers will only work if useJson is set to true + * headers will only work if protocolNode is HTTP_JSON */ export interface CollectorExporterConfig extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; From 12a4d91f6e28ee5343b6882c1aa6f97199896602 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 18:17:04 +0200 Subject: [PATCH 06/13] chore: unifying types for headers --- .../src/platform/browser/CollectorExporter.ts | 10 ++++++---- .../src/platform/node/CollectorExporter.ts | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts index 186b40b81b..660b2e2fd8 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts @@ -26,7 +26,7 @@ import * as collectorTypes from '../../types'; * Collector Exporter Config for Web */ export interface CollectorExporterConfig extends CollectorExporterConfigBase { - headers?: { [key: string]: string }; + headers?: Partial>; } const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace'; @@ -37,10 +37,10 @@ const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace'; export class CollectorExporter extends CollectorExporterBase< CollectorExporterConfig > { - DEFAULT_HEADERS: Record = { + DEFAULT_HEADERS: Partial> = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; - private _headers: Record; + private _headers: Partial>; private _useXHR: boolean = false; /** @@ -120,7 +120,9 @@ export class CollectorExporter extends CollectorExporterBase< xhr.setRequestHeader('Accept', 'application/json'); xhr.setRequestHeader('Content-Type', 'application/json'); Object.entries(this._headers).forEach(([k, v]) => { - xhr.setRequestHeader(k, v); + if (typeof v !== 'undefined') { + xhr.setRequestHeader(k, v); + } }); xhr.send(body); diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index de7c5096b9..fdf7dececb 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -41,7 +41,7 @@ import { GRPCQueueItem, TraceServiceClient } from './types'; export interface CollectorExporterConfig extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; metadata?: grpc.Metadata; - headers?: Record; + headers?: Partial>; protocolNode?: CollectorProtocolNode; } @@ -51,14 +51,14 @@ export interface CollectorExporterConfig extends CollectorExporterConfigBase { export class CollectorExporter extends CollectorExporterBase< CollectorExporterConfig > { - DEFAULT_HEADERS: { [key: string]: string } = { + DEFAULT_HEADERS: Partial> = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; isShutDown: boolean = false; traceServiceClient?: TraceServiceClient = undefined; grpcSpansQueue: GRPCQueueItem[] = []; metadata?: grpc.Metadata; - headers: Record; + headers: Partial>; private readonly _protocol: CollectorProtocolNode; /** From ea671fcccb0ea71fd7035fefdd3452e9fdc12145 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 23:07:06 +0200 Subject: [PATCH 07/13] chore: reviews --- examples/collector-exporter-node/start.js | 5 ++++- .../src/platform/node/CollectorExporter.ts | 5 ++++- .../test/node/CollectorExporter.test.ts | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/examples/collector-exporter-node/start.js b/examples/collector-exporter-node/start.js index 67ddf76713..1078751700 100644 --- a/examples/collector-exporter-node/start.js +++ b/examples/collector-exporter-node/start.js @@ -8,7 +8,10 @@ const { CollectorExporter, CollectorTransportNode } = require('@opentelemetry/ex const exporter = new CollectorExporter({ logger: new ConsoleLogger(LogLevel.DEBUG), serviceName: 'basic-service', - protocol: CollectorTransportNode.HTTP_JSON, + // headers: { + // foo: 'bar' + // }, + // protocol: CollectorTransportNode.HTTP_JSON, }); const provider = new BasicTracerProvider(); diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index fdf7dececb..c9e3e17c5c 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -74,6 +74,9 @@ export class CollectorExporter extends CollectorExporterBase< this.logger.debug('CollectorExporter - using json over http'); } 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 = config.headers || this.DEFAULT_HEADERS; @@ -104,7 +107,7 @@ export class CollectorExporter extends CollectorExporterBase< if (this.isShutDown) { return; } - if (this._protocol) { + if (this._protocol === CollectorProtocolNode.HTTP_JSON) { sendSpansUsingJson(this, spans, onSuccess, onError); } else { sendSpansUsingGrpc(this, spans, onSuccess, onError); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts index 13da3284a2..8b0cb53207 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts @@ -15,6 +15,7 @@ */ import * as protoLoader from '@grpc/proto-loader'; +import { ConsoleLogger, LogLevel } from '@opentelemetry/core'; import * as grpc from 'grpc'; import * as path from 'path'; import * as fs from 'fs'; @@ -138,6 +139,23 @@ 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 CollectorExporter({ + 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'); + }); + }); + describe('export', () => { it('should export spans', done => { const responseSpy = sinon.spy(); From be7a67ba8d2bdf749e9fb02215b80af2b325a4be Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 23:48:36 +0200 Subject: [PATCH 08/13] chore: adding validation for headers, and making the types correct this time --- .../src/platform/browser/CollectorExporter.ts | 14 +++--- .../src/platform/node/CollectorExporter.ts | 10 ++-- .../src/util.ts | 38 +++++++++++++++ .../test/common/utils.test.ts | 48 +++++++++++++++++++ 4 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 packages/opentelemetry-exporter-collector/src/util.ts create mode 100644 packages/opentelemetry-exporter-collector/test/common/utils.test.ts diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts index 660b2e2fd8..567c8914e3 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts @@ -21,12 +21,13 @@ import { import { ReadableSpan } from '@opentelemetry/tracing'; import { toCollectorExportTraceServiceRequest } from '../../transform'; import * as collectorTypes from '../../types'; +import { parseHeaders } from '../../util'; /** * Collector Exporter Config for Web */ export interface CollectorExporterConfig extends CollectorExporterConfigBase { - headers?: Partial>; + headers?: Partial>; } const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace'; @@ -37,10 +38,10 @@ const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace'; export class CollectorExporter extends CollectorExporterBase< CollectorExporterConfig > { - DEFAULT_HEADERS: Partial> = { + DEFAULT_HEADERS: Record = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; - private _headers: Partial>; + private _headers: Record; private _useXHR: boolean = false; /** @@ -48,7 +49,8 @@ export class CollectorExporter extends CollectorExporterBase< */ constructor(config: CollectorExporterConfig = {}) { 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'; } @@ -120,9 +122,7 @@ export class CollectorExporter extends CollectorExporterBase< xhr.setRequestHeader('Accept', 'application/json'); xhr.setRequestHeader('Content-Type', 'application/json'); Object.entries(this._headers).forEach(([k, v]) => { - if (typeof v !== 'undefined') { - xhr.setRequestHeader(k, v); - } + xhr.setRequestHeader(k, v); }); xhr.send(body); diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts index c9e3e17c5c..b5e7cc1794 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts @@ -22,6 +22,7 @@ import { } from '../../CollectorExporterBase'; import { CollectorProtocolNode } from '../../enums'; import * as collectorTypes from '../../types'; +import { parseHeaders } from '../../util'; import { DEFAULT_COLLECTOR_URL_GRPC, onInitWithGrpc, @@ -41,7 +42,7 @@ import { GRPCQueueItem, TraceServiceClient } from './types'; export interface CollectorExporterConfig extends CollectorExporterConfigBase { credentials?: grpc.ChannelCredentials; metadata?: grpc.Metadata; - headers?: Partial>; + headers?: Partial>; protocolNode?: CollectorProtocolNode; } @@ -51,14 +52,14 @@ export interface CollectorExporterConfig extends CollectorExporterConfigBase { export class CollectorExporter extends CollectorExporterBase< CollectorExporterConfig > { - DEFAULT_HEADERS: Partial> = { + DEFAULT_HEADERS: Record = { [collectorTypes.OT_REQUEST_HEADER]: '1', }; isShutDown: boolean = false; traceServiceClient?: TraceServiceClient = undefined; grpcSpansQueue: GRPCQueueItem[] = []; metadata?: grpc.Metadata; - headers: Partial>; + headers: Record; private readonly _protocol: CollectorProtocolNode; /** @@ -79,7 +80,8 @@ export class CollectorExporter extends CollectorExporterBase< } } this.metadata = config.metadata; - this.headers = config.headers || this.DEFAULT_HEADERS; + this.headers = + parseHeaders(config.headers, this.logger) || this.DEFAULT_HEADERS; } onShutdown(): void { diff --git a/packages/opentelemetry-exporter-collector/src/util.ts b/packages/opentelemetry-exporter-collector/src/util.ts new file mode 100644 index 0000000000..1cb1b18aae --- /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/common/utils.test.ts b/packages/opentelemetry-exporter-collector/test/common/utils.test.ts new file mode 100644 index 0000000000..b5fb8d3507 --- /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, {}); + }); + }); +}); From 57580862eee3ef8ff242a6a2e13274987053a587 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 23:58:11 +0200 Subject: [PATCH 09/13] chore: linting --- packages/opentelemetry-context-base/src/NoopContextManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-context-base/src/NoopContextManager.ts b/packages/opentelemetry-context-base/src/NoopContextManager.ts index e02816f221..4787d19c94 100644 --- a/packages/opentelemetry-context-base/src/NoopContextManager.ts +++ b/packages/opentelemetry-context-base/src/NoopContextManager.ts @@ -29,7 +29,7 @@ export class NoopContextManager implements types.ContextManager { return fn(); } - bind(target: T, context?: Context): T { + bind(target: T, _context?: Context): T { return target; } From fe166e0851c10b0914680266008c9e6f3131a881 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Fri, 26 Jun 2020 23:59:22 +0200 Subject: [PATCH 10/13] chore: linting --- examples/collector-exporter-node/start.js | 2 +- packages/opentelemetry-context-base/src/NoopContextManager.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/collector-exporter-node/start.js b/examples/collector-exporter-node/start.js index 1078751700..a067df542b 100644 --- a/examples/collector-exporter-node/start.js +++ b/examples/collector-exporter-node/start.js @@ -11,7 +11,7 @@ const exporter = new CollectorExporter({ // headers: { // foo: 'bar' // }, - // protocol: CollectorTransportNode.HTTP_JSON, + protocol: CollectorTransportNode.HTTP_JSON, }); const provider = new BasicTracerProvider(); diff --git a/packages/opentelemetry-context-base/src/NoopContextManager.ts b/packages/opentelemetry-context-base/src/NoopContextManager.ts index 4787d19c94..e02816f221 100644 --- a/packages/opentelemetry-context-base/src/NoopContextManager.ts +++ b/packages/opentelemetry-context-base/src/NoopContextManager.ts @@ -29,7 +29,7 @@ export class NoopContextManager implements types.ContextManager { return fn(); } - bind(target: T, _context?: Context): T { + bind(target: T, context?: Context): T { return target; } From 2ba480cd6862ab7ad041cfd24d92a33c538d3230 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Tue, 30 Jun 2020 17:14:02 +0200 Subject: [PATCH 11/13] chore: fixes after merge --- packages/opentelemetry-exporter-collector/README.md | 2 +- .../test/node/CollectorExporterWithJson.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/README.md b/packages/opentelemetry-exporter-collector/README.md index a657b0e8b6..8cfda2f5a2 100644 --- a/packages/opentelemetry-exporter-collector/README.md +++ b/packages/opentelemetry-exporter-collector/README.md @@ -118,7 +118,7 @@ const { CollectorExporter, CollectorTransportNode } = require('@opentelemetry/e const collectorOptions = { protocolNode: CollectorTransportNode.HTTP_JSON, serviceName: 'basic-service', - url: '', // url is optional and can be omitted - default is http://localhost:55678/v1/trace + 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 diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts index 8f296df808..f641f8579d 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorExporterWithJson.test.ts @@ -168,7 +168,7 @@ describe('CollectorExporter - node with json over http', () => { setTimeout(() => { assert.strictEqual( collectorExporter['url'], - 'http://localhost:55678/v1/trace' + 'http://localhost:55680/v1/trace' ); done(); }); From ff77305bcc612aa7cc7f29d3fc799a313524ce19 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Thu, 2 Jul 2020 17:11:39 +0200 Subject: [PATCH 12/13] chore: reviews --- .../src/enums.ts | 1 + .../platform/node/CollectorTraceExporter.ts | 3 +++ .../src/platform/node/utilWithJson.ts | 1 + .../test/node/CollectorTraceExporter.test.ts | 24 +++++++++++++++---- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/enums.ts b/packages/opentelemetry-exporter-collector/src/enums.ts index adea96ac68..08c2fd9f2b 100644 --- a/packages/opentelemetry-exporter-collector/src/enums.ts +++ b/packages/opentelemetry-exporter-collector/src/enums.ts @@ -16,6 +16,7 @@ /** * Collector transport protocol node options + * Default is GRPC */ export enum CollectorProtocolNode { GRPC, diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts index df1444cb39..849f76e2b3 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts @@ -60,6 +60,9 @@ export class CollectorTraceExporter extends CollectorTraceExporterBase< : 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) { diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts index 52ed0e6b43..1276f9f9a9 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithJson.ts @@ -53,6 +53,7 @@ export function sendSpansUsingJson( method: 'POST', headers: { 'Content-Length': Buffer.byteLength(body), + 'Content-Type': 'application/json', ...collector.headers, }, }; diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts index cb7bc28efa..ebe433e929 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts @@ -16,23 +16,24 @@ import * as protoLoader from '@grpc/proto-loader'; import { ConsoleLogger, LogLevel } from '@opentelemetry/core'; -import * as grpc from 'grpc'; -import * as path from 'path'; -import * as fs from 'fs'; 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'; @@ -154,6 +155,21 @@ const testCollectorExporter = (params: TestParams) => 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', () => { From f73aa1ed3c061904a9d8d3718a59de777cd4fbf9 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 6 Jul 2020 20:35:25 +0200 Subject: [PATCH 13/13] chore: merge branch 'master' into collector_json --- .../src/platform/node/utilWithGrpc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts index b91a22a597..0e1712ed90 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/utilWithGrpc.ts @@ -79,7 +79,7 @@ export function sendSpansUsingGrpc( exportTraceServiceRequest, collector.metadata, ( - err: collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceError + err: collectorTypes.ExportServiceError ) => { if (err) { collector.logger.error(