From fb97582ea86e40909cae09506e80eae6982f743d Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 8 May 2024 17:44:09 +0200 Subject: [PATCH 01/11] feat(exporters)!: use transport interface in node.js exporters --- .../test/node/OTLPLogExporter.test.ts | 59 +++-- .../test/logHelper.ts | 2 +- .../test/node/OTLPLogExporter.test.ts | 72 ++++--- .../test/node/CollectorTraceExporter.test.ts | 80 ++++--- .../test/node/nodeHelpers.ts | 2 +- .../test/node/OTLPTraceExporter.test.ts | 104 +++++---- .../test/traceHelper.ts | 2 +- .../test/node/CollectorMetricExporter.test.ts | 52 +++-- .../test/node/nodeHelpers.ts | 2 +- .../test/OTLPMetricExporter.test.ts | 203 ++++++++++-------- .../test/metricsHelper.ts | 2 +- .../src/export-response.ts | 10 +- .../src/exporter-transport.ts | 0 .../packages/otlp-exporter-base/src/index.ts | 8 + .../src/is-export-retryable.ts | 40 ++++ .../otlp-exporter-base/src/platform/index.ts | 3 - .../src/platform/node/OTLPExporterNodeBase.ts | 69 ++++-- .../platform/node/http-exporter-transport.ts | 67 ++++++ .../src/platform/node/http-transport-types.ts | 34 +++ .../src/platform/node/http-transport-utils.ts | 153 +++++++++++++ .../src/platform/node/index.ts | 1 - .../src/platform/node/util.ts | 169 --------------- .../otlp-exporter-base/test/node/util.test.ts | 188 ---------------- .../src/OTLPGRPCExporterNodeBase.ts | 2 +- .../src/grpc-exporter-transport.ts | 6 +- .../test/OTLPGRPCExporterNodeBase.test.ts | 7 +- .../test/grpc-exporter-transport.test.ts | 2 +- 27 files changed, 725 insertions(+), 614 deletions(-) rename experimental/packages/{otlp-grpc-exporter-base => otlp-exporter-base}/src/export-response.ts (79%) rename experimental/packages/{otlp-grpc-exporter-base => otlp-exporter-base}/src/exporter-transport.ts (100%) create mode 100644 experimental/packages/otlp-exporter-base/src/is-export-retryable.ts create mode 100644 experimental/packages/otlp-exporter-base/src/platform/node/http-exporter-transport.ts create mode 100644 experimental/packages/otlp-exporter-base/src/platform/node/http-transport-types.ts create mode 100644 experimental/packages/otlp-exporter-base/src/platform/node/http-transport-utils.ts diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index e7f1c92e5d8..b521006128c 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -32,6 +32,7 @@ import { PassThrough, Stream } from 'stream'; import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; import { ExportResultCode } from '@opentelemetry/core'; import { VERSION } from '../../src/version'; +import {nextTick} from "process"; let fakeRequest: PassThrough; @@ -43,7 +44,7 @@ class MockedResponse extends Stream { super(); } - send(data: string) { + send(data: Uint8Array) { this.emit('data', data); this.emit('end'); } @@ -65,6 +66,9 @@ describe('OTLPLogExporter', () => { afterEach(() => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.restore(); }); @@ -152,10 +156,12 @@ describe('OTLPLogExporter', () => { assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); collectorExporter.export(logs, () => {}); @@ -165,10 +171,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(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); @@ -180,10 +188,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(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); @@ -192,10 +202,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]; @@ -222,9 +235,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'); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + }); return fakeRequest as any; }); @@ -237,9 +252,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'); + nextTick(() => { + const mockRes = new MockedResponse(400); + cb(mockRes); + mockRes.send(Buffer.from('failure')); + }); return fakeRequest as any; }); diff --git a/experimental/packages/exporter-logs-otlp-proto/test/logHelper.ts b/experimental/packages/exporter-logs-otlp-proto/test/logHelper.ts index ab2f0885033..c5a110ca1ac 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/logHelper.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/logHelper.ts @@ -175,7 +175,7 @@ export class MockedResponse extends Stream { super(); } - send(data: string) { + send(data: Uint8Array) { this.emit('data', data); this.emit('end'); } diff --git a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts index 71a88635b9a..8e85cbbdfca 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts @@ -38,6 +38,7 @@ import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; import { VERSION } from '../../src/version'; import { Root } from 'protobufjs'; import * as path from 'path'; +import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -61,6 +62,9 @@ describe('OTLPLogExporter - node with proto over http', () => { afterEach(() => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.restore(); }); @@ -209,10 +213,12 @@ describe('OTLPLogExporter - node with proto over http', () => { assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); collectorExporter.export(logs, () => {}); @@ -222,10 +228,12 @@ describe('OTLPLogExporter - node with proto over http', () => { 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(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); collectorExporter.export(logs, () => {}); @@ -236,10 +244,12 @@ describe('OTLPLogExporter - node with proto over http', () => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); collectorExporter.export(logs, () => {}); @@ -247,6 +257,9 @@ describe('OTLPLogExporter - node with proto over http', () => { it('should successfully send the logs', done => { const fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.stub(http, 'request').returns(fakeRequest as any); let buff = Buffer.from(''); @@ -277,9 +290,11 @@ describe('OTLPLogExporter - node with proto over http', () => { 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'); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + }); return fakeRequest as any; }); @@ -292,9 +307,11 @@ describe('OTLPLogExporter - node with proto over http', () => { 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'); + nextTick(() => { + const mockRes = new MockedResponse(400); + cb(mockRes); + mockRes.send(Buffer.from('failure')); + }); return fakeRequest as any; }); @@ -329,6 +346,9 @@ describe('OTLPLogExporter - node with proto over http', () => { it('should successfully send the logs', done => { const fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.stub(http, 'request').returns(fakeRequest as any); const spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; @@ -368,7 +388,7 @@ describe('export - real http request destroyed before response received', () => setTimeout(() => { res.statusCode = 200; res.end(); - }, 200); + }, 1000); }); before(done => { server.listen(8082, done); @@ -386,11 +406,15 @@ describe('export - real http request destroyed before response received', () => logs.push(Object.assign({}, mockedReadableLogRecord)); collectorExporter.export(logs, result => { - assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); + try { + assert.strictEqual(result.code, ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + } catch (e) { + done(e); + } }); }); it('should log the timeout request error message when timeout is 100', done => { diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index e9af4ec37b4..f275d430cd7 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -51,6 +51,9 @@ describe('OTLPTraceExporter - node with json over http', () => { afterEach(() => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.restore(); }); @@ -234,11 +237,13 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); const options = args[0]; assert.strictEqual(options.hostname, 'foo.bar.com'); @@ -252,11 +257,13 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); const options = args[0]; assert.strictEqual(options.headers['foo'], 'bar'); @@ -268,11 +275,13 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); const options = args[0]; assert.strictEqual(options.headers['Content-Encoding'], undefined); @@ -284,11 +293,14 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); const options = args[0]; const agent = options.agent; @@ -302,12 +314,15 @@ describe('OTLPTraceExporter - node with json over http', () => { const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => {}); - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; + const mockRes = new MockedResponse(200); + + nextTick(() => { + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); - callback(mockRes); - mockRes.send('success'); clock.restore(); nextTick(() => { @@ -319,7 +334,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const callback2 = args2[1]; callback2(mockRes); - mockRes2.send('success'); + mockRes2.send(Buffer.from('success')); const [firstExportAgent, secondExportAgent] = stubRequest.args.map( a => a[0].agent @@ -334,7 +349,7 @@ describe('OTLPTraceExporter - node with json over http', () => { it('should successfully send the spans', done => { let buff = Buffer.from(''); - fakeRequest.on('end', () => { + fakeRequest.on('finish', () => { const responseBody = buff.toString(); const json = JSON.parse(responseBody) as IExportTraceServiceRequest; const span1 = json.resourceSpans?.[0].scopeSpans?.[0].spans?.[0]; @@ -357,7 +372,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const callback = args[1]; callback(mockRes); - mockRes.send('success'); + mockRes.send(Buffer.from('success')); }); it('should log the successful message', done => { @@ -367,12 +382,15 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, responseSpy); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); + setTimeout(() => { assert.strictEqual(stubLoggerError.args.length, 0); assert.strictEqual( @@ -389,11 +407,14 @@ describe('OTLPTraceExporter - node with json over http', () => { collectorExporter.export(spans, responseSpy); setTimeout(() => { - const mockResError = new MockedResponse(400); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockResError); - mockResError.send('failed'); + + nextTick(() => { + const mockRes = new MockedResponse(400); + callback(mockRes); + mockRes.send(Buffer.from('failure')); + }); setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; @@ -401,7 +422,6 @@ describe('OTLPTraceExporter - node with json over http', () => { const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.code, 400); - assert.strictEqual(error.data, 'failed'); done(); }); }); @@ -431,7 +451,7 @@ describe('OTLPTraceExporter - node with json over http', () => { it('should successfully send the spans', done => { let buff = Buffer.from(''); - fakeRequest.on('end', () => { + fakeRequest.on('finish', () => { const responseBody = zlib.gunzipSync(buff).toString(); const json = JSON.parse(responseBody) as IExportTraceServiceRequest; @@ -455,7 +475,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const callback = args[1]; callback(mockRes); - mockRes.send('success'); + mockRes.send(Buffer.from('success')); }); }); @@ -483,6 +503,9 @@ describe('OTLPTraceExporter - node with json over http', () => { describe('export - with timeout', () => { beforeEach(() => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; @@ -501,6 +524,7 @@ describe('OTLPTraceExporter - node with json over http', () => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); }); + it('should log the timeout request error message', done => { const responseSpy = sinon.spy(); collectorExporter.export(spans, responseSpy); @@ -513,7 +537,7 @@ describe('OTLPTraceExporter - node with json over http', () => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); const error = result.error as OTLPExporterError; assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); + assert.deepEqual(error, { code: 'ECONNRESET' }); done(); }); diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/nodeHelpers.ts b/experimental/packages/exporter-trace-otlp-http/test/node/nodeHelpers.ts index d2dce6517b2..e63d21b17c9 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/nodeHelpers.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/nodeHelpers.ts @@ -24,7 +24,7 @@ export class MockedResponse extends Stream { super(); } - send(data: string) { + send(data: Uint8Array) { this.emit('data', data); this.emit('end'); } diff --git a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts index 90c212e8aaa..9ccaef82bbc 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts @@ -38,6 +38,7 @@ import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; import { Root } from 'protobufjs'; import { VERSION } from '../../src/version'; import * as path from 'path'; +import { nextTick } from 'process'; const dir = path.resolve(__dirname, '../../../otlp-transformer/protos'); const root = new Root(); @@ -63,6 +64,9 @@ describe('OTLPTraceExporter - node with proto over http', () => { afterEach(() => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.restore(); }); @@ -211,14 +215,20 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should open the connection', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - assert.strictEqual(options.hostname, 'foo.bar.com'); - assert.strictEqual(options.method, 'POST'); - assert.strictEqual(options.path, '/'); - - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); + try { + assert.strictEqual(options.hostname, 'foo.bar.com'); + assert.strictEqual(options.method, 'POST'); + assert.strictEqual(options.path, '/'); + } catch (e) { + done(e); + } + + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); @@ -229,10 +239,13 @@ describe('OTLPTraceExporter - node with proto over http', () => { 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(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); + return fakeRequest as any; }); @@ -244,10 +257,13 @@ describe('OTLPTraceExporter - node with proto over http', () => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); + return fakeRequest as any; }); @@ -256,10 +272,13 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should successfully send the spans', done => { const fakeRequest = new Stream.PassThrough(); + 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 data = exportRequestServiceProto.decode(buff); const json = data?.toJSON() as IExportTraceServiceRequest; const span1 = json.resourceSpans?.[0].scopeSpans?.[0].spans?.[0]; @@ -286,24 +305,33 @@ describe('OTLPTraceExporter - node with proto over http', () => { 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'); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + }); + return fakeRequest as any; }); collectorExporter.export(spans, result => { - assert.strictEqual(result.code, ExportResultCode.SUCCESS); - assert.strictEqual(spyLoggerError.args.length, 0); - done(); + try { + assert.strictEqual(result.code, ExportResultCode.SUCCESS); + assert.strictEqual(spyLoggerError.args.length, 0); + done(); + } catch (e) { + done(e); + } }); }); 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'); + nextTick(() => { + const mockRes = new MockedResponse(400); + cb(mockRes); + mockRes.send(Buffer.from('failure')); + }); return fakeRequest as any; }); @@ -338,12 +366,15 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should successfully send the spans', done => { const fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.stub(http, 'request').returns(fakeRequest as any); const spySetHeader = sinon.spy(); (fakeRequest as any).setHeader = spySetHeader; let buff = Buffer.from(''); - fakeRequest.on('end', () => { + fakeRequest.on('finish', () => { const unzippedBuff = zlib.gunzipSync(buff); const data = exportRequestServiceProto.decode(unzippedBuff); const json = data?.toJSON() as IExportTraceServiceRequest; @@ -377,7 +408,7 @@ describe('export - real http request destroyed before response received', () => setTimeout(() => { res.statusCode = 200; res.end(); - }, 200); + }, 1000); }); before(done => { server.listen(8080, done); @@ -385,23 +416,6 @@ describe('export - real http request destroyed before response received', () => after(done => { server.close(done); }); - it('should log the timeout request error message when timeout is 1', done => { - collectorExporterConfig = { - url: 'http://localhost:8080', - timeoutMillis: 1, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); - - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }); it('should log the timeout request error message when timeout is 100', done => { collectorExporterConfig = { url: 'http://localhost:8080', diff --git a/experimental/packages/exporter-trace-otlp-proto/test/traceHelper.ts b/experimental/packages/exporter-trace-otlp-proto/test/traceHelper.ts index 3a21d9b79d9..efbdc032830 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/traceHelper.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/traceHelper.ts @@ -268,7 +268,7 @@ export class MockedResponse extends Stream { super(); } - send(data: string) { + send(data: Uint8Array) { this.emit('data', data); this.emit('end'); } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index b72ed5f8cb8..c74b2433645 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -56,6 +56,7 @@ import { } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; import { VERSION } from '../../src/version'; +import {nextTick} from "process"; let fakeRequest: PassThrough; @@ -74,6 +75,9 @@ describe('OTLPMetricExporter - node with json over http', () => { afterEach(async () => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); await shutdown(); sinon.restore(); }); @@ -468,11 +472,13 @@ describe('OTLPMetricExporter - node with json over http', () => { collectorExporter.export(metrics, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); const options = args[0]; assert.strictEqual(options.hostname, 'foo.bar.com'); @@ -486,11 +492,14 @@ describe('OTLPMetricExporter - node with json over http', () => { collectorExporter.export(metrics, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); + const options = args[0]; assert.strictEqual(options.headers['foo'], 'bar'); done(); @@ -501,11 +510,14 @@ describe('OTLPMetricExporter - node with json over http', () => { collectorExporter.export(metrics, () => {}); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); const options = args[0]; const agent = options.agent; assert.strictEqual(agent.keepAlive, true); @@ -584,7 +596,7 @@ describe('OTLPMetricExporter - node with json over http', () => { const callback = args[1]; callback(mockRes); - mockRes.send('success'); + mockRes.send(Buffer.from('success')); }); it('should log the successful message', done => { @@ -595,11 +607,15 @@ describe('OTLPMetricExporter - node with json over http', () => { collectorExporter.export(metrics, responseSpy); setTimeout(() => { - const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('success'); + + nextTick(() => { + const mockRes = new MockedResponse(200); + callback(mockRes); + mockRes.send(Buffer.from('success')); + }); + setTimeout(() => { assert.strictEqual(stubLoggerError.args.length, 0); assert.strictEqual( @@ -619,18 +635,20 @@ describe('OTLPMetricExporter - node with json over http', () => { collectorExporter.export(metrics, responseSpy); setTimeout(() => { - const mockRes = new MockedResponse(400); const args = stubRequest.args[0]; const callback = args[1]; - callback(mockRes); - mockRes.send('failed'); + nextTick(() => { + const mockRes = new MockedResponse(400); + callback(mockRes); + mockRes.send(Buffer.from('failure')); + }); + setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.code, 400); - assert.strictEqual(error.data, 'failed'); done(); }); }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/nodeHelpers.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/nodeHelpers.ts index d2dce6517b2..e63d21b17c9 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/nodeHelpers.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/nodeHelpers.ts @@ -24,7 +24,7 @@ export class MockedResponse extends Stream { super(); } - send(data: string) { + send(data: Uint8Array) { this.emit('data', data); this.emit('end'); } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index 969f4334582..eede365e9fe 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -44,6 +44,7 @@ import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { VERSION } from '../src/version'; import { Root } from 'protobufjs'; import * as path from 'path'; +import {nextTick} from "process"; let fakeRequest: PassThrough; @@ -70,6 +71,9 @@ describe('OTLPMetricExporter - node with proto over http', () => { afterEach(() => { fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.restore(); }); @@ -246,10 +250,12 @@ describe('OTLPMetricExporter - node with proto over http', () => { assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); @@ -260,11 +266,12 @@ describe('OTLPMetricExporter - node with proto over http', () => { 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(); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); return fakeRequest as any; }); @@ -273,14 +280,19 @@ describe('OTLPMetricExporter - node with proto over http', () => { it('should have keep alive and keepAliveMsecs option set', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - assert.strictEqual(options.agent.keepAlive, true); - assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - - done(); + try { + assert.strictEqual(options.agent.keepAlive, true); + assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); + + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + done(); + }); + } catch (e) { + done(e); + } return fakeRequest as any; }); @@ -289,74 +301,84 @@ describe('OTLPMetricExporter - node with proto over http', () => { it('should successfully send metrics', done => { const fakeRequest = new Stream.PassThrough(); + Object.defineProperty(fakeRequest, 'setTimeout', { + value: function (_timeout: number) {}, + }); sinon.stub(http, 'request').returns(fakeRequest as any); let buff = Buffer.from(''); - fakeRequest.on('end', () => { - const data = exportRequestServiceProto.decode(buff); - const json = data?.toJSON() as any; - - // The order of the metrics is not guaranteed. - const counterIndex = metrics.scopeMetrics[0].metrics.findIndex( - it => it.descriptor.name === 'int-counter' - ); - const observableIndex = metrics.scopeMetrics[0].metrics.findIndex( - it => it.descriptor.name === 'double-observable-gauge' - ); - const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex( - it => it.descriptor.name === 'int-histogram' - ); - - const metric1 = - json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; - const metric2 = - json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; - const metric3 = - json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; - - assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); - ensureExportedCounterIsCorrect( - metric1, - metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime, - metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].startTime - ); - assert.ok( - typeof metric2 !== 'undefined', - "observable gauge doesn't exist" - ); - ensureExportedObservableGaugeIsCorrect( - metric2, - metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] - .endTime, - metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] - .startTime - ); - assert.ok( - typeof metric3 !== 'undefined', - "value recorder doesn't exist" - ); - ensureExportedHistogramIsCorrect( - metric3, - metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0].endTime, - metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] - .startTime, - [0, 100], - ['0', '2', '0'] - ); - - ensureExportMetricsServiceRequestIsSet(json); - done(); + fakeRequest.on('finish', () => { + try { + const data = exportRequestServiceProto.decode(buff); + const json = data?.toJSON() as any; + + // The order of the metrics is not guaranteed. + const counterIndex = metrics.scopeMetrics[0].metrics.findIndex( + it => it.descriptor.name === 'int-counter' + ); + const observableIndex = metrics.scopeMetrics[0].metrics.findIndex( + it => it.descriptor.name === 'double-observable-gauge' + ); + const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex( + it => it.descriptor.name === 'int-histogram' + ); + + const metric1 = + json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; + const metric2 = + json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; + const metric3 = + json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; + + assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); + ensureExportedCounterIsCorrect( + metric1, + metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime, + metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0] + .startTime + ); + assert.ok( + typeof metric2 !== 'undefined', + "observable gauge doesn't exist" + ); + ensureExportedObservableGaugeIsCorrect( + metric2, + metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] + .endTime, + metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] + .startTime + ); + assert.ok( + typeof metric3 !== 'undefined', + "value recorder doesn't exist" + ); + ensureExportedHistogramIsCorrect( + metric3, + metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] + .endTime, + metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] + .startTime, + [0, 100], + ['0', '2', '0'] + ); + + ensureExportMetricsServiceRequestIsSet(json); + done(); + } catch (e) { + done(e); + } }); fakeRequest.on('data', chunk => { buff = Buffer.concat([buff, chunk]); }); - const clock = sinon.useFakeTimers(); - collectorExporter.export(metrics, () => {}); - clock.tick(200); - clock.restore(); + try { + collectorExporter.export(metrics, () => {}); + } catch (error) { + done(error); + } }); it('should log the successful message', done => { @@ -364,9 +386,12 @@ describe('OTLPMetricExporter - node with proto over http', () => { 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'); + nextTick(() => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send(Buffer.from('success')); + }); + return fakeRequest as any; }); @@ -377,20 +402,26 @@ describe('OTLPMetricExporter - node with proto over http', () => { }); }); - it('should log the error message', done => { + it('should return the error code message', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - const mockResError = new MockedResponse(400); - cb(mockResError); - mockResError.send('failed'); + nextTick(() => { + const mockRes = new MockedResponse(400); + cb(mockRes); + mockRes.send(Buffer.from('failure')); + }); return fakeRequest as any; }); collectorExporter.export(metrics, result => { - assert.strictEqual(result.code, ExportResultCode.FAILED); - // @ts-expect-error verify error code - assert.strictEqual(result.error.code, 400); - done(); + try { + assert.strictEqual(result.code, ExportResultCode.FAILED); + // @ts-expect-error verify error code + assert.strictEqual(result.error.code, 400); + done(); + } catch (e) { + done(e); + } }); }); }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts index ef70a75c054..effc732b370 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts @@ -227,7 +227,7 @@ export class MockedResponse extends Stream { super(); } - send(data: string) { + send(data: Uint8Array) { this.emit('data', data); this.emit('end'); } diff --git a/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts b/experimental/packages/otlp-exporter-base/src/export-response.ts similarity index 79% rename from experimental/packages/otlp-grpc-exporter-base/src/export-response.ts rename to experimental/packages/otlp-exporter-base/src/export-response.ts index c13af631e13..4e0eb9703e8 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts +++ b/experimental/packages/otlp-exporter-base/src/export-response.ts @@ -24,4 +24,12 @@ export interface ExportResponseFailure { error: Error; } -export type ExportResponse = ExportResponseSuccess | ExportResponseFailure; +export interface ExportResponseRetryable { + status: 'retryable'; + retryInMillis?: number; +} + +export type ExportResponse = + | ExportResponseSuccess + | ExportResponseFailure + | ExportResponseRetryable; diff --git a/experimental/packages/otlp-grpc-exporter-base/src/exporter-transport.ts b/experimental/packages/otlp-exporter-base/src/exporter-transport.ts similarity index 100% rename from experimental/packages/otlp-grpc-exporter-base/src/exporter-transport.ts rename to experimental/packages/otlp-exporter-base/src/exporter-transport.ts diff --git a/experimental/packages/otlp-exporter-base/src/index.ts b/experimental/packages/otlp-exporter-base/src/index.ts index 9ded1037826..c6a7b815f96 100644 --- a/experimental/packages/otlp-exporter-base/src/index.ts +++ b/experimental/packages/otlp-exporter-base/src/index.ts @@ -27,3 +27,11 @@ export { configureExporterTimeout, invalidTimeout, } from './util'; + +export { + ExportResponse, + ExportResponseFailure, + ExportResponseSuccess, +} from './export-response'; + +export { IExporterTransport } from './exporter-transport'; diff --git a/experimental/packages/otlp-exporter-base/src/is-export-retryable.ts b/experimental/packages/otlp-exporter-base/src/is-export-retryable.ts new file mode 100644 index 00000000000..8b4569987bb --- /dev/null +++ b/experimental/packages/otlp-exporter-base/src/is-export-retryable.ts @@ -0,0 +1,40 @@ +/* + * 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. + */ + +export function isExportRetryable(statusCode: number): boolean { + const retryCodes = [429, 502, 503, 504]; + return retryCodes.includes(statusCode); +} + +export function parseRetryAfterToMills( + retryAfter?: string | undefined | null +): number | undefined { + if (retryAfter == null) { + return undefined; + } + + const seconds = Number.parseInt(retryAfter, 10); + if (Number.isInteger(seconds)) { + return seconds > 0 ? seconds * 1000 : -1; + } + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After#directives + const delay = new Date(retryAfter).getTime() - Date.now(); + + if (delay >= 0) { + return delay; + } + return 0; +} diff --git a/experimental/packages/otlp-exporter-base/src/platform/index.ts b/experimental/packages/otlp-exporter-base/src/platform/index.ts index fc857a58020..08a2b63613f 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/index.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/index.ts @@ -16,9 +16,6 @@ export { OTLPExporterNodeBase, - sendWithHttp, - createHttpAgent, - configureCompression, OTLPExporterNodeConfigBase, CompressionAlgorithm, } from './node'; diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts index 5d39153831d..9af1773000a 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts @@ -14,17 +14,16 @@ * limitations under the License. */ -import type * as http from 'http'; -import type * as https from 'https'; - import { OTLPExporterBase } from '../../OTLPExporterBase'; import { OTLPExporterNodeConfigBase, CompressionAlgorithm } from './types'; -import * as otlpTypes from '../../types'; import { parseHeaders } from '../../util'; -import { createHttpAgent, sendWithHttp, configureCompression } from './util'; +import { configureCompression } from './util'; import { diag } from '@opentelemetry/api'; import { getEnv, baggageUtils } from '@opentelemetry/core'; import { ISerializer } from '@opentelemetry/otlp-transformer'; +import { IExporterTransport } from '../../exporter-transport'; +import { createHttpExporterTransport } from './http-exporter-transport'; +import { OTLPExporterError } from '../../types'; /** * Collector Metric Exporter abstract base class @@ -35,10 +34,9 @@ export abstract class OTLPExporterNodeBase< > extends OTLPExporterBase { DEFAULT_HEADERS: Record = {}; headers: Record; - agent: http.Agent | https.Agent | undefined; compression: CompressionAlgorithm; private _serializer: ISerializer; - private _contentType: string; + private _transport: IExporterTransport; constructor( config: OTLPExporterNodeConfigBase = {}, @@ -46,7 +44,6 @@ export abstract class OTLPExporterNodeBase< contentType: string ) { super(config); - this._contentType = contentType; // eslint-disable-next-line @typescript-eslint/no-explicit-any if ((config as any).metadata) { diag.warn('Metadata cannot be set when using http'); @@ -54,11 +51,35 @@ export abstract class OTLPExporterNodeBase< this.headers = Object.assign( this.DEFAULT_HEADERS, parseHeaders(config.headers), - baggageUtils.parseKeyPairsIntoRecord(getEnv().OTEL_EXPORTER_OTLP_HEADERS) + baggageUtils.parseKeyPairsIntoRecord(getEnv().OTEL_EXPORTER_OTLP_HEADERS), + { 'Content-Type': contentType } ); - this.agent = createHttpAgent(config); this.compression = configureCompression(config.compression); this._serializer = serializer; + + // populate keepAlive for use with new settings + if (config?.keepAlive != null) { + if (config.httpAgentOptions != null) { + if (config.httpAgentOptions.keepAlive == null) { + // specific setting is not set, populate with non-specific setting. + config.httpAgentOptions.keepAlive = config.keepAlive; + } + // do nothing, use specific setting otherwise + } else { + // populate specific option if AgentOptions does not exist. + config.httpAgentOptions = { + keepAlive: config.keepAlive, + }; + } + } + + this._transport = createHttpExporterTransport({ + agentOptions: config.httpAgentOptions ?? { keepAlive: true }, + compression: this.compression, + headers: this.headers, + url: this.url, + timeoutMillis: this.timeoutMillis, + }); } onInit(_config: OTLPExporterNodeConfigBase): void {} @@ -66,22 +87,30 @@ export abstract class OTLPExporterNodeBase< send( objects: ExportItem[], onSuccess: () => void, - onError: (error: otlpTypes.OTLPExporterError) => void + onError: (error: OTLPExporterError) => void ): void { if (this._shutdownOnce.isCalled) { diag.debug('Shutdown already started. Cannot send objects'); return; } - const promise = new Promise((resolve, reject) => { - sendWithHttp( - this, - this._serializer.serializeRequest(objects) ?? new Uint8Array(), - this._contentType, - resolve, - reject - ); - }).then(onSuccess, onError); + const data = this._serializer.serializeRequest(objects); + + if (data == null) { + onError(new Error('Could not serialize message')); + return; + } + + const promise = this._transport.send(data).then(response => { + if (response.status === 'success') { + onSuccess(); + return; + } + if (response.status === 'failure' && response.error) { + onError(response.error); + } + onError(new OTLPExporterError('Export failed with unknown error')); + }, onError); this._sendingPromises.push(promise); const popPromise = () => { diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/http-exporter-transport.ts b/experimental/packages/otlp-exporter-base/src/platform/node/http-exporter-transport.ts new file mode 100644 index 00000000000..79647f1d9b3 --- /dev/null +++ b/experimental/packages/otlp-exporter-base/src/platform/node/http-exporter-transport.ts @@ -0,0 +1,67 @@ +/* + * 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 type { + HttpRequestParameters, + sendWithHttp, +} from './http-transport-types'; + +// NOTE: do not change these type imports to actual imports. Doing so WILL break `@opentelemetry/instrumentation-http`, +// as they'd be imported before the http/https modules can be wrapped. +import type * as https from 'https'; +import type * as http from 'http'; +import { ExportResponse } from '../../export-response'; +import { IExporterTransport } from '../../exporter-transport'; + +class HttpExporterTransport implements IExporterTransport { + private _send: sendWithHttp | null = null; + private _agent: http.Agent | https.Agent | null = null; + + constructor(private _parameters: HttpRequestParameters) {} + + async send(data: Uint8Array): Promise { + if (this._send == null) { + // Lazy require to ensure that http/https is not required before instrumentations can wrap it. + const { + sendWithHttp, + createHttpAgent, + // eslint-disable-next-line @typescript-eslint/no-var-requires + } = require('./http-transport-utils'); + this._agent = createHttpAgent( + this._parameters.url, + this._parameters.agentOptions + ); + this._send = sendWithHttp; + } + + return new Promise(resolve => { + // this will always be defined + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this._send?.(this._parameters, this._agent!, data, result => { + resolve(result); + }); + }); + } + shutdown() { + // intentionally left empty, nothing to do. + } +} + +export function createHttpExporterTransport( + parameters: HttpRequestParameters +): IExporterTransport { + return new HttpExporterTransport(parameters); +} diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/http-transport-types.ts b/experimental/packages/otlp-exporter-base/src/platform/node/http-transport-types.ts new file mode 100644 index 00000000000..da33d02cd93 --- /dev/null +++ b/experimental/packages/otlp-exporter-base/src/platform/node/http-transport-types.ts @@ -0,0 +1,34 @@ +/* + * 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 type * as http from 'http'; +import type * as https from 'https'; +import { ExportResponse } from '../../export-response'; + +export type sendWithHttp = ( + params: HttpRequestParameters, + agent: http.Agent | https.Agent, + data: Uint8Array, + onDone: (response: ExportResponse) => void +) => void; + +export interface HttpRequestParameters { + timeoutMillis: number; + url: string; + headers: Record; + compression: 'gzip' | 'none'; + agentOptions: http.AgentOptions | https.AgentOptions; +} diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/http-transport-utils.ts b/experimental/packages/otlp-exporter-base/src/platform/node/http-transport-utils.ts new file mode 100644 index 00000000000..e1c13855a55 --- /dev/null +++ b/experimental/packages/otlp-exporter-base/src/platform/node/http-transport-utils.ts @@ -0,0 +1,153 @@ +/* + * 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 http from 'http'; +import * as https from 'https'; +import * as zlib from 'zlib'; +import { Readable } from 'stream'; +import { HttpRequestParameters } from './http-transport-types'; +import { ExportResponse } from '../../export-response'; +import { + isExportRetryable, + parseRetryAfterToMills, +} from '../../is-export-retryable'; +import { OTLPExporterError } from '../../types'; + +export const DEFAULT_EXPORT_INITIAL_BACKOFF = 1000; +export const DEFAULT_EXPORT_MAX_BACKOFF = 5000; +export const DEFAULT_EXPORT_BACKOFF_MULTIPLIER = 1.5; + +/** + * Sends data using http + * @param params + * @param agent + * @param data + * @param onDone + */ +export function sendWithHttp( + params: HttpRequestParameters, + agent: http.Agent | https.Agent, + data: Uint8Array, + onDone: (response: ExportResponse) => void +): void { + const parsedUrl = new URL(params.url); + const nodeVersion = Number(process.versions.node.split('.')[0]); + + const options: http.RequestOptions | https.RequestOptions = { + hostname: parsedUrl.hostname, + port: parsedUrl.port, + path: parsedUrl.pathname, + method: 'POST', + headers: { + ...params.headers, + }, + agent: agent, + }; + + const request = parsedUrl.protocol === 'http:' ? http.request : https.request; + + const req = request(options, (res: http.IncomingMessage) => { + const responseData: Buffer[] = []; + res.on('data', chunk => responseData.push(chunk)); + + res.on('end', () => { + if (req.destroyed) { + return; + } + if (res.statusCode && res.statusCode < 299) { + onDone({ + status: 'success', + data: Buffer.concat(responseData), + }); + } else if (res.statusCode && isExportRetryable(res.statusCode)) { + onDone({ + status: 'retryable', + retryInMillis: parseRetryAfterToMills(res.headers['retry-after']), + }); + } else { + const error = new OTLPExporterError(res.statusMessage, res.statusCode); + onDone({ + status: 'failure', + error, + }); + } + }); + }); + + req.setTimeout(params.timeoutMillis, () => { + req.destroy(); + onDone({ + status: 'failure', + error: new Error('Request Timeout'), + }); + }); + req.on('error', (error: Error | any) => { + onDone({ + status: 'failure', + error: error, + }); + }); + + const reportTimeoutErrorEvent = nodeVersion >= 14 ? 'close' : 'abort'; + req.on(reportTimeoutErrorEvent, () => { + onDone({ + status: 'failure', + error: new Error('Request timed out'), + }); + }); + + compressAndSend(req, params.compression, data, (error: Error) => { + onDone({ + status: 'failure', + error, + }); + }); +} + +function compressAndSend( + req: http.ClientRequest, + compression: 'gzip' | 'none', + data: Uint8Array, + onError: (error: Error) => void +) { + let dataStream = readableFromUint8Array(data); + + if (compression === 'gzip') { + req.setHeader('Content-Encoding', 'gzip'); + dataStream = dataStream + .on('error', onError) + .pipe(zlib.createGzip()) + .on('error', onError); + } + + dataStream.pipe(req); +} + +function readableFromUint8Array(buff: string | Uint8Array): Readable { + const readable = new Readable(); + readable.push(buff); + readable.push(null); + + return readable; +} + +export function createHttpAgent( + rawUrl: string, + agentOptions: http.AgentOptions | https.AgentOptions +) { + const parsedUrl = new URL(rawUrl); + const Agent = parsedUrl.protocol === 'http:' ? http.Agent : https.Agent; + return new Agent(agentOptions); +} diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/index.ts b/experimental/packages/otlp-exporter-base/src/platform/node/index.ts index b8b13bda202..fcfca512a86 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/index.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/index.ts @@ -15,5 +15,4 @@ */ export { OTLPExporterNodeBase } from './OTLPExporterNodeBase'; -export { sendWithHttp, createHttpAgent, configureCompression } from './util'; export { OTLPExporterNodeConfigBase, CompressionAlgorithm } from './types'; diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts index d9bca47088f..06a55af9ed0 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -16,179 +16,10 @@ import * as url from 'url'; import * as http from 'http'; import * as https from 'https'; -import * as zlib from 'zlib'; -import { Readable } from 'stream'; -import { OTLPExporterNodeBase } from './OTLPExporterNodeBase'; import { OTLPExporterNodeConfigBase } from '.'; import { diag } from '@opentelemetry/api'; import { CompressionAlgorithm } from './types'; import { getEnv } from '@opentelemetry/core'; -import { OTLPExporterError } from '../../types'; -import { - DEFAULT_EXPORT_MAX_ATTEMPTS, - DEFAULT_EXPORT_INITIAL_BACKOFF, - DEFAULT_EXPORT_BACKOFF_MULTIPLIER, - DEFAULT_EXPORT_MAX_BACKOFF, - isExportRetryable, - parseRetryAfterToMills, -} from '../../util'; - -/** - * Sends data using http - * @param collector - * @param data - * @param contentType - * @param onSuccess - * @param onError - */ -export function sendWithHttp( - collector: OTLPExporterNodeBase, - data: string | Uint8Array, - contentType: string, - onSuccess: () => void, - onError: (error: OTLPExporterError) => void -): void { - const exporterTimeout = collector.timeoutMillis; - const parsedUrl = new url.URL(collector.url); - const nodeVersion = Number(process.versions.node.split('.')[0]); - let retryTimer: ReturnType; - let req: http.ClientRequest; - let reqIsDestroyed = false; - - const exporterTimer = setTimeout(() => { - clearTimeout(retryTimer); - reqIsDestroyed = true; - - if (req.destroyed) { - const err = new OTLPExporterError('Request Timeout'); - onError(err); - } else { - // req.abort() was deprecated since v14 - nodeVersion >= 14 ? req.destroy() : req.abort(); - } - }, exporterTimeout); - - const options: http.RequestOptions | https.RequestOptions = { - hostname: parsedUrl.hostname, - port: parsedUrl.port, - path: parsedUrl.pathname, - method: 'POST', - headers: { - 'Content-Type': contentType, - ...collector.headers, - }, - agent: collector.agent, - }; - - const request = parsedUrl.protocol === 'http:' ? http.request : https.request; - - const sendWithRetry = ( - retries = DEFAULT_EXPORT_MAX_ATTEMPTS, - minDelay = DEFAULT_EXPORT_INITIAL_BACKOFF - ) => { - req = request(options, (res: http.IncomingMessage) => { - let responseData = ''; - res.on('data', chunk => (responseData += chunk)); - - res.on('aborted', () => { - if (reqIsDestroyed) { - const err = new OTLPExporterError('Request Timeout'); - onError(err); - } - }); - - res.on('end', () => { - if (reqIsDestroyed === false) { - if (res.statusCode && res.statusCode < 299) { - diag.debug(`statusCode: ${res.statusCode}`, responseData); - onSuccess(); - // clear all timers since request was completed and promise was resolved - clearTimeout(exporterTimer); - clearTimeout(retryTimer); - } else if ( - res.statusCode && - isExportRetryable(res.statusCode) && - retries > 0 - ) { - let retryTime: number; - minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; - - // retry after interval specified in Retry-After header - if (res.headers['retry-after']) { - retryTime = parseRetryAfterToMills(res.headers['retry-after']!); - } else { - // exponential backoff with jitter - retryTime = Math.round( - Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + - minDelay - ); - } - - retryTimer = setTimeout(() => { - sendWithRetry(retries - 1, minDelay); - }, retryTime); - } else { - const error = new OTLPExporterError( - res.statusMessage, - res.statusCode, - responseData - ); - onError(error); - // clear all timers since request was completed and promise was resolved - clearTimeout(exporterTimer); - clearTimeout(retryTimer); - } - } - }); - }); - - req.on('error', (error: Error | any) => { - if (reqIsDestroyed) { - const err = new OTLPExporterError('Request Timeout', error.code); - onError(err); - } else { - onError(error); - } - clearTimeout(exporterTimer); - clearTimeout(retryTimer); - }); - - req.on('abort', () => { - if (reqIsDestroyed) { - const err = new OTLPExporterError('Request Timeout'); - onError(err); - } - clearTimeout(exporterTimer); - clearTimeout(retryTimer); - }); - - switch (collector.compression) { - case CompressionAlgorithm.GZIP: { - req.setHeader('Content-Encoding', 'gzip'); - const dataStream = readableFromUnit8Array(data); - dataStream - .on('error', onError) - .pipe(zlib.createGzip()) - .on('error', onError) - .pipe(req); - - break; - } - default: - req.end(Buffer.from(data)); - break; - } - }; - sendWithRetry(); -} - -function readableFromUnit8Array(buff: string | Uint8Array): Readable { - const readable = new Readable(); - readable.push(buff); - readable.push(null); - - return readable; -} export function createHttpAgent( config: OTLPExporterNodeConfigBase diff --git a/experimental/packages/otlp-exporter-base/test/node/util.test.ts b/experimental/packages/otlp-exporter-base/test/node/util.test.ts index 0d3891cae63..9ba2d55592f 100644 --- a/experimental/packages/otlp-exporter-base/test/node/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/node/util.test.ts @@ -16,7 +16,6 @@ import * as assert from 'assert'; import { configureExporterTimeout, invalidTimeout } from '../../src/util'; -import { sendWithHttp } from '../../src/platform/node/util'; import { CompressionAlgorithm } from '../../src/platform/node/types'; import { configureCompression } from '../../src/platform/node/util'; import { diag } from '@opentelemetry/api'; @@ -24,31 +23,8 @@ import * as sinon from 'sinon'; import { OTLPExporterNodeBase } from '../../src/platform/node/OTLPExporterNodeBase'; import { OTLPExporterNodeConfigBase } from '../../src/platform/node/types'; -import { OTLPExporterError } from '../../src/types'; -import { PassThrough } from 'stream'; -import * as http from 'http'; -import * as zlib from 'zlib'; import { ISerializer } from '@opentelemetry/otlp-transformer'; -// Meant to simulate http.IncomingMessage, at least the parts that sendWithHttp cares about -// but make it a PassThrough so we can inspect it for the test -class HttpResponse extends PassThrough { - statusCode: number; - statusMessage: string; - - constructor(statusCode = 200, statusMessage = 'OK') { - super(); - this.statusCode = statusCode; - this.statusMessage = statusMessage; - } -} - -// Meant to simulate http.ClientRequest, at least the parts that sendWithHttp cares about -// but make it a PassThrough so we can inspect it for the test -class HttpRequest extends PassThrough { - setHeader(name: string, value: string) {} -} - // Barebones exporter for use by sendWithHttp type ExporterConfig = OTLPExporterNodeConfigBase; class Exporter extends OTLPExporterNodeBase { @@ -188,167 +164,3 @@ describe('configureCompression', () => { ); }); }); - -describe('sendWithHttp', () => { - let exporter: Exporter; - let httpRequestStub: sinon.SinonStub; - let mockRequest: HttpRequest; - let setHeaderSpy: sinon.SinonSpy; - - const spanData: object = { - foo: 'bar', - bar: 'baz', - }; - - beforeEach(() => { - // Create stub of http.request (used by sendWithHttp) - httpRequestStub = sinon.stub(http, 'request'); - - // Mock out a request - mockRequest = new HttpRequest(); - setHeaderSpy = sinon.spy(mockRequest, 'setHeader'); - - // Mock out response - const response = new HttpResponse(); - response.end('OK'); - - // Stub out http.request so it calls our callback with the mocked response - // and also so it returns our mocked request stream so we can observe. We don't - // really care about the response for the purpose of this test, but we do want - // to observe the request compression behavior. - httpRequestStub.returns(mockRequest).callsArgWith(1, response); - }); - - afterEach(function () { - httpRequestStub.restore(); - setHeaderSpy.restore(); - }); - - it('should send with no compression if configured to do so', () => { - exporter = new Exporter( - { - url: 'http://foobar.com', - compression: CompressionAlgorithm.NONE, - }, - noopSerializer, - '' - ); - const data = JSON.stringify(spanData); - - // Show that data is written to the request stream - let requestData = ''; - mockRequest.on('data', chunk => (requestData += chunk)); - mockRequest.on('end', () => { - assert.strictEqual(requestData, data); - }); - - // use fake timers to replace setTimeout in sendWithHttp function - const clock = sinon.useFakeTimers(); - - sendWithHttp( - exporter, - data, - 'application/json', - () => { - // Show that we aren't setting the gzip encoding header - assert(setHeaderSpy.withArgs('Content-Encoding', 'gzip').notCalled); - }, - (err: OTLPExporterError) => { - assert.fail(err); - } - ); - - clock.restore(); - }); - - it('should send with gzip compression if configured to do so', () => { - exporter = new Exporter( - { - url: 'http://foobar.com', - compression: CompressionAlgorithm.GZIP, - }, - noopSerializer, - '' - ); - - const data = JSON.stringify(spanData); - const compressedData = zlib.gzipSync(Buffer.from(data)); - - // Show that compressed data is written to the request stream - const buffers: Buffer[] = []; - mockRequest.on('data', chunk => buffers.push(Buffer.from(chunk))); - mockRequest.on('end', () => { - assert(Buffer.concat(buffers).equals(compressedData)); - }); - - // use fake timers to replace setTimeout in sendWithHttp function - const clock = sinon.useFakeTimers(); - - sendWithHttp( - exporter, - data, - 'application/json', - () => { - // Show that we are setting the gzip encoding header - assert(setHeaderSpy.withArgs('Content-Encoding', 'gzip').calledOnce); - }, - (err: OTLPExporterError) => { - assert.fail(err); - } - ); - - clock.restore(); - }); - - it('should work with gzip compression enabled even after multiple requests', () => { - exporter = new Exporter( - { - url: 'http://foobar.com', - compression: CompressionAlgorithm.GZIP, - }, - noopSerializer, - '' - ); - - const data = JSON.stringify(spanData); - const compressedData = zlib.gzipSync(Buffer.from(data)); - - for (let i = 0; i < 5; i++) { - mockRequest = new HttpRequest(); - setHeaderSpy.restore(); - setHeaderSpy = sinon.spy(mockRequest, 'setHeader'); - - const response = new HttpResponse(); - response.end('OK'); - - httpRequestStub.restore(); - httpRequestStub = sinon.stub(http, 'request'); - httpRequestStub.returns(mockRequest).callsArgWith(1, response); - - // Show that compressed data is written to the request stream - const buffers: Buffer[] = []; - mockRequest.on('data', chunk => buffers.push(Buffer.from(chunk))); - mockRequest.on('end', () => { - assert(Buffer.concat(buffers).equals(compressedData)); - }); - - // use fake timers to replace setTimeout in sendWithHttp function - const clock = sinon.useFakeTimers(); - - sendWithHttp( - exporter, - data, - 'application/json', - () => { - // Show that we are setting the gzip encoding header - assert(setHeaderSpy.withArgs('Content-Encoding', 'gzip').calledOnce); - }, - (err: OTLPExporterError) => { - assert.fail(err); - } - ); - - clock.restore(); - } - }); -}); diff --git a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts index 8ffc1458b25..646e674bf5c 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts @@ -28,7 +28,7 @@ import { } from './grpc-exporter-transport'; import { configureCompression, configureCredentials } from './util'; import { ISerializer } from '@opentelemetry/otlp-transformer'; -import { IExporterTransport } from './exporter-transport'; +import { IExporterTransport } from '@opentelemetry/otlp-exporter-base'; /** * OTLP Exporter abstract base class diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index 81e7ff4c4d1..342e83d91e6 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -22,8 +22,10 @@ import type { ChannelCredentials, Client, } from '@grpc/grpc-js'; -import { ExportResponse } from './export-response'; -import { IExporterTransport } from './exporter-transport'; +import { + ExportResponse, + IExporterTransport, +} from '@opentelemetry/otlp-exporter-base'; // values taken from '@grpc/grpc-js` so that we don't need to require/import it. const GRPC_COMPRESSION_NONE = 0; diff --git a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts index d1fe77f962b..99677dd05a7 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts @@ -19,8 +19,11 @@ import * as assert from 'assert'; import { OTLPGRPCExporterNodeBase } from '../src/OTLPGRPCExporterNodeBase'; import { OTLPGRPCExporterConfigNode } from '../src/types'; import { mockedReadableSpan } from './traceHelper'; -import { ExportResponse, ExportResponseSuccess } from '../src/export-response'; -import { IExporterTransport } from '../src/exporter-transport'; +import { + ExportResponse, + ExportResponseSuccess, + IExporterTransport, +} from '@opentelemetry/otlp-exporter-base'; import { ISerializer } from '@opentelemetry/otlp-transformer'; import sinon = require('sinon'); diff --git a/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts index f7e9769f45d..6a08727fe8a 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts @@ -28,7 +28,7 @@ import { types } from 'util'; import { ExportResponseFailure, ExportResponseSuccess, -} from '../src/export-response'; +} from '@opentelemetry/otlp-exporter-base'; const testServiceDefinition = { export: { From cf6f479b9fbd71943eee02f48e7f17fd29835d21 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 8 May 2024 17:49:06 +0200 Subject: [PATCH 02/11] feat(exporters): hide compression property --- .../test/node/CollectorTraceExporter.test.ts | 14 -------------- .../src/platform/node/OTLPExporterNodeBase.ts | 6 ++---- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index f275d430cd7..c5e39d2d8f6 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -200,20 +200,6 @@ describe('OTLPTraceExporter - node with json over http', () => { assert.strictEqual(collectorExporter.headers.bar, 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); - it('should use compression defined via env', () => { - envSource.OTEL_EXPORTER_OTLP_COMPRESSION = 'gzip'; - const collectorExporter = new OTLPTraceExporter(); - assert.strictEqual(collectorExporter.compression, 'gzip'); - envSource.OTEL_EXPORTER_OTLP_COMPRESSION = ''; - }); - it('should override global compression config with signal compression defined via env', () => { - envSource.OTEL_EXPORTER_OTLP_COMPRESSION = 'foo'; - envSource.OTEL_EXPORTER_OTLP_TRACES_COMPRESSION = 'gzip'; - const collectorExporter = new OTLPTraceExporter(); - assert.strictEqual(collectorExporter.compression, 'gzip'); - envSource.OTEL_EXPORTER_OTLP_COMPRESSION = ''; - envSource.OTEL_EXPORTER_OTLP_TRACES_COMPRESSION = ''; - }); }); describe('export', () => { diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts index 9af1773000a..ca768a0aafa 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts @@ -15,7 +15,7 @@ */ import { OTLPExporterBase } from '../../OTLPExporterBase'; -import { OTLPExporterNodeConfigBase, CompressionAlgorithm } from './types'; +import { OTLPExporterNodeConfigBase } from './types'; import { parseHeaders } from '../../util'; import { configureCompression } from './util'; import { diag } from '@opentelemetry/api'; @@ -34,7 +34,6 @@ export abstract class OTLPExporterNodeBase< > extends OTLPExporterBase { DEFAULT_HEADERS: Record = {}; headers: Record; - compression: CompressionAlgorithm; private _serializer: ISerializer; private _transport: IExporterTransport; @@ -54,7 +53,6 @@ export abstract class OTLPExporterNodeBase< baggageUtils.parseKeyPairsIntoRecord(getEnv().OTEL_EXPORTER_OTLP_HEADERS), { 'Content-Type': contentType } ); - this.compression = configureCompression(config.compression); this._serializer = serializer; // populate keepAlive for use with new settings @@ -75,7 +73,7 @@ export abstract class OTLPExporterNodeBase< this._transport = createHttpExporterTransport({ agentOptions: config.httpAgentOptions ?? { keepAlive: true }, - compression: this.compression, + compression: configureCompression(config.compression), headers: this.headers, url: this.url, timeoutMillis: this.timeoutMillis, From 4d785ce53a6f3c6d4bc98587e72e56f47ba13ee7 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 17 May 2024 16:50:20 +0200 Subject: [PATCH 03/11] feat(otlp-exporter-base)!: remove header property --- .../src/platform/node/OTLPLogExporter.ts | 17 +++++----- .../test/node/OTLPLogExporter.test.ts | 10 +++--- .../src/platform/node/OTLPLogExporter.ts | 9 +++-- .../test/node/OTLPLogExporter.test.ts | 18 +++++----- .../src/platform/node/OTLPTraceExporter.ts | 9 +++-- .../test/node/CollectorTraceExporter.test.ts | 20 +++++------ .../src/platform/node/OTLPTraceExporter.ts | 9 +++-- .../test/node/OTLPTraceExporter.test.ts | 20 +++++------ .../src/platform/node/OTLPMetricExporter.ts | 9 +++-- .../test/node/CollectorMetricExporter.test.ts | 34 +++++++++++++------ .../src/OTLPMetricExporter.ts | 9 +++-- .../test/OTLPMetricExporter.test.ts | 31 +++++++++++------ .../src/OTLPExporterBase.ts | 3 ++ .../src/platform/node/OTLPExporterNodeBase.ts | 20 +++++------ .../otlp-exporter-base/test/node/util.test.ts | 2 +- 15 files changed, 118 insertions(+), 102 deletions(-) diff --git a/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts index 093061424b6..1837993e911 100644 --- a/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts @@ -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 { diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index b521006128c..0d5caef7de6 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -87,7 +87,7 @@ describe('OTLPLogExporter', () => { it('should include user-agent header by default', () => { const exporter = new OTLPLogExporter(); assert.strictEqual( - exporter.headers['User-Agent'], + exporter['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -95,7 +95,7 @@ describe('OTLPLogExporter', () => { 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']['_parameters']['headers']['foo'], 'bar'); delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS; }); @@ -110,13 +110,13 @@ 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']['_parameters']['headers']['foo'], 'constructor'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts index 989d5e237a1..cd2e1085d48 100644 --- a/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-proto/src/platform/node/OTLPLogExporter.ts @@ -45,15 +45,14 @@ export class OTLPLogExporter implements LogRecordExporter { constructor(config: OTLPExporterConfigBase = {}) { - super(config, ProtobufLogsSerializer, 'application/x-protobuf'); - this.headers = { - ...this.headers, - ...USER_AGENT, + super(config, ProtobufLogsSerializer, { ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS ), ...parseHeaders(config?.headers), - }; + ...USER_AGENT, + 'Content-Type': 'application/x-protobuf', + }); } getDefaultUrl(config: OTLPExporterConfigBase): string { diff --git a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts index 8e85cbbdfca..d1519c0834f 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts @@ -156,34 +156,34 @@ describe('OTLPLogExporter - node with proto over http', () => { it('should include user-agent header by default', () => { const exporter = new OTLPLogExporter(); assert.strictEqual( - exporter.headers['User-Agent'], + exporter['_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 collectorExporter = new OTLPLogExporter(); - assert.strictEqual(collectorExporter.headers.foo, 'bar'); + const exporter = new OTLPLogExporter(); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=boo'; - const collectorExporter = new OTLPLogExporter(); - assert.strictEqual(collectorExporter.headers.foo, 'boo'); - assert.strictEqual(collectorExporter.headers.bar, 'foo'); + const exporter = new OTLPLogExporter(); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'boo'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); 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']['_parameters']['headers']['foo'], 'constructor'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts index e8e77831fb9..d62e0336b64 100644 --- a/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-http/src/platform/node/OTLPTraceExporter.ts @@ -43,15 +43,14 @@ export class OTLPTraceExporter implements SpanExporter { constructor(config: OTLPExporterNodeConfigBase = {}) { - super(config, JsonTraceSerializer, 'application/json'); - this.headers = { - ...this.headers, - ...USER_AGENT, + super(config, JsonTraceSerializer, { ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS ), ...parseHeaders(config?.headers), - }; + ...USER_AGENT, + 'Content-Type': 'application/json', + }); } getDefaultUrl(config: OTLPExporterNodeConfigBase): string { diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index c5e39d2d8f6..819258e3f8a 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -169,35 +169,35 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; - const collectorExporter = new OTLPTraceExporter(); - assert.strictEqual(collectorExporter.headers.foo, 'bar'); + const exporter = new OTLPTraceExporter(); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should include user agent in header', () => { - const collectorExporter = new OTLPTraceExporter(); + const exporter = new OTLPTraceExporter(); assert.strictEqual( - collectorExporter.headers['User-Agent'], + exporter['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; - const collectorExporter = new OTLPTraceExporter(); - assert.strictEqual(collectorExporter.headers.foo, 'boo'); - assert.strictEqual(collectorExporter.headers.bar, 'foo'); + const exporter = new OTLPTraceExporter(); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'boo'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override headers defined via env with headers defined in constructor', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; - const collectorExporter = new OTLPTraceExporter({ + const exporter = new OTLPTraceExporter({ headers: { foo: 'constructor', }, }); - assert.strictEqual(collectorExporter.headers.foo, 'constructor'); - assert.strictEqual(collectorExporter.headers.bar, 'foo'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'constructor'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts index bb3c916dbb8..cc60b0ab02f 100644 --- a/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-proto/src/platform/node/OTLPTraceExporter.ts @@ -43,15 +43,14 @@ export class OTLPTraceExporter implements SpanExporter { constructor(config: OTLPExporterNodeConfigBase = {}) { - super(config, ProtobufTraceSerializer, 'application/x-protobuf'); - this.headers = { - ...this.headers, - ...USER_AGENT, + super(config, ProtobufTraceSerializer, { ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS ), ...parseHeaders(config?.headers), - }; + ...USER_AGENT, + 'Content-Type': 'application/x-protobuf', + }); } getDefaultUrl(config: OTLPExporterNodeConfigBase) { diff --git a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts index 9ccaef82bbc..0dedd1c4a17 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts @@ -71,10 +71,10 @@ describe('OTLPTraceExporter - node with proto over http', () => { }); describe('default behavior for headers', () => { - const collectorExporter = new OTLPTraceExporter(); + const exporter = new OTLPTraceExporter(); it('should include user agent in header', () => { assert.strictEqual( - collectorExporter.headers['User-Agent'], + exporter['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -168,28 +168,28 @@ describe('OTLPTraceExporter - node with proto over http', () => { }); it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; - const collectorExporter = new OTLPTraceExporter(); - assert.strictEqual(collectorExporter.headers.foo, 'bar'); + const exporter = new OTLPTraceExporter(); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; - const collectorExporter = new OTLPTraceExporter(); - assert.strictEqual(collectorExporter.headers.foo, 'boo'); - assert.strictEqual(collectorExporter.headers.bar, 'foo'); + const exporter = new OTLPTraceExporter(); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'boo'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override headers defined via env with headers defined in constructor', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; - const collectorExporter = new OTLPTraceExporter({ + const exporter = new OTLPTraceExporter({ headers: { foo: 'constructor', }, }); - assert.strictEqual(collectorExporter.headers.foo, 'constructor'); - assert.strictEqual(collectorExporter.headers.bar, 'foo'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'constructor'); + assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts index d812c37b26d..c18b7a22dcf 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/platform/node/OTLPMetricExporter.ts @@ -42,15 +42,14 @@ class OTLPExporterNodeProxy extends OTLPExporterNodeBase< IExportMetricsServiceResponse > { constructor(config?: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions) { - super(config, JsonMetricsSerializer, 'application/json'); - this.headers = { - ...this.headers, - ...USER_AGENT, + super(config, JsonMetricsSerializer, { ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_METRICS_HEADERS ), ...parseHeaders(config?.headers), - }; + ...USER_AGENT, + 'Content-Type': 'application/json', + }); } getDefaultUrl(config: OTLPExporterNodeConfigBase): string { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index c74b2433645..ad87558f60a 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -56,7 +56,7 @@ import { } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; import { VERSION } from '../../src/version'; -import {nextTick} from "process"; +import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -341,38 +341,50 @@ describe('OTLPMetricExporter - node with json over http', () => { }); it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; - const collectorExporter = new OTLPMetricExporter(); - assert.strictEqual(collectorExporter._otlpExporter.headers.foo, 'bar'); + const exporter = new OTLPMetricExporter(); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + 'bar' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should include user agent in header', () => { - const collectorExporter = new OTLPMetricExporter(); + const exporter = new OTLPMetricExporter(); assert.strictEqual( - collectorExporter._otlpExporter.headers['User-Agent'], + exporter._otlpExporter['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; - const collectorExporter = new OTLPMetricExporter(); - assert.strictEqual(collectorExporter._otlpExporter.headers.foo, 'boo'); - assert.strictEqual(collectorExporter._otlpExporter.headers.bar, 'foo'); + const exporter = new OTLPMetricExporter(); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + 'boo' + ); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override headers defined via env with headers defined in constructor', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; - const collectorExporter = new OTLPMetricExporter({ + const exporter = new OTLPMetricExporter({ headers: { foo: 'constructor', }, }); assert.strictEqual( - collectorExporter._otlpExporter.headers.foo, + exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], 'constructor' ); - assert.strictEqual(collectorExporter._otlpExporter.headers.bar, 'foo'); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should use delta temporality defined via env', () => { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts index 150181db014..b42946a9a36 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/OTLPMetricExporter.ts @@ -42,15 +42,14 @@ class OTLPMetricExporterNodeProxy extends OTLPExporterNodeBase< IExportMetricsServiceResponse > { constructor(config?: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions) { - super(config, ProtobufMetricsSerializer, 'application/x-protobuf'); - this.headers = { - ...this.headers, - ...USER_AGENT, + super(config, ProtobufMetricsSerializer, { ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_METRICS_HEADERS ), ...parseHeaders(config?.headers), - }; + ...USER_AGENT, + 'Content-Type': 'application/x-protobuf', + }); } getDefaultUrl(config: OTLPExporterNodeConfigBase) { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index eede365e9fe..b18228a7574 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -44,7 +44,7 @@ import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { VERSION } from '../src/version'; import { Root } from 'protobufjs'; import * as path from 'path'; -import {nextTick} from "process"; +import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -78,10 +78,10 @@ describe('OTLPMetricExporter - node with proto over http', () => { }); describe('default behavior for headers', () => { - const collectorExporter = new OTLPMetricExporter(); + const exporter = new OTLPMetricExporter(); it('should include user agent in header', () => { assert.strictEqual( - collectorExporter._otlpExporter.headers['User-Agent'], + exporter._otlpExporter['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -179,31 +179,40 @@ describe('OTLPMetricExporter - node with proto over http', () => { }); it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; - const collectorExporter = new OTLPMetricExporter(); - assert.strictEqual(collectorExporter._otlpExporter.headers.foo, 'bar'); + const exporter = new OTLPMetricExporter(); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + 'bar' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; - const collectorExporter = new OTLPMetricExporter(); - assert.strictEqual(collectorExporter._otlpExporter.headers.foo, 'boo'); - assert.strictEqual(collectorExporter._otlpExporter.headers.bar, 'foo'); + const exporter = new OTLPMetricExporter(); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + 'boo' + ); + assert.strictEqual( + exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override headers defined via env with headers defined in constructor', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; - const collectorExporter = new OTLPMetricExporter({ + const exporter = new OTLPMetricExporter({ headers: { foo: 'constructor', }, }); assert.strictEqual( - collectorExporter._otlpExporter.headers.foo, + exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], 'constructor' ); - assert.strictEqual(collectorExporter._otlpExporter.headers.bar, 'foo'); + assert.strictEqual(exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts b/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts index 6581e2cf229..0e8c37f00d2 100644 --- a/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts +++ b/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts @@ -35,6 +35,9 @@ export abstract class OTLPExporterBase< ExportItem, > { public readonly url: string; + /** + * @deprecated scheduled for removal. This is only used in tests. + */ public readonly hostname: string | undefined; public readonly timeoutMillis: number; protected _concurrencyLimit: number; diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts index ca768a0aafa..c1187d31ef4 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts @@ -16,7 +16,6 @@ import { OTLPExporterBase } from '../../OTLPExporterBase'; import { OTLPExporterNodeConfigBase } from './types'; -import { parseHeaders } from '../../util'; import { configureCompression } from './util'; import { diag } from '@opentelemetry/api'; import { getEnv, baggageUtils } from '@opentelemetry/core'; @@ -32,27 +31,19 @@ export abstract class OTLPExporterNodeBase< ExportItem, ServiceResponse, > extends OTLPExporterBase { - DEFAULT_HEADERS: Record = {}; - headers: Record; private _serializer: ISerializer; private _transport: IExporterTransport; constructor( config: OTLPExporterNodeConfigBase = {}, serializer: ISerializer, - contentType: string + signalSpecificHeaders: Record ) { super(config); // eslint-disable-next-line @typescript-eslint/no-explicit-any if ((config as any).metadata) { diag.warn('Metadata cannot be set when using http'); } - this.headers = Object.assign( - this.DEFAULT_HEADERS, - parseHeaders(config.headers), - baggageUtils.parseKeyPairsIntoRecord(getEnv().OTEL_EXPORTER_OTLP_HEADERS), - { 'Content-Type': contentType } - ); this._serializer = serializer; // populate keepAlive for use with new settings @@ -70,11 +61,18 @@ export abstract class OTLPExporterNodeBase< }; } } + const nonSignalSpecificHeaders = baggageUtils.parseKeyPairsIntoRecord( + getEnv().OTEL_EXPORTER_OTLP_HEADERS + ); this._transport = createHttpExporterTransport({ agentOptions: config.httpAgentOptions ?? { keepAlive: true }, compression: configureCompression(config.compression), - headers: this.headers, + headers: Object.assign( + {}, + nonSignalSpecificHeaders, + signalSpecificHeaders + ), url: this.url, timeoutMillis: this.timeoutMillis, }); diff --git a/experimental/packages/otlp-exporter-base/test/node/util.test.ts b/experimental/packages/otlp-exporter-base/test/node/util.test.ts index 9ba2d55592f..19abbee9520 100644 --- a/experimental/packages/otlp-exporter-base/test/node/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/node/util.test.ts @@ -44,7 +44,7 @@ const noopSerializer: ISerializer = { describe('force flush', () => { it('forceFlush should flush spans and return', async () => { - const exporter = new Exporter({}, noopSerializer, ''); + const exporter = new Exporter({}, noopSerializer, {}); await exporter.forceFlush(); }); }); From e8635df5f9bb791516f57ca55c7b02ffdc48dd6c Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 31 May 2024 12:31:15 +0200 Subject: [PATCH 04/11] feat(otlp-exporter-base): add retrying transport --- .../test/node/OTLPLogExporter.test.ts | 8 +- .../test/node/OTLPLogExporter.test.ts | 12 +- .../test/node/CollectorTraceExporter.test.ts | 12 +- .../test/node/OTLPTraceExporter.test.ts | 12 +- .../test/node/CollectorMetricExporter.test.ts | 12 +- .../test/OTLPMetricExporter.test.ts | 12 +- .../src/platform/node/OTLPExporterNodeBase.ts | 23 ++- .../src/retryable-transport.ts | 66 +++++++ .../test/common/retrying-transport.test.ts | 180 ++++++++++++++++++ .../otlp-exporter-base/test/testHelper.ts | 33 ++++ 10 files changed, 326 insertions(+), 44 deletions(-) create mode 100644 experimental/packages/otlp-exporter-base/src/retryable-transport.ts create mode 100644 experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index 0d5caef7de6..5a43b89587a 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -87,7 +87,7 @@ describe('OTLPLogExporter', () => { it('should include user-agent header by default', () => { const exporter = new OTLPLogExporter(); assert.strictEqual( - exporter['_transport']['_parameters']['headers']['User-Agent'], + exporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -95,7 +95,7 @@ describe('OTLPLogExporter', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=bar'; const exporter = new OTLPLogExporter(); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS; }); @@ -115,8 +115,8 @@ describe('OTLPLogExporter', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts index d1519c0834f..bd38b2322cc 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts @@ -156,22 +156,22 @@ describe('OTLPLogExporter - node with proto over http', () => { it('should include user-agent header by default', () => { const exporter = new OTLPLogExporter(); assert.strictEqual( - exporter['_transport']['_parameters']['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['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=boo'; const exporter = new OTLPLogExporter(); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'boo'); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); @@ -182,8 +182,8 @@ describe('OTLPLogExporter - node with proto over http', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 819258e3f8a..e5792ca2693 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -170,13 +170,13 @@ describe('OTLPTraceExporter - node with json over http', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should include user agent in header', () => { const exporter = new OTLPTraceExporter(); assert.strictEqual( - exporter['_transport']['_parameters']['headers']['User-Agent'], + exporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -184,8 +184,8 @@ describe('OTLPTraceExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'boo'); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); @@ -196,8 +196,8 @@ describe('OTLPTraceExporter - node with json over http', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts index 0dedd1c4a17..b6c9d4178aa 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts @@ -74,7 +74,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { const exporter = new OTLPTraceExporter(); it('should include user agent in header', () => { assert.strictEqual( - exporter['_transport']['_parameters']['headers']['User-Agent'], + exporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -169,15 +169,15 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'boo'); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo'); + assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); @@ -188,8 +188,8 @@ describe('OTLPTraceExporter - node with proto over http', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index ad87558f60a..960f7571d19 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -343,7 +343,7 @@ describe('OTLPMetricExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar' ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -351,7 +351,7 @@ describe('OTLPMetricExporter - node with json over http', () => { it('should include user agent in header', () => { const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['User-Agent'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -360,11 +360,11 @@ describe('OTLPMetricExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo' ); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo' ); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; @@ -378,11 +378,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }, }); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], 'constructor' ); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo' ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index b18228a7574..a6ae7ae7102 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -81,7 +81,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { const exporter = new OTLPMetricExporter(); it('should include user agent in header', () => { assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['User-Agent'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -181,7 +181,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar' ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -191,11 +191,11 @@ describe('OTLPMetricExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo' ); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo' ); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; @@ -209,10 +209,10 @@ describe('OTLPMetricExporter - node with proto over http', () => { }, }); assert.strictEqual( - exporter._otlpExporter['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], 'constructor' ); - assert.strictEqual(exporter._otlpExporter['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual(exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts index c1187d31ef4..5ab20427f06 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/OTLPExporterNodeBase.ts @@ -23,6 +23,7 @@ import { ISerializer } from '@opentelemetry/otlp-transformer'; import { IExporterTransport } from '../../exporter-transport'; import { createHttpExporterTransport } from './http-exporter-transport'; import { OTLPExporterError } from '../../types'; +import { createRetryingTransport } from '../../retryable-transport'; /** * Collector Metric Exporter abstract base class @@ -65,16 +66,18 @@ export abstract class OTLPExporterNodeBase< getEnv().OTEL_EXPORTER_OTLP_HEADERS ); - this._transport = createHttpExporterTransport({ - agentOptions: config.httpAgentOptions ?? { keepAlive: true }, - compression: configureCompression(config.compression), - headers: Object.assign( - {}, - nonSignalSpecificHeaders, - signalSpecificHeaders - ), - url: this.url, - timeoutMillis: this.timeoutMillis, + this._transport = createRetryingTransport({ + transport: createHttpExporterTransport({ + agentOptions: config.httpAgentOptions ?? { keepAlive: true }, + compression: configureCompression(config.compression), + headers: Object.assign( + {}, + nonSignalSpecificHeaders, + signalSpecificHeaders + ), + url: this.url, + timeoutMillis: this.timeoutMillis, + }), }); } diff --git a/experimental/packages/otlp-exporter-base/src/retryable-transport.ts b/experimental/packages/otlp-exporter-base/src/retryable-transport.ts new file mode 100644 index 00000000000..7c902fd8b72 --- /dev/null +++ b/experimental/packages/otlp-exporter-base/src/retryable-transport.ts @@ -0,0 +1,66 @@ +/* + * 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 { IExporterTransport } from './exporter-transport'; +import { ExportResponse } from './export-response'; + +const MAX_ATTEMPTS = 5; +const INITIAL_BACKOFF = 1000; +const MAX_BACKOFF = 5000; +const BACKOFF_MULTIPLIER = 1.5; + +class RetryingTransport implements IExporterTransport { + constructor(private _transport: IExporterTransport) {} + + private retry(data: Uint8Array, inMillis: number): Promise { + return new Promise((resolve, reject) => { + setTimeout(() => { + this._transport.send(data).then(resolve, reject); + }, inMillis); + }); + } + + async send(data: Uint8Array): Promise { + let result = await this._transport.send(data); + let attempts = MAX_ATTEMPTS; + let nextBackoff = INITIAL_BACKOFF; + + // TODO: I'm not 100% sure this is correct, please review in-depth. + while (result.status === 'retryable' && attempts > 0) { + attempts--; + const upperBound = Math.min(nextBackoff, MAX_BACKOFF); + const backoff = Math.random() * upperBound; + nextBackoff = nextBackoff * BACKOFF_MULTIPLIER; + result = await this.retry(data, result.retryInMillis ?? backoff); + } + + return result; + } + + shutdown() { + return this._transport.shutdown(); + } +} + +/** + * Creates an Exporter Transport that retries on 'retryable' response. + */ +export function createRetryingTransport(options: { + // Underlying transport to wrap. + transport: IExporterTransport; +}): IExporterTransport { + return new RetryingTransport(options.transport); +} diff --git a/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts b/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts new file mode 100644 index 00000000000..374d7960c57 --- /dev/null +++ b/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts @@ -0,0 +1,180 @@ +/* + * 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 sinon from 'sinon'; +import * as assert from 'assert'; +import { IExporterTransport } from '../../src'; +import { createRetryingTransport } from '../../src/retryable-transport'; +import { ExportResponse } from '../../src'; +import { assertRejects } from '../testHelper'; + +describe('RetryingTransport', function () { + describe('send', function () { + it('does not retry when underlying transport succeeds', async function () { + // arrange + const expectedResponse: ExportResponse = { + status: 'success', + }; + const mockData = Uint8Array.from([1, 2, 3]); + + const transportStubs = { + // make transport succeed + send: sinon.stub().returns(Promise.resolve(expectedResponse)), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const transport = createRetryingTransport({ transport: mockTransport }); + + // act + const actualResponse = await transport.send(mockData); + + // assert + sinon.assert.calledOnceWithExactly(transportStubs.send, mockData); + assert.deepEqual(actualResponse, expectedResponse); + }); + + it('does not retry when underlying transport fails', async function () { + // arrange + const expectedResponse: ExportResponse = { + status: 'failure', + error: new Error(), + }; + const mockData = Uint8Array.from([1, 2, 3]); + + const transportStubs = { + // make transport fail + send: sinon.stub().returns(Promise.resolve(expectedResponse)), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const transport = createRetryingTransport({ transport: mockTransport }); + + // act + const actualResponse = await transport.send(mockData); + + // assert + sinon.assert.calledOnceWithExactly(transportStubs.send, mockData); + assert.deepEqual(actualResponse, expectedResponse); + }); + + it('does not retry when underlying transport rejects', async function () { + // arrange + const expectedError = new Error('error'); + const mockData = Uint8Array.from([1, 2, 3]); + + const transportStubs = { + // make transport reject + send: sinon.stub().rejects(expectedError), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const transport = createRetryingTransport({ transport: mockTransport }); + + // act + await assertRejects(() => transport.send(mockData)); + + // assert + sinon.assert.calledOnceWithExactly(transportStubs.send, mockData); + }); + + it('does retry when the underlying transport returns retryable', async function () { + // arrange + const retryResponse: ExportResponse = { + status: 'retryable', + }; + const successResponse: ExportResponse = { + status: 'success', + }; + const mockData = Uint8Array.from([1, 2, 3]); + + const transportStubs = { + send: sinon + .stub() + .onFirstCall() + .returns(Promise.resolve(retryResponse)) + .onSecondCall() + .returns(Promise.resolve(successResponse)), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const transport = createRetryingTransport({ transport: mockTransport }); + + // act + const actualResponse = await transport.send(mockData); + + // assert + sinon.assert.calledTwice(transportStubs.send); + sinon.assert.alwaysCalledWithExactly(transportStubs.send, mockData); + assert.deepEqual(actualResponse, successResponse); + }); + + it('does reject when the underlying transport rejects on retry', async function () { + // arrange + const expectedError = new Error('error'); + const retryResponse: ExportResponse = { + status: 'retryable', + }; + + const mockData = Uint8Array.from([1, 2, 3]); + + const transportStubs = { + send: sinon + .stub() + .onFirstCall() + .resolves(retryResponse) + .onSecondCall() + .rejects(expectedError), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const transport = createRetryingTransport({ transport: mockTransport }); + + // act + await assertRejects(() => transport.send(mockData)); + + // assert + sinon.assert.calledTwice(transportStubs.send); + sinon.assert.alwaysCalledWithExactly(transportStubs.send, mockData); + }); + + it('does retry 5 times, then resolves as retryable', async function () { + // arrange + // make random return low values so that it does not actually need to wait long for the backoff. + Math.random = sinon.stub().returns(0.001); + + const retryResponse: ExportResponse = { + status: 'retryable', + }; + + const mockData = Uint8Array.from([1, 2, 3]); + + const transportStubs = { + send: sinon.stub().resolves(retryResponse), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const transport = createRetryingTransport({ transport: mockTransport }); + + // act + const result = await transport.send(mockData); + + // assert + sinon.assert.callCount(transportStubs.send, 6); // 1 initial try and 5 retries + sinon.assert.alwaysCalledWithExactly(transportStubs.send, mockData); + assert.strictEqual(result, retryResponse); + }); + }); +}); diff --git a/experimental/packages/otlp-exporter-base/test/testHelper.ts b/experimental/packages/otlp-exporter-base/test/testHelper.ts index 41b0c95882e..c6932375116 100644 --- a/experimental/packages/otlp-exporter-base/test/testHelper.ts +++ b/experimental/packages/otlp-exporter-base/test/testHelper.ts @@ -77,3 +77,36 @@ export function ensureHeadersContain( ); }); } + +/** + * Changes to the below code should be applied to opentelemetry-core/test/test-utils.ts too. + */ + +interface ErrorLikeConstructor { + new (): Error; +} + +/** + * Node.js v8.x and browser compatible `assert.rejects`. + */ +export async function assertRejects( + actual: any, + expected?: RegExp | ErrorLikeConstructor +) { + let rejected; + try { + if (typeof actual === 'function') { + await actual(); + } else { + await actual; + } + } catch (err) { + rejected = true; + if (expected != null) { + assert.throws(() => { + throw err; + }, expected); + } + } + assert(rejected, 'Promise not rejected'); +} From b039d496da299d4d8f0bdb302b44b7816d94aad6 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 31 May 2024 13:02:24 +0200 Subject: [PATCH 05/11] fix: lint --- .../test/node/OTLPLogExporter.test.ts | 21 ++++++++++---- .../test/node/OTLPLogExporter.test.ts | 29 +++++++++++++++---- .../test/node/CollectorTraceExporter.test.ts | 29 +++++++++++++++---- .../test/node/OTLPTraceExporter.test.ts | 29 +++++++++++++++---- .../test/node/CollectorMetricExporter.test.ts | 24 +++++++++++---- .../test/OTLPMetricExporter.test.ts | 27 +++++++++++++---- 6 files changed, 124 insertions(+), 35 deletions(-) diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index 5a43b89587a..f83d35534da 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -32,7 +32,7 @@ import { PassThrough, Stream } from 'stream'; import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; import { ExportResultCode } from '@opentelemetry/core'; import { VERSION } from '../../src/version'; -import {nextTick} from "process"; +import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -87,7 +87,9 @@ describe('OTLPLogExporter', () => { it('should include user-agent header by default', () => { const exporter = new OTLPLogExporter(); assert.strictEqual( - exporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], + exporter['_transport']['_transport']['_parameters']['headers'][ + 'User-Agent' + ], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -95,7 +97,10 @@ describe('OTLPLogExporter', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=bar'; const exporter = new OTLPLogExporter(); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'bar' + ); delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS; }); @@ -115,8 +120,14 @@ describe('OTLPLogExporter', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts index bd38b2322cc..1339c2994bd 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts @@ -156,22 +156,33 @@ describe('OTLPLogExporter - node with proto over http', () => { it('should include user-agent header by default', () => { const exporter = new OTLPLogExporter(); assert.strictEqual( - exporter['_transport']['_transport']['_parameters']['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['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'bar' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=boo'; const exporter = new OTLPLogExporter(); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'boo' + ); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); @@ -182,8 +193,14 @@ describe('OTLPLogExporter - node with proto over http', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index e5792ca2693..222a61f2d2e 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -170,13 +170,18 @@ describe('OTLPTraceExporter - node with json over http', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'bar' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should include user agent in header', () => { const exporter = new OTLPTraceExporter(); assert.strictEqual( - exporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], + exporter['_transport']['_transport']['_parameters']['headers'][ + 'User-Agent' + ], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -184,8 +189,14 @@ describe('OTLPTraceExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'boo' + ); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); @@ -196,8 +207,14 @@ describe('OTLPTraceExporter - node with json over http', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts index b6c9d4178aa..05f5b9a9e37 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts @@ -74,7 +74,9 @@ describe('OTLPTraceExporter - node with proto over http', () => { const exporter = new OTLPTraceExporter(); it('should include user agent in header', () => { assert.strictEqual( - exporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], + exporter['_transport']['_transport']['_parameters']['headers'][ + 'User-Agent' + ], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -169,15 +171,24 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'bar'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'bar' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should override global headers config with signal headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; const exporter = new OTLPTraceExporter(); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'boo'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['foo'], + 'boo' + ); + assert.strictEqual( + exporter['_transport']['_transport']['_parameters']['headers']['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); @@ -188,8 +199,14 @@ describe('OTLPTraceExporter - node with proto over http', () => { foo: 'constructor', }, }); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['headers']['foo'], 'constructor'); - assert.strictEqual(exporter['_transport']['_transport']['_parameters']['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 = ''; }); }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 960f7571d19..49c65227bc4 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -343,7 +343,9 @@ describe('OTLPMetricExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['foo'], 'bar' ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -351,7 +353,9 @@ describe('OTLPMetricExporter - node with json over http', () => { it('should include user agent in header', () => { const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -360,11 +364,15 @@ describe('OTLPMetricExporter - node with json over http', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['foo'], 'boo' ); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['bar'], 'foo' ); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; @@ -378,11 +386,15 @@ describe('OTLPMetricExporter - node with json over http', () => { }, }); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['foo'], 'constructor' ); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['bar'], 'foo' ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index a6ae7ae7102..1aef6d45050 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -81,7 +81,9 @@ describe('OTLPMetricExporter - node with proto over http', () => { const exporter = new OTLPMetricExporter(); it('should include user agent in header', () => { assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['User-Agent'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['User-Agent'], `OTel-OTLP-Exporter-JavaScript/${VERSION}` ); }); @@ -181,7 +183,9 @@ describe('OTLPMetricExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['foo'], 'bar' ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; @@ -191,11 +195,15 @@ describe('OTLPMetricExporter - node with proto over http', () => { envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = 'foo=boo'; const exporter = new OTLPMetricExporter(); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['foo'], 'boo' ); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['bar'], 'foo' ); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; @@ -209,10 +217,17 @@ describe('OTLPMetricExporter - node with proto over http', () => { }, }); assert.strictEqual( - exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['foo'], + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['foo'], 'constructor' ); - assert.strictEqual(exporter._otlpExporter['_transport']['_transport']['_parameters']['headers']['bar'], 'foo'); + assert.strictEqual( + exporter._otlpExporter['_transport']['_transport']['_parameters'][ + 'headers' + ]['bar'], + 'foo' + ); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); From c284ec4b80f31122ac0a91aa634685b573286ca5 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 31 May 2024 13:11:45 +0200 Subject: [PATCH 06/11] chore: add changelog entry --- experimental/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 65d1819485c..f210b770af3 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -20,6 +20,10 @@ All notable changes to experimental packages in this project will be documented * (internal) the `@opentelemetry/otlp-exporter-proto-base` package has been removed, and will from now on be deprecated in `npm` * feat(instrumentation): remove default value for config in base instrumentation constructor [#4695](https://github.com/open-telemetry/opentelemetry-js/pull/4695): @blumamir * fix(instrumentation)!: remove unused supportedVersions from Instrumentation interface [#4694](https://github.com/open-telemetry/opentelemetry-js/pull/4694) @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 ### :rocket: (Enhancement) From b19734e79e2c537368588b8fba00639c30a7fe42 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 12 Jun 2024 09:51:11 +0200 Subject: [PATCH 07/11] fix: use queueMicrotask over nextTick --- .../test/node/OTLPLogExporter.test.ts | 11 +++++------ .../test/node/OTLPLogExporter.test.ts | 11 +++++------ .../test/node/CollectorTraceExporter.test.ts | 17 ++++++++--------- .../test/node/OTLPTraceExporter.test.ts | 11 +++++------ .../test/node/CollectorMetricExporter.test.ts | 11 +++++------ .../test/OTLPMetricExporter.test.ts | 11 +++++------ 6 files changed, 33 insertions(+), 39 deletions(-) diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index f83d35534da..8e6b076ed60 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -32,7 +32,6 @@ import { PassThrough, Stream } from 'stream'; import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; import { ExportResultCode } from '@opentelemetry/core'; import { VERSION } from '../../src/version'; -import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -167,7 +166,7 @@ describe('OTLPLogExporter', () => { assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -182,7 +181,7 @@ describe('OTLPLogExporter', () => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -199,7 +198,7 @@ describe('OTLPLogExporter', () => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -246,7 +245,7 @@ describe('OTLPLogExporter', () => { const spyLoggerError = sinon.stub(diag, 'error'); sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -263,7 +262,7 @@ describe('OTLPLogExporter', () => { it('should log the error message', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(400); cb(mockRes); mockRes.send(Buffer.from('failure')); diff --git a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts index 1339c2994bd..9cf3961fe5d 100644 --- a/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-proto/test/node/OTLPLogExporter.test.ts @@ -38,7 +38,6 @@ import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; import { VERSION } from '../../src/version'; import { Root } from 'protobufjs'; import * as path from 'path'; -import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -230,7 +229,7 @@ describe('OTLPLogExporter - node with proto over http', () => { assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -245,7 +244,7 @@ describe('OTLPLogExporter - node with proto over http', () => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -261,7 +260,7 @@ describe('OTLPLogExporter - node with proto over http', () => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -307,7 +306,7 @@ describe('OTLPLogExporter - node with proto over http', () => { const spyLoggerError = sinon.stub(diag, 'error'); sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -324,7 +323,7 @@ describe('OTLPLogExporter - node with proto over http', () => { it('should log the error message', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(400); cb(mockRes); mockRes.send(Buffer.from('failure')); diff --git a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts index 222a61f2d2e..171a8dcf14b 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/node/CollectorTraceExporter.test.ts @@ -33,7 +33,6 @@ import { ensureSpanIsCorrect, mockedReadableSpan, } from '../traceHelper'; -import { nextTick } from 'process'; import { MockedResponse } from './nodeHelpers'; import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; import { VERSION } from '../../src/version'; @@ -242,7 +241,7 @@ describe('OTLPTraceExporter - node with json over http', () => { setTimeout(() => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -262,7 +261,7 @@ describe('OTLPTraceExporter - node with json over http', () => { setTimeout(() => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -280,7 +279,7 @@ describe('OTLPTraceExporter - node with json over http', () => { setTimeout(() => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -299,7 +298,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -321,14 +320,14 @@ describe('OTLPTraceExporter - node with json over http', () => { const callback = args[1]; const mockRes = new MockedResponse(200); - nextTick(() => { + queueMicrotask(() => { callback(mockRes); mockRes.send(Buffer.from('success')); }); clock.restore(); - nextTick(() => { + queueMicrotask(() => { const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => {}); @@ -388,7 +387,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -413,7 +412,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(400); callback(mockRes); mockRes.send(Buffer.from('failure')); diff --git a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts index 05f5b9a9e37..6adf335c1a2 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/node/OTLPTraceExporter.test.ts @@ -38,7 +38,6 @@ import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; import { Root } from 'protobufjs'; import { VERSION } from '../../src/version'; import * as path from 'path'; -import { nextTick } from 'process'; const dir = path.resolve(__dirname, '../../../otlp-transformer/protos'); const root = new Root(); @@ -240,7 +239,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { done(e); } - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -256,7 +255,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -274,7 +273,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -322,7 +321,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { const spyLoggerError = sinon.stub(diag, 'error'); sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -344,7 +343,7 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should log the error message', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(400); cb(mockRes); mockRes.send(Buffer.from('failure')); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts index 49c65227bc4..7b19b84f029 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/node/CollectorMetricExporter.test.ts @@ -56,7 +56,6 @@ import { } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; import { VERSION } from '../../src/version'; -import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -498,7 +497,7 @@ describe('OTLPMetricExporter - node with json over http', () => { setTimeout(() => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -518,7 +517,7 @@ describe('OTLPMetricExporter - node with json over http', () => { setTimeout(() => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -537,7 +536,7 @@ describe('OTLPMetricExporter - node with json over http', () => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -634,7 +633,7 @@ describe('OTLPMetricExporter - node with json over http', () => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); callback(mockRes); mockRes.send(Buffer.from('success')); @@ -661,7 +660,7 @@ describe('OTLPMetricExporter - node with json over http', () => { setTimeout(() => { const args = stubRequest.args[0]; const callback = args[1]; - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(400); callback(mockRes); mockRes.send(Buffer.from('failure')); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts index 1aef6d45050..300fd8eb059 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/OTLPMetricExporter.test.ts @@ -44,7 +44,6 @@ import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { VERSION } from '../src/version'; import { Root } from 'protobufjs'; import * as path from 'path'; -import { nextTick } from 'process'; let fakeRequest: PassThrough; @@ -274,7 +273,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { assert.strictEqual(options.method, 'POST'); assert.strictEqual(options.path, '/'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -290,7 +289,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -308,7 +307,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -410,7 +409,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { const spyLoggerError = sinon.stub(diag, 'error'); sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(200); cb(mockRes); mockRes.send(Buffer.from('success')); @@ -428,7 +427,7 @@ describe('OTLPMetricExporter - node with proto over http', () => { it('should return the error code message', done => { sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - nextTick(() => { + queueMicrotask(() => { const mockRes = new MockedResponse(400); cb(mockRes); mockRes.send(Buffer.from('failure')); From 4df0efad11bf9f7a4747c13937c53c72be88f3b2 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 12 Jun 2024 09:52:19 +0200 Subject: [PATCH 08/11] chore: move changelog entry to unreleased --- experimental/CHANGELOG.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index c4e3cb758d9..bf5f346ebcc 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -7,6 +7,11 @@ All notable changes to experimental packages in this project will be documented ### :boom: Breaking Change +* 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 + ### :rocket: (Enhancement) * refactor(instrumentation-fetch): move fetch to use SEMATRR [#4632](https://github.com/open-telemetry/opentelemetry-js/pull/4632) @@ -45,10 +50,6 @@ All notable changes to experimental packages in this project will be documented * feat(sdk-node)!: simplify type of `instrumentations` option * Breaking changes: * replaces `InstrumentationOptions` with `(Instrumentation | Instrumentation[])[]` -* 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 ### :rocket: (Enhancement) From bb5ec8c82714f60a2578a887c34b9e7ee54778a7 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 21 Jun 2024 13:30:04 +0200 Subject: [PATCH 09/11] chore: note that user-agent cannot be overwritten by users anymore --- experimental/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 0f59f28dedd..6070e68bb3d 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -11,6 +11,8 @@ All notable changes to experimental packages in this project will be documented * (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) From 767eedc0101f0dd0a50fcbe9e442c1b75a2a0b9b Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 21 Jun 2024 13:32:38 +0200 Subject: [PATCH 10/11] fix: export missing ExportResponseRetryable --- experimental/packages/otlp-exporter-base/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/packages/otlp-exporter-base/src/index.ts b/experimental/packages/otlp-exporter-base/src/index.ts index c6a7b815f96..a2722f7ef5d 100644 --- a/experimental/packages/otlp-exporter-base/src/index.ts +++ b/experimental/packages/otlp-exporter-base/src/index.ts @@ -32,6 +32,7 @@ export { ExportResponse, ExportResponseFailure, ExportResponseSuccess, + ExportResponseRetryable, } from './export-response'; export { IExporterTransport } from './exporter-transport'; From f1577c02fd3ac6e093c1ee2165dcf25cfe15f61f Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 21 Jun 2024 15:08:47 +0200 Subject: [PATCH 11/11] fix: retry jitter --- .../otlp-exporter-base/src/retryable-transport.ts | 12 +++++++++--- .../test/common/retrying-transport.test.ts | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/retryable-transport.ts b/experimental/packages/otlp-exporter-base/src/retryable-transport.ts index 7c902fd8b72..e48192c1fd2 100644 --- a/experimental/packages/otlp-exporter-base/src/retryable-transport.ts +++ b/experimental/packages/otlp-exporter-base/src/retryable-transport.ts @@ -21,6 +21,14 @@ const MAX_ATTEMPTS = 5; const INITIAL_BACKOFF = 1000; const MAX_BACKOFF = 5000; const BACKOFF_MULTIPLIER = 1.5; +const JITTER = 0.2; + +/** + * Get a pseudo-random jitter that falls in the range of [-JITTER, +JITTER] + */ +function getJitter() { + return Math.random() * (2 * JITTER) - JITTER; +} class RetryingTransport implements IExporterTransport { constructor(private _transport: IExporterTransport) {} @@ -38,11 +46,9 @@ class RetryingTransport implements IExporterTransport { let attempts = MAX_ATTEMPTS; let nextBackoff = INITIAL_BACKOFF; - // TODO: I'm not 100% sure this is correct, please review in-depth. while (result.status === 'retryable' && attempts > 0) { attempts--; - const upperBound = Math.min(nextBackoff, MAX_BACKOFF); - const backoff = Math.random() * upperBound; + const backoff = Math.min(nextBackoff, MAX_BACKOFF) + getJitter(); nextBackoff = nextBackoff * BACKOFF_MULTIPLIER; result = await this.retry(data, result.retryInMillis ?? backoff); } diff --git a/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts b/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts index 374d7960c57..cd7be6ebbbe 100644 --- a/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts +++ b/experimental/packages/otlp-exporter-base/test/common/retrying-transport.test.ts @@ -152,8 +152,8 @@ describe('RetryingTransport', function () { it('does retry 5 times, then resolves as retryable', async function () { // arrange - // make random return low values so that it does not actually need to wait long for the backoff. - Math.random = sinon.stub().returns(0.001); + // make random return a negative value so that what's passed to setTimeout() is negative and therefore gets executed immediately. + Math.random = sinon.stub().returns(-Infinity); const retryResponse: ExportResponse = { status: 'retryable',