Skip to content

Commit

Permalink
feat(exporters)!: use transport interface in node.js exporters (open-…
Browse files Browse the repository at this point in the history 
…telemetry#4743)

* feat(exporters)!: use transport interface in node.js exporters

* feat(exporters): hide compression property

* feat(otlp-exporter-base)!: remove header property

* feat(otlp-exporter-base): add retrying transport

* fix: lint

* chore: add changelog entry

* fix: use queueMicrotask over nextTick

* chore: move changelog entry to unreleased

* chore: note that user-agent cannot be overwritten by users anymore

* fix: export missing ExportResponseRetryable

* fix: retry jitter
  • Loading branch information
@pichlermarc @Zirak
pichlermarc authored and Zirak committed Sep 14, 2024
1 parent f2b3ea0 commit 9bb4ff4
Show file tree
Hide file tree
Showing 38 changed files with 1,225 additions and 741 deletions.
8 changes: 7 additions & 1 deletion experimental/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ All notable changes to experimental packages in this project will be documented
### :boom: Breaking Change

* fix(instrumentation)!:remove unused description property from interface [#4847](https://github.com/open-telemetry/opentelemetry-js/pull/4847) @blumamir
* feat(exporter-*-otlp-*)!: use transport interface in node.js exporters [#4743](https://github.com/open-telemetry/opentelemetry-js/pull/4743) @pichlermarc
* (user-facing) `headers` was intended for internal use has been removed from all exporters
* (user-facing) `compression` was intended for internal use and has been removed from all exporters
* (user-facing) `hostname` was intended for use in tests and is not used by any exporters, it will be removed in a future release
* fix(exporter-*-otlp-*)!: ensure `User-Agent` header cannot be overwritten by the user [#4743](https://github.com/open-telemetry/opentelemetry-js/pull/4743) @pichlermarc
* allowing overrides of the `User-Agent` header was not specification compliant.

### :rocket: (Enhancement)

Expand Down Expand Up @@ -79,7 +85,7 @@ All notable changes to experimental packages in this project will be documented

### :bug: (Bug Fix)

* fix(instrumentation): Update `import-in-the-middle` to fix [numerous bugs](https://github.com/DataDog/import-in-the-middle/releases/tag/v1.8.0) [#4745](https://github.com/open-telemetry/opentelemetry-js/pull/4745) @timfish
* fix(instrumentation): Update `import-in-the-middle` to fix [numerous bugs](https://github.com/DataDog/import-in-the-middle/pull/91) [#4745](https://github.com/open-telemetry/opentelemetry-js/pull/4745) @timfish

### :books: (Refine Doc)

Expand Down
17 changes: 8 additions & 9 deletions experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,15 @@ export class OTLPLogExporter
...config,
},
JsonLogsSerializer,
'application/json'
{
...baggageUtils.parseKeyPairsIntoRecord(
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
...parseHeaders(config?.headers),
...USER_AGENT,
'Content-Type': 'application/json',
}
);
this.headers = {
...this.headers,
...USER_AGENT,
...baggageUtils.parseKeyPairsIntoRecord(
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
...parseHeaders(config?.headers),
};
}

getDefaultUrl(config: OTLPExporterNodeConfigBase): string {
Expand Down
79 changes: 53 additions & 26 deletions experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class MockedResponse extends Stream {
super();
}

send(data: string) {
send(data: Uint8Array) {
this.emit('data', data);
this.emit('end');
}
Expand All @@ -65,6 +65,9 @@ describe('OTLPLogExporter', () => {

afterEach(() => {
fakeRequest = new Stream.PassThrough();
Object.defineProperty(fakeRequest, 'setTimeout', {
value: function (_timeout: number) {},
});
sinon.restore();
});

Expand All @@ -83,15 +86,20 @@ describe('OTLPLogExporter', () => {
it('should include user-agent header by default', () => {
const exporter = new OTLPLogExporter();
assert.strictEqual(
exporter.headers['User-Agent'],
exporter['_transport']['_transport']['_parameters']['headers'][
'User-Agent'
],
`OTel-OTLP-Exporter-JavaScript/${VERSION}`
);
});

it('should use headers defined via env', () => {
envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=bar';
const exporter = new OTLPLogExporter();
assert.strictEqual(exporter.headers.foo, 'bar');
assert.strictEqual(
exporter['_transport']['_transport']['_parameters']['headers']['foo'],
'bar'
);
delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS;
});

Expand All @@ -106,13 +114,19 @@ describe('OTLPLogExporter', () => {

it('should override headers defined via env with headers defined in constructor', () => {
envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo';
const collectorExporter = new OTLPLogExporter({
const exporter = new OTLPLogExporter({
headers: {
foo: 'constructor',
},
});
assert.strictEqual(collectorExporter.headers.foo, 'constructor');
assert.strictEqual(collectorExporter.headers.bar, 'foo');
assert.strictEqual(
exporter['_transport']['_transport']['_parameters']['headers']['foo'],
'constructor'
);
assert.strictEqual(
exporter['_transport']['_transport']['_parameters']['headers']['bar'],
'foo'
);
envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
});
});
Expand Down Expand Up @@ -152,10 +166,12 @@ describe('OTLPLogExporter', () => {
assert.strictEqual(options.method, 'POST');
assert.strictEqual(options.path, '/');

const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send('success');
done();
queueMicrotask(() => {
const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send(Buffer.from('success'));
done();
});
return fakeRequest as any;
});
collectorExporter.export(logs, () => {});
Expand All @@ -165,10 +181,12 @@ describe('OTLPLogExporter', () => {
sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
assert.strictEqual(options.headers['foo'], 'bar');

const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send('success');
done();
queueMicrotask(() => {
const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send(Buffer.from('success'));
done();
});
return fakeRequest as any;
});

Expand All @@ -180,10 +198,12 @@ describe('OTLPLogExporter', () => {
assert.strictEqual(options.agent.keepAlive, true);
assert.strictEqual(options.agent.options.keepAliveMsecs, 2000);

const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send('success');
done();
queueMicrotask(() => {
const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send(Buffer.from('success'));
done();
});
return fakeRequest as any;
});

Expand All @@ -192,10 +212,13 @@ describe('OTLPLogExporter', () => {

it('should successfully send the logs', done => {
const fakeRequest = new Stream.PassThrough();
sinon.stub(http, 'request').returns(fakeRequest as any);
Object.defineProperty(fakeRequest, 'setTimeout', {
value: function (_timeout: number) {},
});

sinon.stub(http, 'request').returns(fakeRequest as any);
let buff = Buffer.from('');
fakeRequest.on('end', () => {
fakeRequest.on('finish', () => {
const responseBody = buff.toString();
const json = JSON.parse(responseBody) as IExportLogsServiceRequest;
const log1 = json.resourceLogs?.[0].scopeLogs?.[0].logRecords?.[0];
Expand All @@ -222,9 +245,11 @@ describe('OTLPLogExporter', () => {
const spyLoggerError = sinon.stub(diag, 'error');

sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send('success');
queueMicrotask(() => {
const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send(Buffer.from('success'));
});
return fakeRequest as any;
});

Expand All @@ -237,9 +262,11 @@ describe('OTLPLogExporter', () => {

it('should log the error message', done => {
sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
const mockResError = new MockedResponse(400);
cb(mockResError);
mockResError.send('failed');
queueMicrotask(() => {
const mockRes = new MockedResponse(400);
cb(mockRes);
mockRes.send(Buffer.from('failure'));
});

return fakeRequest as any;
});
Expand Down
12 changes: 5 additions & 7 deletions experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,14 @@ export class OTLPLogExporter
implements LogRecordExporter
{
constructor(config: OTLPExporterConfigBase = {}) {
super(config, ProtobufLogsSerializer, 'application/x-protobuf');
const env = getEnv();
this.headers = {
...this.headers,
...USER_AGENT,
super(config, ProtobufLogsSerializer, {
...baggageUtils.parseKeyPairsIntoRecord(
env.OTEL_EXPORTER_OTLP_LOGS_HEADERS
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
...parseHeaders(config?.headers),
};
...USER_AGENT,
'Content-Type': 'application/x-protobuf',
});
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
Expand Down
2 changes: 1 addition & 1 deletion experimental/packages/exporter-logs-otlp-proto/test/logHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export class MockedResponse extends Stream {
super();
}

send(data: string) {
send(data: Uint8Array) {
this.emit('data', data);
this.emit('end');
}
Expand Down
Loading

0 comments on commit 9bb4ff4

Please sign in to comment.