Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: collector exporter custom headers and metadata #1204

Merged
merged 7 commits into from
Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/opentelemetry-exporter-collector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@ npm install --save @opentelemetry/exporter-collector
```

## Usage in Web
The CollectorExporter in Web expects the endpoint to end in `/v1/trace`.
The CollectorExporter 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 { CollectorExporter } from '@opentelemetry/exporter-collector';

const collectorOptions = {
url: '<opentelemetry-collector-url>' // url is optional and can be omitted - default is http://localhost:55678/v1/trace
url: '<opentelemetry-collector-url>', // url is optional and can be omitted - default is http://localhost:55678/v1/trace
headers: {}, //an optional object containing custom headers to be sent with each request
};

const provider = new WebTracerProvider();
Expand All @@ -44,6 +45,7 @@ const { CollectorExporter } = require('@opentelemetry/exporter-collector');
const collectorOptions = {
serviceName: 'basic-service',
url: '<opentelemetry-collector-url>' // url is optional and can be omitted - default is localhost:55678
metadata, // an optional grpc.Metadata object to be sent with each request
mwear marked this conversation as resolved.
Show resolved Hide resolved
};

const provider = new BasicTracerProvider();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ import { ReadableSpan } from '@opentelemetry/tracing';
import { toCollectorExportTraceServiceRequest } from '../../transform';
import * as collectorTypes from '../../types';

export type CollectorExporterConfig = CollectorExporterConfigBase;
/**
* Collector Exporter Config for Web
*/
export interface CollectorExporterConfig extends CollectorExporterConfigBase {
headers?: { [key: string]: string };
}

const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace';

Expand All @@ -32,6 +37,22 @@ const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace';
export class CollectorExporter extends CollectorExporterBase<
CollectorExporterConfig
> {
DEFAULT_HEADERS: { [key: string]: string } = {
[collectorTypes.OT_REQUEST_HEADER]: '1',
};
private _headers: { [key: string]: string };
private _useXHR: boolean = false;

/**
* @param config
*/
constructor(config: CollectorExporterConfig = {}) {
super(config);
this._headers = config.headers || this.DEFAULT_HEADERS;
this._useXHR =
!!config.headers || typeof navigator.sendBeacon !== 'function';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be !!this._headers || ..., or do we explicitly not care about sending only DEFAULT_HEADERS here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are custom headers to be sent, then spans must be sent using an XMLHTTPRequest, the beacon doesn't support arbitrary headers. If there are not custom headers, and the browser has beacon support, spans will be sent using it.

There is one default header, it's collectorTypes.OT_REQUEST_HEADER which is a signal to the XMLHTTPRequest plugin not to trace the request. This would come into play if there are not any custom headers, and the browser does not have beacon support. A better long term solution for this would be #1040. It would clean this up and we could remove default headers altogether.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense thanks!

}

onInit(): void {
window.addEventListener('unload', this.shutdown);
}
Expand All @@ -53,13 +74,12 @@ export class CollectorExporter extends CollectorExporterBase<
spans,
this
);

const body = JSON.stringify(exportTraceServiceRequest);

if (typeof navigator.sendBeacon === 'function') {
this._sendSpansWithBeacon(body, onSuccess, onError);
} else {
if (this._useXHR) {
this._sendSpansWithXhr(body, onSuccess, onError);
} else {
this._sendSpansWithBeacon(body, onSuccess, onError);
}
}

Expand Down Expand Up @@ -97,9 +117,12 @@ export class CollectorExporter extends CollectorExporterBase<
) {
const xhr = new XMLHttpRequest();
xhr.open('POST', this.url);
xhr.setRequestHeader(collectorTypes.OT_REQUEST_HEADER, '1');
xhr.setRequestHeader('Accept', 'application/json');
xhr.setRequestHeader('Content-Type', 'application/json');
Object.entries(this._headers).forEach(([k, v]) => {
xhr.setRequestHeader(k, v);
});

xhr.send(body);

xhr.onreadystatechange = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const DEFAULT_COLLECTOR_URL = 'localhost:55678';
*/
export interface CollectorExporterConfig extends CollectorExporterConfigBase {
credentials?: grpc.ChannelCredentials;
metadata?: grpc.Metadata;
}

/**
Expand All @@ -47,12 +48,14 @@ export class CollectorExporter extends CollectorExporterBase<
isShutDown: boolean = false;
traceServiceClient?: TraceServiceClient = undefined;
grpcSpansQueue: GRPCQueueItem[] = [];
metadata?: grpc.Metadata;

/**
* @param config
*/
constructor(config: CollectorExporterConfig = {}) {
super(config);
this.metadata = config.metadata;
}

onShutdown(): void {
Expand Down Expand Up @@ -115,6 +118,7 @@ export class CollectorExporter extends CollectorExporterBase<

this.traceServiceClient.export(
exportTraceServiceRequest,
this.metadata,
(
err: collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceError
) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,9 @@ export interface GRPCQueueItem {
* Trace Service Client for sending spans
*/
export interface TraceServiceClient extends grpc.Client {
export: (request: any, callback: Function) => {};
export: (
request: any,
metadata: grpc.Metadata | undefined,
callback: Function
) => {};
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
ensureSpanIsCorrect,
ensureExportTraceServiceRequestIsSet,
ensureWebResourceIsCorrect,
ensureHeadersContain,
mockedReadableSpan,
} from '../helper';
const sendBeacon = navigator.sendBeacon;
Expand All @@ -44,14 +45,6 @@ describe('CollectorExporter - web', () => {
spyOpen = sinon.stub(XMLHttpRequest.prototype, 'open');
spySend = sinon.stub(XMLHttpRequest.prototype, 'send');
spyBeacon = sinon.stub(navigator, 'sendBeacon');
collectorExporterConfig = {
hostName: 'foo',
logger: new NoopLogger(),
serviceName: 'bar',
attributes: {},
url: 'http://foo.bar.com',
};
collectorExporter = new CollectorExporter(collectorExporterConfig);
spans = [];
spans.push(Object.assign({}, mockedReadableSpan));
});
Expand All @@ -64,7 +57,21 @@ describe('CollectorExporter - web', () => {
});

describe('export', () => {
beforeEach(() => {
collectorExporterConfig = {
hostName: 'foo',
logger: new NoopLogger(),
serviceName: 'bar',
attributes: {},
url: 'http://foo.bar.com',
};
});

describe('when "sendBeacon" is available', () => {
beforeEach(() => {
collectorExporter = new CollectorExporter(collectorExporterConfig);
});

it('should successfully send the spans using sendBeacon', done => {
collectorExporter.export(spans, () => {});

Expand Down Expand Up @@ -139,6 +146,7 @@ describe('CollectorExporter - web', () => {
let server: any;
beforeEach(() => {
(window.navigator as any).sendBeacon = false;
collectorExporter = new CollectorExporter(collectorExporterConfig);
server = sinon.fakeServer.create();
});
afterEach(() => {
Expand Down Expand Up @@ -216,6 +224,78 @@ describe('CollectorExporter - web', () => {
done();
});
});

it('should send custom headers', done => {
collectorExporter.export(spans, () => {});

setTimeout(() => {
const request = server.requests[0];
request.respond(200);

assert.strictEqual(spyBeacon.callCount, 0);
done();
});
});
});
});

describe('export with custom headers', () => {
let server: any;
const customHeaders = {
foo: 'bar',
bar: 'baz',
};

beforeEach(() => {
collectorExporterConfig = {
logger: new NoopLogger(),
headers: customHeaders,
};
server = sinon.fakeServer.create();
});

afterEach(() => {
server.restore();
});

describe('when "sendBeacon" is available', () => {
beforeEach(() => {
collectorExporter = new CollectorExporter(collectorExporterConfig);
});
it('should successfully send custom headers using XMLHTTPRequest', done => {
collectorExporter.export(spans, () => {});

setTimeout(() => {
const [{ requestHeaders }] = server.requests;

ensureHeadersContain(requestHeaders, customHeaders);
assert.strictEqual(spyBeacon.callCount, 0);
assert.strictEqual(spyOpen.callCount, 0);

done();
});
});
});

describe('when "sendBeacon" is NOT available', () => {
beforeEach(() => {
(window.navigator as any).sendBeacon = false;
collectorExporter = new CollectorExporter(collectorExporterConfig);
});

it('should successfully send spans using XMLHttpRequest', done => {
collectorExporter.export(spans, () => {});

setTimeout(() => {
const [{ requestHeaders }] = server.requests;

ensureHeadersContain(requestHeaders, customHeaders);
assert.strictEqual(spyBeacon.callCount, 0);
assert.strictEqual(spyOpen.callCount, 0);

done();
});
});
});
});
});
Expand Down
24 changes: 24 additions & 0 deletions packages/opentelemetry-exporter-collector/test/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Resource } from '@opentelemetry/resources';
import * as assert from 'assert';
import { opentelemetryProto } from '../src/types';
import * as collectorTypes from '../src/types';
import * as grpc from 'grpc';

if (typeof Buffer === 'undefined') {
(window as any).Buffer = {
Expand Down Expand Up @@ -509,3 +510,26 @@ export function ensureExportTraceServiceRequestIsSet(
const spans = instrumentationLibrarySpans[0].spans;
assert.strictEqual(spans && spans.length, 1, 'spans are missing');
}

export function ensureMetadataIsCorrect(
actual: grpc.Metadata,
expected: grpc.Metadata
) {
//ignore user agent
expected.remove('user-agent');
actual.remove('user-agent');
assert.deepStrictEqual(actual.getMap(), expected.getMap());
}

export function ensureHeadersContain(
actual: { [key: string]: string },
expected: { [key: string]: string }
) {
Object.entries(expected).forEach(([k, v]) => {
assert.strictEqual(
v,
actual[k],
`Expected ${actual} to contain ${k}: ${v}`
);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import * as collectorTypes from '../../src/types';
import {
ensureResourceIsCorrect,
ensureExportedSpanIsCorrect,
ensureMetadataIsCorrect,
mockedReadableSpan,
} from '../helper';

Expand All @@ -41,18 +42,23 @@ const includeDirs = [path.resolve(__dirname, '../../src/platform/node/protos')];
const address = 'localhost:1501';

type TestParams = {
useTLS: boolean;
useTLS?: boolean;
metadata?: grpc.Metadata;
};

const metadata = new grpc.Metadata();
metadata.set('k', 'v');

const testCollectorExporter = (params: TestParams) =>
describe(`CollectorExporter - node ${
params.useTLS ? 'with TLS' : ''
}`, () => {
params.useTLS ? 'with' : 'without'
} TLS, ${params.metadata ? 'with' : 'without'} metadata`, () => {
let collectorExporter: CollectorExporter;
let server: grpc.Server;
let exportedData:
| collectorTypes.opentelemetryProto.trace.v1.ResourceSpans
| undefined;
let reqMetadata: grpc.Metadata | undefined;

before(done => {
server = new grpc.Server();
Expand All @@ -75,9 +81,11 @@ const testCollectorExporter = (params: TestParams) =>
{
Export: (data: {
request: collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceRequest;
metadata: grpc.Metadata;
}) => {
try {
exportedData = data.request.resourceSpans[0];
reqMetadata = data.metadata;
} catch (e) {
exportedData = undefined;
}
Expand Down Expand Up @@ -117,6 +125,7 @@ const testCollectorExporter = (params: TestParams) =>
serviceName: 'basic-service',
url: address,
credentials,
metadata: params.metadata,
});

const provider = new BasicTracerProvider();
Expand All @@ -126,6 +135,7 @@ const testCollectorExporter = (params: TestParams) =>

afterEach(() => {
exportedData = undefined;
reqMetadata = undefined;
});

describe('export', () => {
Expand Down Expand Up @@ -153,6 +163,9 @@ const testCollectorExporter = (params: TestParams) =>
ensureResourceIsCorrect(resource);
}
}
if (params.metadata && reqMetadata) {
ensureMetadataIsCorrect(reqMetadata, params.metadata);
}
done();
}, 200);
});
Expand All @@ -179,3 +192,4 @@ describe('CollectorExporter - node (getDefaultUrl)', () => {

testCollectorExporter({ useTLS: true });
testCollectorExporter({ useTLS: false });
testCollectorExporter({ metadata });