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(exporters)!: use transport interface in node.js exporters #4743

Merged
Show file tree
Hide file tree
Changes from all 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: 7 additions & 1 deletion experimental/CHANGELOG.md
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
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,
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this may change when #4447 is solved. Could you please add a reference here?

Copy link
Member Author

@pichlermarc pichlermarc Jun 21, 2024

Choose a reason for hiding this comment

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

@david-luna, sorry I think I've lost the train of thought from #4447.

Would you like me to add a reference that precedence may change

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a note in the CHANGELOG.md: bb5ec8c

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @pichlermarc !

Once the exporters are finalized I'll check other SDKs and ask in the spec SIG about #4447 and the possibility to override the UA via env vars.

'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
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
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
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