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.

Sign up for GitHub

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: use Blob in sendBeacon to add application/json type #2336

Merged
merged 14 commits into from
Aug 10, 2021
Merged

feat: use Blob in sendBeacon to add application/json type #2336

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6feaea8
fix(opentelemetry-exporter-collector): issue#2321 use Blob for method…
jufab Jul 7, 2021
3cc3c90
fix: test unit browser error
jufab Jul 8, 2021
9d7a4f7
Merge branch 'main' into fix-issue-2321
jufab Jul 22, 2021
9cf813c
feat: add contentType configuration for beacon
jufab Jul 26, 2021
1c68c75
fix: some lint
jufab Jul 26, 2021
b891b21
Merge branch 'open-telemetry:main' into fix-issue-2321
jufab Jul 26, 2021
c5fdb2a
Merge branch 'open-telemetry:main' into fix-issue-2321
jufab Jul 27, 2021
9218835
fix: use BlobPropertyBag for Blob
jufab Jul 27, 2021
e07167b
Merge branch 'open-telemetry:main' into fix-issue-2321
jufab Aug 1, 2021
e9207d1
fix: assertion name
jufab Aug 1, 2021
17e27ae
fix: blob on public sendWithBeacon only
jufab Aug 3, 2021
120cb87
Merge branch 'main' into fix-issue-2321
dyladan Aug 9, 2021
fdb7a4b
Merge branch 'main' into fix-issue-2321
dyladan Aug 10, 2021
5ccc02e
Style nit
dyladan Aug 10, 2021
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
2 changes: 1 addition & 1 deletion packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export abstract class CollectorExporterBase<
* @param items
* @param resultCallback
*/
export(items: ExportItem[], resultCallback: (result: ExportResult) => void) {
export(items: ExportItem[], resultCallback: (result: ExportResult) => void): void {
if (this._isShutdown) {
resultCallback({
code: ExportResultCode.FAILED,
Expand Down
15 changes: 11 additions & 4 deletions ...ges/opentelemetry-exporter-collector/src/platform/browser/CollectorExporterBrowserBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,22 @@ import { sendWithBeacon, sendWithXhr } from './util';
import { diag } from '@opentelemetry/api';
import { getEnv, baggageUtils } from '@opentelemetry/core';

const DEFAULT_BLOB_PROPERTY_BAG: BlobPropertyBag = { type: 'application/json' };

/**
* Collector Metric Exporter abstract base class
*/
export abstract class CollectorExporterBrowserBase<
ExportItem,
ServiceRequest
> extends CollectorExporterBase<
> extends CollectorExporterBase<
CollectorExporterConfigBase,
ExportItem,
ServiceRequest
> {
> {
protected _headers: Record<string, string>;
private _useXHR: boolean = false;
private _blobPropertBag: BlobPropertyBag = {};

/**
* @param config
Expand All @@ -53,6 +56,10 @@ export abstract class CollectorExporterBrowserBase<
);
} else {
this._headers = {};
this._blobPropertBag = {
MSNev marked this conversation as resolved.
Show resolved Hide resolved
...DEFAULT_BLOB_PROPERTY_BAG,
...config.beaconBlobPropertyBag
}
}
}

Expand All @@ -68,7 +75,7 @@ export abstract class CollectorExporterBrowserBase<
items: ExportItem[],
onSuccess: () => void,
onError: (error: collectorTypes.CollectorExporterError) => void
) {
): void {
if (this._isShutdown) {
diag.debug('Shutdown already started. Cannot send objects');
return;
Expand All @@ -94,7 +101,7 @@ export abstract class CollectorExporterBrowserBase<
if (this._useXHR) {
sendWithXhr(body, this.url, this._headers, _onSuccess, _onError);
} else {
sendWithBeacon(body, this.url, _onSuccess, _onError);
sendWithBeacon(body, this.url, this._blobPropertBag, _onSuccess, _onError);
MSNev marked this conversation as resolved.
Show resolved Hide resolved
}
});
this._sendingPromises.push(promise);
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-exporter-collector/src/platform/browser/CollectorTraceExporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class CollectorTraceExporter
return toCollectorExportTraceServiceRequest(spans, this, true);
}

getDefaultUrl(config: CollectorExporterConfigBase) {
getDefaultUrl(config: CollectorExporterConfigBase): string {
return typeof config.url === 'string'
? config.url
: getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0
Expand Down
7 changes: 4 additions & 3 deletions packages/opentelemetry-exporter-collector/src/platform/browser/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ import * as collectorTypes from '../../types';
export function sendWithBeacon(
body: string,
url: string,
blobPropertyBag: BlobPropertyBag,
onSuccess: () => void,
onError: (error: collectorTypes.CollectorExporterError) => void
) {
if (navigator.sendBeacon(url, body)) {
): void {
if (navigator.sendBeacon(url, new Blob([body], blobPropertyBag))) {
diag.debug('sendBeacon - can send', body);
onSuccess();
} else {
Expand All @@ -52,7 +53,7 @@ export function sendWithXhr(
headers: Record<string, string>,
onSuccess: () => void,
onError: (error: collectorTypes.CollectorExporterError) => void
) {
): void {
const xhr = new XMLHttpRequest();
xhr.open('POST', url);

Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-exporter-collector/src/platform/node/CollectorTraceExporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class CollectorTraceExporter
return toCollectorExportTraceServiceRequest(spans, this, true);
}

getDefaultUrl(config: CollectorExporterNodeConfigBase) {
getDefaultUrl(config: CollectorExporterNodeConfigBase) : string {
return typeof config.url === 'string'
? config.url
: getEnv().OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.length > 0
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry-exporter-collector/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ export interface CollectorExporterConfigBase {
attributes?: SpanAttributes;
url?: string;
concurrencyLimit?: number;
beaconBlobPropertyBag?: BlobPropertyBag;
jufab marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,11 @@ describe('CollectorMetricExporter - web', () => {
it('should successfully send metrics using sendBeacon', done => {
collectorExporter.export(metrics, () => {});

setTimeout(() => {
setTimeout(async () => {
const args = stubBeacon.args[0];
const url = args[0];
const body = args[1];
const blob:Blob = args[1];
dyladan marked this conversation as resolved.
Show resolved Hide resolved
const body = await blob.text();
const json = JSON.parse(
body
) as collectorTypes.opentelemetryProto.collector.metrics.v1.ExportMetricsServiceRequest;
Expand Down
23 changes: 14 additions & 9 deletions packages/opentelemetry-exporter-collector/test/browser/CollectorTraceExporter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('CollectorTraceExporter - web', () => {
hostname: 'foo',
attributes: {},
url: 'http://foo.bar.com',
beaconBlobPropertyBag: { type: 'text/plain' },
};
});

Expand All @@ -67,12 +68,14 @@ describe('CollectorTraceExporter - web', () => {
});

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

setTimeout(() => {
setTimeout(async () => {
const args = stubBeacon.args[0];
const url = args[0];
const body = args[1];
const blob: Blob = args[1];
const body = await blob.text();
const header: string = blob.type;
const json = JSON.parse(
body
) as collectorTypes.opentelemetryProto.collector.trace.v1.ExportTraceServiceRequest;
Expand All @@ -95,6 +98,8 @@ describe('CollectorTraceExporter - web', () => {

assert.strictEqual(stubOpen.callCount, 0);

assert.strictEqual(header, collectorExporterConfig.beaconBlobPropertyBag?.type);
jufab marked this conversation as resolved.
Show resolved Hide resolved

ensureExportTraceServiceRequestIsSet(json);

done();
Expand All @@ -107,7 +112,7 @@ describe('CollectorTraceExporter - web', () => {
const spyLoggerError = sinon.stub(diag, 'error');
stubBeacon.returns(true);

collectorTraceExporter.export(spans, () => {});
collectorTraceExporter.export(spans, () => { });

setTimeout(() => {
const response: any = spyLoggerDebug.args[1][0];
Expand Down Expand Up @@ -143,7 +148,7 @@ describe('CollectorTraceExporter - web', () => {
});

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

setTimeout(() => {
const request = server.requests[0];
Expand Down Expand Up @@ -181,7 +186,7 @@ describe('CollectorTraceExporter - web', () => {
const spyLoggerDebug = sinon.stub(diag, 'debug');
const spyLoggerError = sinon.stub(diag, 'error');

collectorTraceExporter.export(spans, () => {});
collectorTraceExporter.export(spans, () => { });

setTimeout(() => {
const request = server.requests[0];
Expand Down Expand Up @@ -210,7 +215,7 @@ describe('CollectorTraceExporter - web', () => {
});

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

setTimeout(() => {
const request = server.requests[0];
Expand Down Expand Up @@ -248,7 +253,7 @@ describe('CollectorTraceExporter - web', () => {
);
});
it('should successfully send custom headers using XMLHTTPRequest', done => {
collectorTraceExporter.export(spans, () => {});
collectorTraceExporter.export(spans, () => { });

setTimeout(() => {
const [{ requestHeaders }] = server.requests;
Expand All @@ -271,7 +276,7 @@ describe('CollectorTraceExporter - web', () => {
});

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

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