From 7376ce57f53b66008e65e5d3241c76780cbd488f Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 4 Aug 2022 16:26:03 -0500 Subject: [PATCH 01/19] feat(otlp-exporter-base): add retries to sendWithHttp Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../src/platform/node/util.ts | 115 +++++++++++------- .../otlp-exporter-base/test/node/util.test.ts | 15 +++ 2 files changed, 84 insertions(+), 46 deletions(-) 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 2d2a47471dc..647f7c26341 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -25,6 +25,10 @@ import { CompressionAlgorithm } from './types'; import { getEnv } from '@opentelemetry/core'; import { OTLPExporterError } from '../../types'; +const DEFAULT_MAX_ATTEMPTS = 4; +const DEFAULT_INITIAL_BACKOFF = 1000; +const DEFAULT_BACKOFF_MULTIPLIER = 1.5; + /** * Sends data using http * @param collector @@ -44,8 +48,11 @@ export function sendWithHttp( const parsedUrl = new url.URL(collector.url); let reqIsDestroyed: boolean; const nodeVersion = Number(process.versions.node.split('.')[0]); + let retryTimer: ReturnType; + let req: http.ClientRequest; const exporterTimer = setTimeout(() => { + clearTimeout(retryTimer); reqIsDestroyed = true; // req.abort() was deprecated since v14 if (nodeVersion >= 14) { @@ -69,63 +76,79 @@ export function sendWithHttp( const request = parsedUrl.protocol === 'http:' ? http.request : https.request; - const req = request(options, (res: http.IncomingMessage) => { - let responseData = ''; - res.on('data', chunk => (responseData += chunk)); + const sendWithRetry = (retries = DEFAULT_MAX_ATTEMPTS, backoffMillis = DEFAULT_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) { + if (res.statusCode && res.statusCode < 299) { + diag.debug(`statusCode: ${res.statusCode}`, responseData); + onSuccess(); + } else if (res.statusCode && isRetryable(res.statusCode) && retries > 0) { + retryTimer = setTimeout(() => { + return sendWithRetry(retries - 1, backoffMillis * DEFAULT_BACKOFF_MULTIPLIER); + }, backoffMillis); + } 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); + } + }); + }); - res.on('aborted', () => { + req.on('error', (error: Error | any) => { if (reqIsDestroyed) { const err = new OTLPExporterError( - 'Request Timeout' + 'Request Timeout', error.code ); onError(err); + } else { + onError(error); } + clearTimeout(exporterTimer); + clearTimeout(retryTimer); }); - - res.on('end', () => { - if (!reqIsDestroyed) { - if (res.statusCode && res.statusCode < 299) { - diag.debug(`statusCode: ${res.statusCode}`, responseData); - onSuccess(); - } else { - const error = new OTLPExporterError( - res.statusMessage, - res.statusCode, - responseData - ); - onError(error); - } - clearTimeout(exporterTimer); + + switch (collector.compression) { + case CompressionAlgorithm.GZIP: { + req.setHeader('Content-Encoding', 'gzip'); + const dataStream = readableFromBuffer(data); + dataStream.on('error', onError) + .pipe(zlib.createGzip()).on('error', onError) + .pipe(req); + + break; } - }); - }); - - req.on('error', (error: Error | any) => { - if (reqIsDestroyed) { - const err = new OTLPExporterError( - 'Request Timeout', error.code - ); - onError(err); - } else { - clearTimeout(exporterTimer); - onError(error); + default: + req.end(data); + break; } - }); + } + sendWithRetry(); +} - switch (collector.compression) { - case CompressionAlgorithm.GZIP: { - req.setHeader('Content-Encoding', 'gzip'); - const dataStream = readableFromBuffer(data); - dataStream.on('error', onError) - .pipe(zlib.createGzip()).on('error', onError) - .pipe(req); +function isRetryable(statusCode: number) { + const retryCodes = [429, 502, 503, 504]; - break; - } - default: - req.end(data); - break; - } + return retryCodes.includes(statusCode); } function readableFromBuffer(buff: string | Buffer): Readable { 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 56dc66f2ee6..5eb781f8568 100644 --- a/experimental/packages/otlp-exporter-base/test/node/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/node/util.test.ts @@ -214,12 +214,17 @@ describe('sendWithHttp', () => { assert.strictEqual(requestData, data); }); + // use fake timers to replace setTimeout in sendWithHttp function + let 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', () => { @@ -238,12 +243,17 @@ describe('sendWithHttp', () => { assert(Buffer.concat(buffers).equals(compressedData)); }); + // use fake timers to replace setTimeout in sendWithHttp function + let 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', () => { @@ -274,12 +284,17 @@ describe('sendWithHttp', () => { assert(Buffer.concat(buffers).equals(compressedData)); }); + // use fake timers to replace setTimeout in sendWithHttp function + let 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(); } }); }); From 43320ee2e9ab1b85887ebe2ca2819f1a742c9c71 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 19 Aug 2022 14:59:52 -0500 Subject: [PATCH 02/19] feat(otlp-exporter-base): add tests and update abort logic Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../test/node/CollectorTraceExporter.test.ts | 73 +++++++++++++++++++ .../src/platform/node/util.ts | 29 ++++++-- 2 files changed, 95 insertions(+), 7 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 4e0a8e4c12c..37d4d987967 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 @@ -588,3 +588,76 @@ describe('export - real http request destroyed after response received', () => { }, 0); }); }); + +describe('export with retry - real http request destroyed after response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + + const server = http.createServer((_, res) => { + res.statusCode = 502; + res.end(); + }); + + before(done => { + server.listen(8081, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 3000, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); + }).timeout(5000); +}); + +describe('export with retry - real http request destroyed after response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + let attempts = -1 + + const server = http.createServer((_, res) => { + attempts++ + if (attempts >= 2) { + res.statusCode = 200; + } else { + res.statusCode = 502; + } + res.end(); + }); + + before(done => { + server.listen(8081, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 3000, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.SUCCESS); + done(); + }); + }).timeout(5000); +}); \ No newline at end of file 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 647f7c26341..b71f142a088 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -56,7 +56,8 @@ export function sendWithHttp( reqIsDestroyed = true; // req.abort() was deprecated since v14 if (nodeVersion >= 14) { - req.destroy(); + // req.destroy(); + req.abort(); } else { req.abort(); } @@ -91,13 +92,16 @@ export function sendWithHttp( }); res.on('end', () => { - if (!reqIsDestroyed) { + if (reqIsDestroyed === undefined) { 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 && isRetryable(res.statusCode) && retries > 0) { retryTimer = setTimeout(() => { - return sendWithRetry(retries - 1, backoffMillis * DEFAULT_BACKOFF_MULTIPLIER); + sendWithRetry(retries - 1, backoffMillis * DEFAULT_BACKOFF_MULTIPLIER); }, backoffMillis); } else { const error = new OTLPExporterError( @@ -106,10 +110,10 @@ export function sendWithHttp( responseData ); onError(error); - } - // clear all timers since request was completed and promise was resolved + // clear all timers since request was completed and promise was resolved clearTimeout(exporterTimer); clearTimeout(retryTimer); + } } }); }); @@ -126,7 +130,18 @@ export function sendWithHttp( 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'); @@ -145,7 +160,7 @@ export function sendWithHttp( sendWithRetry(); } -function isRetryable(statusCode: number) { +function isRetryable(statusCode: number): boolean { const retryCodes = [429, 502, 503, 504]; return retryCodes.includes(statusCode); From 21771d55d82f32fd13928409e677fa0abfb021e5 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 26 Aug 2022 14:36:24 -0500 Subject: [PATCH 03/19] feat(otlp-exporter-base): fix lint Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../test/node/CollectorTraceExporter.test.ts | 28 +++++++++---------- .../src/platform/node/util.ts | 20 ++++++------- .../otlp-exporter-base/test/node/util.test.ts | 6 ++-- 3 files changed, 27 insertions(+), 27 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 37d4d987967..e9a4e685bc1 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 @@ -614,13 +614,13 @@ describe('export with retry - real http request destroyed after response receive spans = []; spans.push(Object.assign({}, mockedReadableSpan)); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + done(); + }); }).timeout(5000); }); @@ -628,10 +628,10 @@ describe('export with retry - real http request destroyed after response receive let collectorExporter: OTLPTraceExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase; let spans: ReadableSpan[]; - let attempts = -1 + let attempts = -1; const server = http.createServer((_, res) => { - attempts++ + attempts++; if (attempts >= 2) { res.statusCode = 200; } else { @@ -655,9 +655,9 @@ describe('export with retry - real http request destroyed after response receive spans = []; spans.push(Object.assign({}, mockedReadableSpan)); - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.SUCCESS); - done(); - }); + collectorExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.SUCCESS); + done(); + }); }).timeout(5000); -}); \ No newline at end of file +}); 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 b71f142a088..3801af7c27d 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -81,7 +81,7 @@ export function sendWithHttp( req = request(options, (res: http.IncomingMessage) => { let responseData = ''; res.on('data', chunk => (responseData += chunk)); - + res.on('aborted', () => { if (reqIsDestroyed) { const err = new OTLPExporterError( @@ -90,15 +90,15 @@ export function sendWithHttp( onError(err); } }); - + res.on('end', () => { if (reqIsDestroyed === undefined) { 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); + // clear all timers since request was completed and promise was resolved + clearTimeout(exporterTimer); + clearTimeout(retryTimer); } else if (res.statusCode && isRetryable(res.statusCode) && retries > 0) { retryTimer = setTimeout(() => { sendWithRetry(retries - 1, backoffMillis * DEFAULT_BACKOFF_MULTIPLIER); @@ -110,9 +110,9 @@ export function sendWithHttp( responseData ); onError(error); - // clear all timers since request was completed and promise was resolved - clearTimeout(exporterTimer); - clearTimeout(retryTimer); + // clear all timers since request was completed and promise was resolved + clearTimeout(exporterTimer); + clearTimeout(retryTimer); } } }); @@ -149,14 +149,14 @@ export function sendWithHttp( dataStream.on('error', onError) .pipe(zlib.createGzip()).on('error', onError) .pipe(req); - + break; } default: req.end(data); break; } - } + }; sendWithRetry(); } 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 5eb781f8568..9c5f0c08e13 100644 --- a/experimental/packages/otlp-exporter-base/test/node/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/node/util.test.ts @@ -215,7 +215,7 @@ describe('sendWithHttp', () => { }); // use fake timers to replace setTimeout in sendWithHttp function - let clock = sinon.useFakeTimers(); + const clock = sinon.useFakeTimers(); sendWithHttp(exporter, data, 'application/json', () => { // Show that we aren't setting the gzip encoding header @@ -244,7 +244,7 @@ describe('sendWithHttp', () => { }); // use fake timers to replace setTimeout in sendWithHttp function - let clock = sinon.useFakeTimers(); + const clock = sinon.useFakeTimers(); sendWithHttp(exporter, data, 'application/json', () => { // Show that we are setting the gzip encoding header @@ -285,7 +285,7 @@ describe('sendWithHttp', () => { }); // use fake timers to replace setTimeout in sendWithHttp function - let clock = sinon.useFakeTimers(); + const clock = sinon.useFakeTimers(); sendWithHttp(exporter, data, 'application/json', () => { // Show that we are setting the gzip encoding header From c9e54b0b3b78d26c8ec914d7ebc4bef4bee86a67 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 22 Sep 2022 10:50:02 -0500 Subject: [PATCH 04/19] feat(otlp-exporter-base): add retry test Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../test/node/CollectorTraceExporter.test.ts | 88 +++---------------- .../src/platform/node/util.ts | 13 +-- 2 files changed, 18 insertions(+), 83 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 e9a4e685bc1..e80c649db75 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 @@ -554,102 +554,32 @@ describe('export - real http request destroyed before response received', () => }); }); -describe('export - real http request destroyed after response received', () => { - let collectorExporter: OTLPTraceExporter; - let collectorExporterConfig: OTLPExporterNodeConfigBase; - let spans: ReadableSpan[]; - - const server = http.createServer((_, res) => { - res.write('writing something'); - }); - before(done => { - server.listen(8081, done); - }); - after(done => { - server.close(done); - }); - it('should log the timeout request error message', done => { - collectorExporterConfig = { - url: 'http://localhost:8081', - timeoutMillis: 300, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); - - setTimeout(() => { - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }, 0); - }); -}); - describe('export with retry - real http request destroyed after response received', () => { let collectorExporter: OTLPTraceExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase; let spans: ReadableSpan[]; + let retries = 0; const server = http.createServer((_, res) => { - res.statusCode = 502; - res.end(); - }); - - before(done => { - server.listen(8081, done); - }); - after(done => { - server.close(done); - }); - it('should log the timeout request error message', done => { - collectorExporterConfig = { - url: 'http://localhost:8081', - timeoutMillis: 3000, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); - - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.FAILED); - const error = result.error as OTLPExporterError; - assert.ok(error !== undefined); - assert.strictEqual(error.message, 'Request Timeout'); - done(); - }); - }).timeout(5000); -}); - -describe('export with retry - real http request destroyed after response received', () => { - let collectorExporter: OTLPTraceExporter; - let collectorExporterConfig: OTLPExporterNodeConfigBase; - let spans: ReadableSpan[]; - let attempts = -1; - - const server = http.createServer((_, res) => { - attempts++; - if (attempts >= 2) { + retries++; + if (retries >= 2) { res.statusCode = 200; } else { res.statusCode = 502; } res.end(); }); - before(done => { server.listen(8081, done); }); - after(done => { + after(function (done) { + this.timeout(3000) server.close(done); }); - it('should log the timeout request error message', done => { + it.only('should log the timeout request error message', done => { collectorExporterConfig = { url: 'http://localhost:8081', - timeoutMillis: 3000, + timeoutMillis: 2500, }; collectorExporter = new OTLPTraceExporter(collectorExporterConfig); spans = []; @@ -657,7 +587,9 @@ describe('export with retry - real http request destroyed after response receive collectorExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.SUCCESS); + assert.strictEqual(retries, 2) done(); }); - }).timeout(5000); + }).timeout(3000); }); + 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 3801af7c27d..a36e272df2a 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -54,12 +54,15 @@ export function sendWithHttp( const exporterTimer = setTimeout(() => { clearTimeout(retryTimer); reqIsDestroyed = true; - // req.abort() was deprecated since v14 - if (nodeVersion >= 14) { - // req.destroy(); - req.abort(); + + if (req.destroyed) { + const err = new OTLPExporterError( + 'Request Timeout' + ); + onError(err); } else { - req.abort(); + // req.abort() was deprecated since v14 + nodeVersion >= 14 ? req.destroy() : req.abort(); } }, exporterTimeout); From 75a50add6986e672b0daf8efae4f505457c4232c Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 6 Oct 2022 15:42:11 -0500 Subject: [PATCH 05/19] feat(otlp-exporter-base): add retry to browser exporter and add tests Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../browser/CollectorTraceExporter.test.ts | 45 +++++++ .../test/node/CollectorTraceExporter.test.ts | 7 +- .../src/platform/browser/util.ts | 111 +++++++++++++----- .../src/platform/node/util.ts | 17 +-- .../packages/otlp-exporter-base/src/util.ts | 3 + 5 files changed, 140 insertions(+), 43 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 8981d865ebb..49332c0e09e 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -599,3 +599,48 @@ describe('when configuring via environment', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); + + +describe('export with retry - real http request destroyed', () => { + let server: any; + let collectorTraceExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterConfigBase; + let spans: ReadableSpan[]; + + beforeEach(() => { + server = sinon.fakeServer.create({ + autoRespond: true + }); + collectorExporterConfig = { + timeoutMillis: 1500 + }; + }); + + afterEach(() => { + server.restore(); + }); + + describe('when "sendBeacon" is NOT available', () => { + beforeEach(() => { + (window.navigator as any).sendBeacon = false; + collectorTraceExporter = new OTLPTraceExporter(collectorExporterConfig); + }); + + it('should log the timeout request error message', done => { + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + let retry = 0; + server.respondWith('http://localhost:4318/v1/traces', function (xhr: any, id: any) { + retry++; + xhr.respond(502); + }); + + collectorTraceExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + assert.strictEqual(retry, 2); + done(); + }); + }).timeout(3000); + }); +}); 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 e80c649db75..3b2cd4c60f7 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 @@ -573,13 +573,12 @@ describe('export with retry - real http request destroyed after response receive server.listen(8081, done); }); after(function (done) { - this.timeout(3000) server.close(done); }); - it.only('should log the timeout request error message', done => { + it('should log the timeout request error message', done => { collectorExporterConfig = { url: 'http://localhost:8081', - timeoutMillis: 2500, + timeoutMillis: 1500, }; collectorExporter = new OTLPTraceExporter(collectorExporterConfig); spans = []; @@ -587,7 +586,7 @@ describe('export with retry - real http request destroyed after response receive collectorExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.SUCCESS); - assert.strictEqual(retries, 2) + assert.strictEqual(retries, 2); done(); }); }).timeout(3000); diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 48440370567..639d4fc82ab 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -15,6 +15,11 @@ */ import { diag } from '@opentelemetry/api'; import { OTLPExporterError } from '../../types'; +import { + DEFAULT_EXPORT_MAX_ATTEMPTS, + DEFAULT_EXPORT_INITIAL_BACKOFF, + DEFAULT_EXPORT_BACKOFF_MULTIPLIER +} from '../../util'; /** * Send metrics/spans using browser navigator.sendBeacon @@ -60,48 +65,92 @@ export function sendWithXhr( onError: (error: OTLPExporterError) => void ): void { let reqIsDestroyed: boolean; + let retryTimer: ReturnType; + let xhr: XMLHttpRequest; const exporterTimer = setTimeout(() => { + clearTimeout(retryTimer); reqIsDestroyed = true; - xhr.abort(); + + if (xhr.readyState === XMLHttpRequest.DONE) { + const err = new OTLPExporterError( + 'Request Timeout' + ); + onError(err); + } else { + xhr.abort(); + } }, exporterTimeout); - const xhr = new XMLHttpRequest(); - xhr.open('POST', url); + const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, backoffMillis = DEFAULT_EXPORT_INITIAL_BACKOFF) => { + xhr = new XMLHttpRequest(); + xhr.open('POST', url); - const defaultHeaders = { - 'Accept': 'application/json', - 'Content-Type': 'application/json', - }; + const defaultHeaders = { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + }; - Object.entries({ - ...defaultHeaders, - ...headers, - }).forEach(([k, v]) => { - xhr.setRequestHeader(k, v); - }); + Object.entries({ + ...defaultHeaders, + ...headers, + }).forEach(([k, v]) => { + xhr.setRequestHeader(k, v); + }); - xhr.send(body); + xhr.send(body); - xhr.onreadystatechange = () => { - if (xhr.readyState === XMLHttpRequest.DONE) { - if (xhr.status >= 200 && xhr.status <= 299) { - clearTimeout(exporterTimer); - diag.debug('xhr success', body); - onSuccess(); - } else if (reqIsDestroyed) { - const error = new OTLPExporterError( - 'Request Timeout', xhr.status + xhr.onreadystatechange = () => { + if (xhr.readyState === XMLHttpRequest.DONE && reqIsDestroyed === undefined) { + if (xhr.status >= 200 && xhr.status <= 299) { + clearTimeout(exporterTimer); + clearTimeout(retryTimer); + diag.debug('xhr success', body); + onSuccess(); + } else if (xhr.status && isRetryable(xhr.status) && retries > 0) { + retryTimer = setTimeout(() => { + sendWithRetry(retries - 1, backoffMillis * DEFAULT_EXPORT_BACKOFF_MULTIPLIER); + }, backoffMillis); + } else { + const error = new OTLPExporterError( + `Failed to export with XHR (status: ${xhr.status})`, + xhr.status + ); + clearTimeout(exporterTimer); + clearTimeout(retryTimer); + onError(error); + } + } + }; + + xhr.onabort = () => { + if (reqIsDestroyed) { + const err = new OTLPExporterError( + 'Request Timeout' ); - onError(error); - } else { - const error = new OTLPExporterError( - `Failed to export with XHR (status: ${xhr.status})`, - xhr.status + onError(err); + } + clearTimeout(exporterTimer); + clearTimeout(retryTimer); + }; + + xhr.onerror = () => { + if (reqIsDestroyed) { + const err = new OTLPExporterError( + 'Request Timeout' ); - clearTimeout(exporterTimer); - onError(error); + onError(err); } - } + clearTimeout(exporterTimer); + clearTimeout(retryTimer); + }; }; + + sendWithRetry(); +} + +function isRetryable(statusCode: number): boolean { + const retryCodes = [429, 502, 503, 504]; + + return retryCodes.includes(statusCode); } 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 a36e272df2a..8c0beca2b2e 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -24,10 +24,11 @@ import { diag } from '@opentelemetry/api'; import { CompressionAlgorithm } from './types'; import { getEnv } from '@opentelemetry/core'; import { OTLPExporterError } from '../../types'; - -const DEFAULT_MAX_ATTEMPTS = 4; -const DEFAULT_INITIAL_BACKOFF = 1000; -const DEFAULT_BACKOFF_MULTIPLIER = 1.5; +import { + DEFAULT_EXPORT_MAX_ATTEMPTS, + DEFAULT_EXPORT_INITIAL_BACKOFF, + DEFAULT_EXPORT_BACKOFF_MULTIPLIER +} from '../../util'; /** * Sends data using http @@ -58,8 +59,8 @@ export function sendWithHttp( if (req.destroyed) { const err = new OTLPExporterError( 'Request Timeout' - ); - onError(err); + ); + onError(err); } else { // req.abort() was deprecated since v14 nodeVersion >= 14 ? req.destroy() : req.abort(); @@ -80,7 +81,7 @@ export function sendWithHttp( const request = parsedUrl.protocol === 'http:' ? http.request : https.request; - const sendWithRetry = (retries = DEFAULT_MAX_ATTEMPTS, backoffMillis = DEFAULT_INITIAL_BACKOFF) => { + const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, backoffMillis = DEFAULT_EXPORT_INITIAL_BACKOFF) => { req = request(options, (res: http.IncomingMessage) => { let responseData = ''; res.on('data', chunk => (responseData += chunk)); @@ -104,7 +105,7 @@ export function sendWithHttp( clearTimeout(retryTimer); } else if (res.statusCode && isRetryable(res.statusCode) && retries > 0) { retryTimer = setTimeout(() => { - sendWithRetry(retries - 1, backoffMillis * DEFAULT_BACKOFF_MULTIPLIER); + sendWithRetry(retries - 1, backoffMillis * DEFAULT_EXPORT_BACKOFF_MULTIPLIER); }, backoffMillis); } else { const error = new OTLPExporterError( diff --git a/experimental/packages/otlp-exporter-base/src/util.ts b/experimental/packages/otlp-exporter-base/src/util.ts index bfe9209758f..4ae24daa1b9 100644 --- a/experimental/packages/otlp-exporter-base/src/util.ts +++ b/experimental/packages/otlp-exporter-base/src/util.ts @@ -18,6 +18,9 @@ import { diag } from '@opentelemetry/api'; import { getEnv } from '@opentelemetry/core'; const DEFAULT_TRACE_TIMEOUT = 10000; +export const DEFAULT_EXPORT_MAX_ATTEMPTS = 4; +export const DEFAULT_EXPORT_INITIAL_BACKOFF = 1000; +export const DEFAULT_EXPORT_BACKOFF_MULTIPLIER = 1.5; /** * Parses headers from config leaving only those that have defined values From 5bb910b34088b5fbc95123ae01cebee474ca223c Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 6 Oct 2022 15:56:37 -0500 Subject: [PATCH 06/19] feat(otlp-exporter-base): refactor Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../test/browser/CollectorTraceExporter.test.ts | 4 +++- .../src/platform/browser/util.ts | 17 ++++++----------- .../src/platform/node/util.ts | 11 +++-------- .../packages/otlp-exporter-base/src/util.ts | 6 ++++++ 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 49332c0e09e..7a9ef8404bf 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -600,7 +600,6 @@ describe('when configuring via environment', () => { }); }); - describe('export with retry - real http request destroyed', () => { let server: any; let collectorTraceExporter: OTLPTraceExporter; @@ -638,6 +637,9 @@ describe('export with retry - real http request destroyed', () => { collectorTraceExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); assert.strictEqual(retry, 2); done(); }); diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 639d4fc82ab..c97406cbed8 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -18,7 +18,8 @@ import { OTLPExporterError } from '../../types'; import { DEFAULT_EXPORT_MAX_ATTEMPTS, DEFAULT_EXPORT_INITIAL_BACKOFF, - DEFAULT_EXPORT_BACKOFF_MULTIPLIER + DEFAULT_EXPORT_BACKOFF_MULTIPLIER, + isExportRetryable } from '../../util'; /** @@ -103,11 +104,11 @@ export function sendWithXhr( xhr.onreadystatechange = () => { if (xhr.readyState === XMLHttpRequest.DONE && reqIsDestroyed === undefined) { if (xhr.status >= 200 && xhr.status <= 299) { - clearTimeout(exporterTimer); - clearTimeout(retryTimer); diag.debug('xhr success', body); onSuccess(); - } else if (xhr.status && isRetryable(xhr.status) && retries > 0) { + clearTimeout(exporterTimer); + clearTimeout(retryTimer); + } else if (xhr.status && isExportRetryable(xhr.status) && retries > 0) { retryTimer = setTimeout(() => { sendWithRetry(retries - 1, backoffMillis * DEFAULT_EXPORT_BACKOFF_MULTIPLIER); }, backoffMillis); @@ -116,9 +117,9 @@ export function sendWithXhr( `Failed to export with XHR (status: ${xhr.status})`, xhr.status ); + onError(error); clearTimeout(exporterTimer); clearTimeout(retryTimer); - onError(error); } } }; @@ -148,9 +149,3 @@ export function sendWithXhr( sendWithRetry(); } - -function isRetryable(statusCode: number): boolean { - const retryCodes = [429, 502, 503, 504]; - - return retryCodes.includes(statusCode); -} 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 8c0beca2b2e..5fb8ada5d01 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -27,7 +27,8 @@ import { OTLPExporterError } from '../../types'; import { DEFAULT_EXPORT_MAX_ATTEMPTS, DEFAULT_EXPORT_INITIAL_BACKOFF, - DEFAULT_EXPORT_BACKOFF_MULTIPLIER + DEFAULT_EXPORT_BACKOFF_MULTIPLIER, + isExportRetryable } from '../../util'; /** @@ -103,7 +104,7 @@ export function sendWithHttp( // clear all timers since request was completed and promise was resolved clearTimeout(exporterTimer); clearTimeout(retryTimer); - } else if (res.statusCode && isRetryable(res.statusCode) && retries > 0) { + } else if (res.statusCode && isExportRetryable(res.statusCode) && retries > 0) { retryTimer = setTimeout(() => { sendWithRetry(retries - 1, backoffMillis * DEFAULT_EXPORT_BACKOFF_MULTIPLIER); }, backoffMillis); @@ -164,12 +165,6 @@ export function sendWithHttp( sendWithRetry(); } -function isRetryable(statusCode: number): boolean { - const retryCodes = [429, 502, 503, 504]; - - return retryCodes.includes(statusCode); -} - function readableFromBuffer(buff: string | Buffer): Readable { const readable = new Readable(); readable.push(buff); diff --git a/experimental/packages/otlp-exporter-base/src/util.ts b/experimental/packages/otlp-exporter-base/src/util.ts index 4ae24daa1b9..7f39a95812c 100644 --- a/experimental/packages/otlp-exporter-base/src/util.ts +++ b/experimental/packages/otlp-exporter-base/src/util.ts @@ -102,3 +102,9 @@ export function invalidTimeout(timeout: number, defaultTimeout: number): number return defaultTimeout; } + +export function isExportRetryable(statusCode: number): boolean { + const retryCodes = [429, 502, 503, 504]; + + return retryCodes.includes(statusCode); +} From 213d22800947776ca203c6220ffb0e721b376946 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 20 Oct 2022 15:57:01 -0500 Subject: [PATCH 07/19] feat(otlp-exporter-base): add jitter Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../browser/CollectorTraceExporter.test.ts | 2 +- .../test/node/CollectorTraceExporter.test.ts | 39 ------------------- .../src/platform/browser/util.ts | 10 +++-- .../src/platform/node/util.ts | 10 +++-- .../packages/otlp-exporter-base/src/util.ts | 3 +- 5 files changed, 17 insertions(+), 47 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 7a9ef8404bf..1093741b615 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -630,7 +630,7 @@ describe('export with retry - real http request destroyed', () => { spans.push(Object.assign({}, mockedReadableSpan)); let retry = 0; - server.respondWith('http://localhost:4318/v1/traces', function (xhr: any, id: any) { + server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { retry++; xhr.respond(502); }); 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 3b2cd4c60f7..22e18dfedba 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 @@ -553,42 +553,3 @@ describe('export - real http request destroyed before response received', () => }, 0); }); }); - -describe('export with retry - real http request destroyed after response received', () => { - let collectorExporter: OTLPTraceExporter; - let collectorExporterConfig: OTLPExporterNodeConfigBase; - let spans: ReadableSpan[]; - let retries = 0; - - const server = http.createServer((_, res) => { - retries++; - if (retries >= 2) { - res.statusCode = 200; - } else { - res.statusCode = 502; - } - res.end(); - }); - before(done => { - server.listen(8081, done); - }); - after(function (done) { - server.close(done); - }); - it('should log the timeout request error message', done => { - collectorExporterConfig = { - url: 'http://localhost:8081', - timeoutMillis: 1500, - }; - collectorExporter = new OTLPTraceExporter(collectorExporterConfig); - spans = []; - spans.push(Object.assign({}, mockedReadableSpan)); - - collectorExporter.export(spans, result => { - assert.strictEqual(result.code, core.ExportResultCode.SUCCESS); - assert.strictEqual(retries, 2); - done(); - }); - }).timeout(3000); -}); - diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index c97406cbed8..739ef6078d6 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -19,6 +19,7 @@ import { DEFAULT_EXPORT_MAX_ATTEMPTS, DEFAULT_EXPORT_INITIAL_BACKOFF, DEFAULT_EXPORT_BACKOFF_MULTIPLIER, + DEFAULT_EXPORT_MAX_BACKOFF, isExportRetryable } from '../../util'; @@ -83,7 +84,7 @@ export function sendWithXhr( } }, exporterTimeout); - const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, backoffMillis = DEFAULT_EXPORT_INITIAL_BACKOFF) => { + const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, minDelay = DEFAULT_EXPORT_INITIAL_BACKOFF) => { xhr = new XMLHttpRequest(); xhr.open('POST', url); @@ -109,9 +110,12 @@ export function sendWithXhr( clearTimeout(exporterTimer); clearTimeout(retryTimer); } else if (xhr.status && isExportRetryable(xhr.status) && retries > 0) { + minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; + const delayWithJitter = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + retryTimer = setTimeout(() => { - sendWithRetry(retries - 1, backoffMillis * DEFAULT_EXPORT_BACKOFF_MULTIPLIER); - }, backoffMillis); + sendWithRetry(retries - 1, minDelay); + }, delayWithJitter); } else { const error = new OTLPExporterError( `Failed to export with XHR (status: ${xhr.status})`, 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 5fb8ada5d01..e2d3f117f8b 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -28,6 +28,7 @@ import { DEFAULT_EXPORT_MAX_ATTEMPTS, DEFAULT_EXPORT_INITIAL_BACKOFF, DEFAULT_EXPORT_BACKOFF_MULTIPLIER, + DEFAULT_EXPORT_MAX_BACKOFF, isExportRetryable } from '../../util'; @@ -82,7 +83,7 @@ export function sendWithHttp( const request = parsedUrl.protocol === 'http:' ? http.request : https.request; - const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, backoffMillis = DEFAULT_EXPORT_INITIAL_BACKOFF) => { + 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)); @@ -105,9 +106,12 @@ export function sendWithHttp( clearTimeout(exporterTimer); clearTimeout(retryTimer); } else if (res.statusCode && isExportRetryable(res.statusCode) && retries > 0) { + minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; + const delayWithJitter = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + retryTimer = setTimeout(() => { - sendWithRetry(retries - 1, backoffMillis * DEFAULT_EXPORT_BACKOFF_MULTIPLIER); - }, backoffMillis); + sendWithRetry(retries - 1, minDelay); + }, delayWithJitter); } else { const error = new OTLPExporterError( res.statusMessage, diff --git a/experimental/packages/otlp-exporter-base/src/util.ts b/experimental/packages/otlp-exporter-base/src/util.ts index 7f39a95812c..56a9110ebeb 100644 --- a/experimental/packages/otlp-exporter-base/src/util.ts +++ b/experimental/packages/otlp-exporter-base/src/util.ts @@ -18,8 +18,9 @@ import { diag } from '@opentelemetry/api'; import { getEnv } from '@opentelemetry/core'; const DEFAULT_TRACE_TIMEOUT = 10000; -export const DEFAULT_EXPORT_MAX_ATTEMPTS = 4; +export const DEFAULT_EXPORT_MAX_ATTEMPTS = 5; export const DEFAULT_EXPORT_INITIAL_BACKOFF = 1000; +export const DEFAULT_EXPORT_MAX_BACKOFF = 5000; export const DEFAULT_EXPORT_BACKOFF_MULTIPLIER = 1.5; /** From 5987db3c859972c4e496d08f88d4ae46d0e45ab6 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 20 Oct 2022 16:05:26 -0500 Subject: [PATCH 08/19] feat(otlp-exporter-base): initialize reqIsDestroyed to false Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../test/browser/CollectorTraceExporter.test.ts | 2 +- .../otlp-exporter-base/src/platform/browser/util.ts | 4 ++-- .../packages/otlp-exporter-base/src/platform/node/util.ts | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 1093741b615..24953d56d12 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -640,7 +640,7 @@ describe('export with retry - real http request destroyed', () => { const error = result.error as OTLPExporterError; assert.ok(error !== undefined); assert.strictEqual(error.message, 'Request Timeout'); - assert.strictEqual(retry, 2); + assert.strictEqual(retry, 1); done(); }); }).timeout(3000); diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 739ef6078d6..b3f093b6ec2 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -66,9 +66,9 @@ export function sendWithXhr( onSuccess: () => void, onError: (error: OTLPExporterError) => void ): void { - let reqIsDestroyed: boolean; let retryTimer: ReturnType; let xhr: XMLHttpRequest; + let reqIsDestroyed = false; const exporterTimer = setTimeout(() => { clearTimeout(retryTimer); @@ -103,7 +103,7 @@ export function sendWithXhr( xhr.send(body); xhr.onreadystatechange = () => { - if (xhr.readyState === XMLHttpRequest.DONE && reqIsDestroyed === undefined) { + if (xhr.readyState === XMLHttpRequest.DONE && reqIsDestroyed === false) { if (xhr.status >= 200 && xhr.status <= 299) { diag.debug('xhr success', body); onSuccess(); 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 e2d3f117f8b..bb6e26601a1 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -49,11 +49,11 @@ export function sendWithHttp( ): void { const exporterTimeout = collector.timeoutMillis; const parsedUrl = new url.URL(collector.url); - let reqIsDestroyed: boolean; 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; @@ -98,7 +98,7 @@ export function sendWithHttp( }); res.on('end', () => { - if (reqIsDestroyed === undefined) { + if (reqIsDestroyed === false) { if (res.statusCode && res.statusCode < 299) { diag.debug(`statusCode: ${res.statusCode}`, responseData); onSuccess(); From 917e5ccac968cd35122a116d8a6fc96eeb4da1d2 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 21 Oct 2022 14:43:31 -0500 Subject: [PATCH 09/19] feat(otlp-exporter-base): add throttle logic Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../browser/CollectorTraceExporter.test.ts | 75 ++++++++++++++++++- .../src/platform/browser/util.ts | 26 ++++++- .../src/platform/node/util.ts | 29 ++++++- 3 files changed, 123 insertions(+), 7 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 24953d56d12..2db4bf85e14 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -624,15 +624,86 @@ describe('export with retry - real http request destroyed', () => { (window.navigator as any).sendBeacon = false; collectorTraceExporter = new OTLPTraceExporter(collectorExporterConfig); }); + it('should log the timeout request error message when retrying with exponential backoff with jitter', done => { + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + let retry = 0; + server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { + retry++; + xhr.respond(503); + }); - it('should log the timeout request error message', done => { + collectorTraceExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + assert.strictEqual(retry, 1); + done(); + }); + }).timeout(3000); + + it('should log the timeout request error message when retry-after header is set to 3 seconds', done => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); let retry = 0; server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { retry++; - xhr.respond(502); + xhr.respond( + 503, + {"Retry-After": 3} + ); + }); + + collectorTraceExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + assert.strictEqual(retry, 1); + done(); + }); + }).timeout(3000); + it('should log the timeout request error message when retry-after header is a date', done => { + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + + let retry = 0; + server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { + retry++; + let d = new Date(); + d.setSeconds(d.getSeconds() + 1) + xhr.respond( + 503, + {"Retry-After": d} + ); + }); + + collectorTraceExporter.export(spans, result => { + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + const error = result.error as OTLPExporterError; + assert.ok(error !== undefined); + assert.strictEqual(error.message, 'Request Timeout'); + assert.strictEqual(retry, 2); + done(); + }); + }).timeout(3000); + it('should log the timeout request error message when retry-after header is a date with long delay', done => { + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + + let retry = 0; + server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { + retry++; + let d = new Date(); + d.setSeconds(d.getSeconds() + 120) + xhr.respond( + 503, + {"Retry-After": d} + ); }); collectorTraceExporter.export(spans, result => { diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index b3f093b6ec2..cef9bbc4315 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -110,12 +110,20 @@ export function sendWithXhr( clearTimeout(exporterTimer); clearTimeout(retryTimer); } else if (xhr.status && isExportRetryable(xhr.status) && retries > 0) { + let retryTime: number; minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; - const delayWithJitter = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + + // retry after interval specified in Retry-After header + if (xhr.getResponseHeader('Retry-After') !== null) { + retryTime = retrieveThrottleTime(xhr.getResponseHeader('Retry-After')!); + } else { + // exponential backoff with jitter + retryTime = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + } retryTimer = setTimeout(() => { sendWithRetry(retries - 1, minDelay); - }, delayWithJitter); + }, retryTime); } else { const error = new OTLPExporterError( `Failed to export with XHR (status: ${xhr.status})`, @@ -153,3 +161,17 @@ export function sendWithXhr( sendWithRetry(); } + +function retrieveThrottleTime(retryAfter: string): number { + // it's a Date object + if (typeof retryAfter === 'object') { + const currentTime = new Date(); + const retryAfterDate = new Date(retryAfter); + + const secondsDiff = Math.round((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + return secondsDiff * 1000; + // it's an integer + } else { + return Number(retryAfter) * 1000; + } +} 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 bb6e26601a1..11af8e940dc 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -53,7 +53,7 @@ export function sendWithHttp( let retryTimer: ReturnType; let req: http.ClientRequest; let reqIsDestroyed = false; - + const exporterTimer = setTimeout(() => { clearTimeout(retryTimer); reqIsDestroyed = true; @@ -106,12 +106,20 @@ export function sendWithHttp( clearTimeout(exporterTimer); clearTimeout(retryTimer); } else if (res.statusCode && isExportRetryable(res.statusCode) && retries > 0) { + let retryTime: number; minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; - const delayWithJitter = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + + // retry after interval specified in Retry-After header + if (res.headers['retry-after'] !== null) { + retryTime = retrieveThrottleTime(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); - }, delayWithJitter); + }, retryTime); } else { const error = new OTLPExporterError( res.statusMessage, @@ -207,3 +215,18 @@ export function configureCompression(compression: CompressionAlgorithm | undefin return definedCompression === CompressionAlgorithm.GZIP ? CompressionAlgorithm.GZIP : CompressionAlgorithm.NONE; } } + +function retrieveThrottleTime(retryAfter: string): number { + // it's a Date object + // a string representing a date will return NaN when converted to a number + if (isNaN(Number(retryAfter))) { + const currentTime = new Date(); + const retryAfterDate = new Date(retryAfter); + + const secondsDiff = Math.round((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + return secondsDiff * 1000; + // it's an integer + } else { + return Number(retryAfter) * 1000; + } +} From 7e1d0c8e8c47cfc4e25ee4ac5cacacd3daf84c88 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 21 Oct 2022 15:06:32 -0500 Subject: [PATCH 10/19] feat(otlp-exporter-base): add retry to readme Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../packages/exporter-trace-otlp-http/README.md | 15 +++++++++++++++ .../test/browser/CollectorTraceExporter.test.ts | 15 +++++++-------- .../packages/exporter-trace-otlp-proto/README.md | 15 +++++++++++++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/README.md b/experimental/packages/exporter-trace-otlp-http/README.md index 448c8caf201..689c811dd31 100644 --- a/experimental/packages/exporter-trace-otlp-http/README.md +++ b/experimental/packages/exporter-trace-otlp-http/README.md @@ -139,6 +139,21 @@ To override the default timeout duration, use the following options: > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. +## OTLP Exporter Retry + +OTLP requires that transient errors be handled with a [retry strategy](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#retry). + +This retry policy has the following configuration, which there is currently no way to customize. + ++ `DEFAULT_EXPORT_MAX_ATTEMPTS`: The maximum number of attempts, including the original request. Defaults to 5. ++ `DEFAULT_EXPORT_INITIAL_BACKOFF`: The initial backoff duration. Defaults to 1 second. ++ `DEFAULT_EXPORT_MAX_BACKOFF`: The maximum backoff duration. Defaults to 5 seconds. ++ `DEFAULT_EXPORT_BACKOFF_MULTIPLIER`: The backoff multiplier. Defaults to 1.5. + +This retry policy first checks if the response has a `'Retry-After'` header. If there is a `'Retry-After'` header, the exporter will wait the amount specified in the `'Retry-After'` header before retrying. If there is no `'Retry-After'` header, the exporter will use an exponential backoff with jitter retry strategy. + + > The exporter will retry exporting within the [exporter timeout configuration](#Exporter-Timeout-Configuration) time. + ## Running opentelemetry-collector locally to see the traces 1. Go to `examples/otlp-exporter-node` diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 2db4bf85e14..cbf81fce808 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -653,7 +653,7 @@ describe('export with retry - real http request destroyed', () => { retry++; xhr.respond( 503, - {"Retry-After": 3} + {'Retry-After': 3} ); }); @@ -670,15 +670,14 @@ describe('export with retry - real http request destroyed', () => { spans = []; spans.push(Object.assign({}, mockedReadableSpan)); - let retry = 0; server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { retry++; - let d = new Date(); - d.setSeconds(d.getSeconds() + 1) + const d = new Date(); + d.setSeconds(d.getSeconds() + 1); xhr.respond( 503, - {"Retry-After": d} + {'Retry-After': d} ); }); @@ -698,11 +697,11 @@ describe('export with retry - real http request destroyed', () => { let retry = 0; server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { retry++; - let d = new Date(); - d.setSeconds(d.getSeconds() + 120) + const d = new Date(); + d.setSeconds(d.getSeconds() + 120); xhr.respond( 503, - {"Retry-After": d} + {'Retry-After': d} ); }); diff --git a/experimental/packages/exporter-trace-otlp-proto/README.md b/experimental/packages/exporter-trace-otlp-proto/README.md index 56551f92887..ce6f9c046dd 100644 --- a/experimental/packages/exporter-trace-otlp-proto/README.md +++ b/experimental/packages/exporter-trace-otlp-proto/README.md @@ -70,6 +70,21 @@ To override the default timeout duration, use the following options: > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. +## OTLP Exporter Retry + +OTLP requires that transient errors be handled with a [retry strategy](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#retry). + +This retry policy has the following configuration, which there is currently no way to customize. + ++ `DEFAULT_EXPORT_MAX_ATTEMPTS`: The maximum number of attempts, including the original request. Defaults to 5. ++ `DEFAULT_EXPORT_INITIAL_BACKOFF`: The initial backoff duration. Defaults to 1 second. ++ `DEFAULT_EXPORT_MAX_BACKOFF`: The maximum backoff duration. Defaults to 5 seconds. ++ `DEFAULT_EXPORT_BACKOFF_MULTIPLIER`: The backoff multiplier. Defaults to 1.5. + +This retry policy first checks if the response has a `'Retry-After'` header. If there is a `'Retry-After'` header, the exporter will wait the amount specified in the `'Retry-After'` header before retrying. If there is no `'Retry-After'` header, the exporter will use an exponential backoff with jitter retry strategy. + + > The exporter will retry exporting within the [exporter timeout configuration](#Exporter-Timeout-Configuration) time. + ## Running opentelemetry-collector locally to see the traces 1. Go to examples/otlp-exporter-node From 7e45c91b68709701399ab25dd13d311417f01531 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 21 Oct 2022 15:10:22 -0500 Subject: [PATCH 11/19] feat(otlp-exporter-base): add changelog Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- experimental/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 44e89e8133d..5d445af9c52 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to experimental packages in this project will be documented * feat: enable tree shaking [#3329](https://github.com/open-telemetry/opentelemetry-js/pull/3329) @pkanal * feat(prometheus): serialize resource as target_info gauge [#3300](https://github.com/open-telemetry/opentelemetry-js/pull/3300) @pichlermarc * deps: remove unused proto-loader dependencies and update grpc-js and proto-loader versions [#3337](https://github.com/open-telemetry/opentelemetry-js/pull/3337) @seemk +* feat(otlp-exporter-base): add retries [#3207](https://github.com/open-telemetry/opentelemetry-js/pull/3207) @svetlanabrennan ### :bug: (Bug Fix) From e327d18ae9ff4eada5381475733a48230d163c46 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 24 Oct 2022 13:55:53 -0500 Subject: [PATCH 12/19] feat(otlp-exporter-base): update throttle time function Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../otlp-exporter-base/src/platform/browser/util.ts | 8 ++++++-- .../otlp-exporter-base/src/platform/node/util.ts | 11 +++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index cef9bbc4315..af33b2c983d 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -114,7 +114,7 @@ export function sendWithXhr( minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; // retry after interval specified in Retry-After header - if (xhr.getResponseHeader('Retry-After') !== null) { + if (xhr.getResponseHeader('Retry-After')) { retryTime = retrieveThrottleTime(xhr.getResponseHeader('Retry-After')!); } else { // exponential backoff with jitter @@ -167,8 +167,12 @@ function retrieveThrottleTime(retryAfter: string): number { if (typeof retryAfter === 'object') { const currentTime = new Date(); const retryAfterDate = new Date(retryAfter); + let secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); - const secondsDiff = Math.round((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + // if throttle date is set to now, difference in seconds might be less than 0 + if (secondsDiff <= 0) { + secondsDiff = 0; + } return secondsDiff * 1000; // it's an integer } else { 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 11af8e940dc..7bd0516ee4d 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -110,7 +110,7 @@ export function sendWithHttp( minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; // retry after interval specified in Retry-After header - if (res.headers['retry-after'] !== null) { + if (res.headers['retry-after']) { retryTime = retrieveThrottleTime(res.headers['retry-after']!); } else { // exponential backoff with jitter @@ -217,13 +217,16 @@ export function configureCompression(compression: CompressionAlgorithm | undefin } function retrieveThrottleTime(retryAfter: string): number { - // it's a Date object - // a string representing a date will return NaN when converted to a number + // it's a Date object - a string representing a date will return NaN when converted to a number if (isNaN(Number(retryAfter))) { const currentTime = new Date(); const retryAfterDate = new Date(retryAfter); + let secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); - const secondsDiff = Math.round((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + // if throttle date is set to now, difference in seconds might be less than 0 + if (secondsDiff <= 0) { + secondsDiff = 0; + } return secondsDiff * 1000; // it's an integer } else { From 3855c0fbfd9543bb14789d8125accd9e40a79c17 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 24 Oct 2022 14:04:01 -0500 Subject: [PATCH 13/19] feat(otlp-exporter-base): refactor sec difference in throttle fun Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../packages/otlp-exporter-base/src/platform/browser/util.ts | 5 +++-- .../packages/otlp-exporter-base/src/platform/node/util.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index af33b2c983d..ff748effc22 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -171,9 +171,10 @@ function retrieveThrottleTime(retryAfter: string): number { // if throttle date is set to now, difference in seconds might be less than 0 if (secondsDiff <= 0) { - secondsDiff = 0; + return 0; + } else { + return secondsDiff * 1000; } - return secondsDiff * 1000; // it's an integer } else { return Number(retryAfter) * 1000; 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 7bd0516ee4d..08b361848ed 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -225,9 +225,10 @@ function retrieveThrottleTime(retryAfter: string): number { // if throttle date is set to now, difference in seconds might be less than 0 if (secondsDiff <= 0) { - secondsDiff = 0; + return 0; + } else { + return secondsDiff * 1000; } - return secondsDiff * 1000; // it's an integer } else { return Number(retryAfter) * 1000; From 4a658e703c96fa299b546a6086fcfd22c497cc4b Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Mon, 24 Oct 2022 14:18:02 -0500 Subject: [PATCH 14/19] feat(otlp-exporter-base): fix lint Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../packages/otlp-exporter-base/src/platform/browser/util.ts | 2 +- .../packages/otlp-exporter-base/src/platform/node/util.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index ff748effc22..7a29fa9739c 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -167,7 +167,7 @@ function retrieveThrottleTime(retryAfter: string): number { if (typeof retryAfter === 'object') { const currentTime = new Date(); const retryAfterDate = new Date(retryAfter); - let secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + const secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); // if throttle date is set to now, difference in seconds might be less than 0 if (secondsDiff <= 0) { 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 08b361848ed..8d8044e1b69 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -221,7 +221,7 @@ function retrieveThrottleTime(retryAfter: string): number { if (isNaN(Number(retryAfter))) { const currentTime = new Date(); const retryAfterDate = new Date(retryAfter); - let secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + const secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); // if throttle date is set to now, difference in seconds might be less than 0 if (secondsDiff <= 0) { From 549e6f65f0b07b14f24655c0b667e413a56b6f29 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 13 Jan 2023 15:25:46 -0600 Subject: [PATCH 15/19] feat(otlp-exporter-base): fix lint Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../src/platform/browser/util.ts | 36 ++++++------ .../src/platform/node/util.ts | 46 +++++++++------- .../otlp-exporter-base/test/node/util.test.ts | 55 ++++++++++++------- 3 files changed, 81 insertions(+), 56 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 90ba6057414..11db34f409f 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -20,7 +20,7 @@ import { DEFAULT_EXPORT_INITIAL_BACKOFF, DEFAULT_EXPORT_BACKOFF_MULTIPLIER, DEFAULT_EXPORT_MAX_BACKOFF, - isExportRetryable + isExportRetryable, } from '../../util'; /** @@ -73,21 +73,22 @@ export function sendWithXhr( reqIsDestroyed = true; if (xhr.readyState === XMLHttpRequest.DONE) { - const err = new OTLPExporterError( - 'Request Timeout' - ); + const err = new OTLPExporterError('Request Timeout'); onError(err); } else { xhr.abort(); } }, exporterTimeout); - const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, minDelay = DEFAULT_EXPORT_INITIAL_BACKOFF) => { + const sendWithRetry = ( + retries = DEFAULT_EXPORT_MAX_ATTEMPTS, + minDelay = DEFAULT_EXPORT_INITIAL_BACKOFF + ) => { xhr = new XMLHttpRequest(); xhr.open('POST', url); const defaultHeaders = { - 'Accept': 'application/json', + Accept: 'application/json', 'Content-Type': 'application/json', }; @@ -113,10 +114,14 @@ export function sendWithXhr( // retry after interval specified in Retry-After header if (xhr.getResponseHeader('Retry-After')) { - retryTime = retrieveThrottleTime(xhr.getResponseHeader('Retry-After')!); + retryTime = retrieveThrottleTime( + xhr.getResponseHeader('Retry-After')! + ); } else { // exponential backoff with jitter - retryTime = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + retryTime = Math.round( + Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay + ); } retryTimer = setTimeout(() => { @@ -136,9 +141,7 @@ export function sendWithXhr( xhr.onabort = () => { if (reqIsDestroyed) { - const err = new OTLPExporterError( - 'Request Timeout' - ); + const err = new OTLPExporterError('Request Timeout'); onError(err); } clearTimeout(exporterTimer); @@ -147,10 +150,7 @@ export function sendWithXhr( xhr.onerror = () => { if (reqIsDestroyed) { - const err = new OTLPExporterError( - 'Request Timeout' - - ); + const err = new OTLPExporterError('Request Timeout'); onError(err); } clearTimeout(exporterTimer); @@ -166,7 +166,9 @@ function retrieveThrottleTime(retryAfter: string): number { if (typeof retryAfter === 'object') { const currentTime = new Date(); const retryAfterDate = new Date(retryAfter); - const secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + const secondsDiff = Math.ceil( + (retryAfterDate.getTime() - currentTime.getTime()) / 1000 + ); // if throttle date is set to now, difference in seconds might be less than 0 if (secondsDiff <= 0) { @@ -174,7 +176,7 @@ function retrieveThrottleTime(retryAfter: string): number { } else { return secondsDiff * 1000; } - // it's an integer + // it's an integer } else { return Number(retryAfter) * 1000; } 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 8c7a00e57c1..67361f95b3b 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -29,7 +29,7 @@ import { DEFAULT_EXPORT_INITIAL_BACKOFF, DEFAULT_EXPORT_BACKOFF_MULTIPLIER, DEFAULT_EXPORT_MAX_BACKOFF, - isExportRetryable + isExportRetryable, } from '../../util'; /** @@ -59,9 +59,7 @@ export function sendWithHttp( reqIsDestroyed = true; if (req.destroyed) { - const err = new OTLPExporterError( - 'Request Timeout' - ); + const err = new OTLPExporterError('Request Timeout'); onError(err); } else { // req.abort() was deprecated since v14 @@ -83,16 +81,17 @@ export function sendWithHttp( const request = parsedUrl.protocol === 'http:' ? http.request : https.request; - const sendWithRetry = (retries = DEFAULT_EXPORT_MAX_ATTEMPTS, minDelay = DEFAULT_EXPORT_INITIAL_BACKOFF) => { + 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' - ); + const err = new OTLPExporterError('Request Timeout'); onError(err); } }); @@ -105,7 +104,11 @@ export function sendWithHttp( // clear all timers since request was completed and promise was resolved clearTimeout(exporterTimer); clearTimeout(retryTimer); - } else if (res.statusCode && isExportRetryable(res.statusCode) && retries > 0) { + } else if ( + res.statusCode && + isExportRetryable(res.statusCode) && + retries > 0 + ) { let retryTime: number; minDelay = DEFAULT_EXPORT_BACKOFF_MULTIPLIER * minDelay; @@ -114,7 +117,10 @@ export function sendWithHttp( retryTime = retrieveThrottleTime(res.headers['retry-after']!); } else { // exponential backoff with jitter - retryTime = Math.round(Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + minDelay); + retryTime = Math.round( + Math.random() * (DEFAULT_EXPORT_MAX_BACKOFF - minDelay) + + minDelay + ); } retryTimer = setTimeout(() => { @@ -137,9 +143,7 @@ export function sendWithHttp( req.on('error', (error: Error | any) => { if (reqIsDestroyed) { - const err = new OTLPExporterError( - 'Request Timeout', error.code - ); + const err = new OTLPExporterError('Request Timeout', error.code); onError(err); } else { onError(error); @@ -150,9 +154,7 @@ export function sendWithHttp( req.on('abort', () => { if (reqIsDestroyed) { - const err = new OTLPExporterError( - 'Request Timeout' - ); + const err = new OTLPExporterError('Request Timeout'); onError(err); } clearTimeout(exporterTimer); @@ -163,8 +165,10 @@ export function sendWithHttp( case CompressionAlgorithm.GZIP: { req.setHeader('Content-Encoding', 'gzip'); const dataStream = readableFromBuffer(data); - dataStream.on('error', onError) - .pipe(zlib.createGzip()).on('error', onError) + dataStream + .on('error', onError) + .pipe(zlib.createGzip()) + .on('error', onError) .pipe(req); break; @@ -227,7 +231,9 @@ function retrieveThrottleTime(retryAfter: string): number { if (isNaN(Number(retryAfter))) { const currentTime = new Date(); const retryAfterDate = new Date(retryAfter); - const secondsDiff = Math.ceil((retryAfterDate.getTime() - currentTime.getTime()) / 1000); + const secondsDiff = Math.ceil( + (retryAfterDate.getTime() - currentTime.getTime()) / 1000 + ); // if throttle date is set to now, difference in seconds might be less than 0 if (secondsDiff <= 0) { @@ -235,7 +241,7 @@ function retrieveThrottleTime(retryAfter: string): number { } else { return secondsDiff * 1000; } - // it's an integer + // it's an integer } else { return Number(retryAfter) * 1000; } 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 5d698086102..86c8df40ab0 100644 --- a/experimental/packages/otlp-exporter-base/test/node/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/node/util.test.ts @@ -228,12 +228,18 @@ describe('sendWithHttp', () => { // 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); - }); + 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(); }); @@ -257,15 +263,20 @@ describe('sendWithHttp', () => { // 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); - }); + 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', () => { @@ -299,12 +310,18 @@ describe('sendWithHttp', () => { // 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); - }); + 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(); } From e9b6981445fd562a07464a23f861a32628cece1d Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Fri, 13 Jan 2023 15:43:34 -0600 Subject: [PATCH 16/19] feat(otlp-exporter-base): fix lint Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../browser/CollectorTraceExporter.test.ts | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts index 537d0b9cd69..4e8bc1d6e17 100644 --- a/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-http/test/browser/CollectorTraceExporter.test.ts @@ -591,10 +591,10 @@ describe('export with retry - real http request destroyed', () => { beforeEach(() => { server = sinon.fakeServer.create({ - autoRespond: true + autoRespond: true, }); collectorExporterConfig = { - timeoutMillis: 1500 + timeoutMillis: 1500, }; }); @@ -612,10 +612,13 @@ describe('export with retry - real http request destroyed', () => { spans.push(Object.assign({}, mockedReadableSpan)); let retry = 0; - server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { - retry++; - xhr.respond(503); - }); + server.respondWith( + 'http://localhost:4318/v1/traces', + function (xhr: any) { + retry++; + xhr.respond(503); + } + ); collectorTraceExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); @@ -632,13 +635,13 @@ describe('export with retry - real http request destroyed', () => { spans.push(Object.assign({}, mockedReadableSpan)); let retry = 0; - server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { - retry++; - xhr.respond( - 503, - {'Retry-After': 3} - ); - }); + server.respondWith( + 'http://localhost:4318/v1/traces', + function (xhr: any) { + retry++; + xhr.respond(503, { 'Retry-After': 3 }); + } + ); collectorTraceExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); @@ -654,15 +657,15 @@ describe('export with retry - real http request destroyed', () => { spans.push(Object.assign({}, mockedReadableSpan)); let retry = 0; - server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { - retry++; - const d = new Date(); - d.setSeconds(d.getSeconds() + 1); - xhr.respond( - 503, - {'Retry-After': d} - ); - }); + server.respondWith( + 'http://localhost:4318/v1/traces', + function (xhr: any) { + retry++; + const d = new Date(); + d.setSeconds(d.getSeconds() + 1); + xhr.respond(503, { 'Retry-After': d }); + } + ); collectorTraceExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); @@ -678,15 +681,15 @@ describe('export with retry - real http request destroyed', () => { spans.push(Object.assign({}, mockedReadableSpan)); let retry = 0; - server.respondWith('http://localhost:4318/v1/traces', function (xhr: any) { - retry++; - const d = new Date(); - d.setSeconds(d.getSeconds() + 120); - xhr.respond( - 503, - {'Retry-After': d} - ); - }); + server.respondWith( + 'http://localhost:4318/v1/traces', + function (xhr: any) { + retry++; + const d = new Date(); + d.setSeconds(d.getSeconds() + 120); + xhr.respond(503, { 'Retry-After': d }); + } + ); collectorTraceExporter.export(spans, result => { assert.strictEqual(result.code, core.ExportResultCode.FAILED); From 90450271bd0ffe86f5b8d8d4ff5e50650995168d Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 16 Feb 2023 10:33:24 -0600 Subject: [PATCH 17/19] feat(otlp-exporter-base): refactor retrieve throttle time func Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../src/platform/browser/util.ts | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 2cd1c60c9c0..0a78b1fe117 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -161,23 +161,19 @@ export function sendWithXhr( sendWithRetry(); } -function retrieveThrottleTime(retryAfter: string): number { - // it's a Date object - if (typeof retryAfter === 'object') { - const currentTime = new Date(); - const retryAfterDate = new Date(retryAfter); - const secondsDiff = Math.ceil( - (retryAfterDate.getTime() - currentTime.getTime()) / 1000 - ); - - // if throttle date is set to now, difference in seconds might be less than 0 - if (secondsDiff <= 0) { - return 0; - } else { - return secondsDiff * 1000; - } - // it's an integer - } else { - return Number(retryAfter) * 1000; +function retrieveThrottleTime(retryAfter?: string | null): number { + if (retryAfter == null) { + return -1; } -} + 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; +} \ No newline at end of file From ea268a37965ea47b35b26fb1dc83ac81f44bf269 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Thu, 16 Feb 2023 10:45:06 -0600 Subject: [PATCH 18/19] feat(otlp-exporter-base): fix lint Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../packages/otlp-exporter-base/src/platform/browser/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 0a78b1fe117..1fa8d1db3c3 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -176,4 +176,4 @@ function retrieveThrottleTime(retryAfter?: string | null): number { return delay; } return 0; -} \ No newline at end of file +} From 1074c2a6a4bfb85a724d282ae4b6d97bbe73536a Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Wed, 22 Feb 2023 11:57:11 -0600 Subject: [PATCH 19/19] feat(otlp-exporter-base): move parseRetryAfterToMills to utils file Signed-off-by: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> --- .../src/platform/browser/util.ts | 20 ++----------- .../src/platform/node/util.ts | 24 ++-------------- .../packages/otlp-exporter-base/src/util.ts | 17 +++++++++++ .../test/common/util.test.ts | 28 +++++++++++++++++++ 4 files changed, 49 insertions(+), 40 deletions(-) diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts index 1fa8d1db3c3..fade4afa88b 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -21,6 +21,7 @@ import { DEFAULT_EXPORT_BACKOFF_MULTIPLIER, DEFAULT_EXPORT_MAX_BACKOFF, isExportRetryable, + parseRetryAfterToMills, } from '../../util'; /** @@ -114,7 +115,7 @@ export function sendWithXhr( // retry after interval specified in Retry-After header if (xhr.getResponseHeader('Retry-After')) { - retryTime = retrieveThrottleTime( + retryTime = parseRetryAfterToMills( xhr.getResponseHeader('Retry-After')! ); } else { @@ -160,20 +161,3 @@ export function sendWithXhr( sendWithRetry(); } - -function retrieveThrottleTime(retryAfter?: string | null): number { - if (retryAfter == null) { - return -1; - } - 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/node/util.ts b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts index 67361f95b3b..fd40981e857 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -30,6 +30,7 @@ import { DEFAULT_EXPORT_BACKOFF_MULTIPLIER, DEFAULT_EXPORT_MAX_BACKOFF, isExportRetryable, + parseRetryAfterToMills, } from '../../util'; /** @@ -114,7 +115,7 @@ export function sendWithHttp( // retry after interval specified in Retry-After header if (res.headers['retry-after']) { - retryTime = retrieveThrottleTime(res.headers['retry-after']!); + retryTime = parseRetryAfterToMills(res.headers['retry-after']!); } else { // exponential backoff with jitter retryTime = Math.round( @@ -225,24 +226,3 @@ export function configureCompression( : CompressionAlgorithm.NONE; } } - -function retrieveThrottleTime(retryAfter: string): number { - // it's a Date object - a string representing a date will return NaN when converted to a number - if (isNaN(Number(retryAfter))) { - const currentTime = new Date(); - const retryAfterDate = new Date(retryAfter); - const secondsDiff = Math.ceil( - (retryAfterDate.getTime() - currentTime.getTime()) / 1000 - ); - - // if throttle date is set to now, difference in seconds might be less than 0 - if (secondsDiff <= 0) { - return 0; - } else { - return secondsDiff * 1000; - } - // it's an integer - } else { - return Number(retryAfter) * 1000; - } -} diff --git a/experimental/packages/otlp-exporter-base/src/util.ts b/experimental/packages/otlp-exporter-base/src/util.ts index daee5fd6a01..f5dc70c9e88 100644 --- a/experimental/packages/otlp-exporter-base/src/util.ts +++ b/experimental/packages/otlp-exporter-base/src/util.ts @@ -120,3 +120,20 @@ export function isExportRetryable(statusCode: number): boolean { return retryCodes.includes(statusCode); } + +export function parseRetryAfterToMills(retryAfter?: string | null): number { + if (retryAfter == null) { + return -1; + } + 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/test/common/util.test.ts b/experimental/packages/otlp-exporter-base/test/common/util.test.ts index d78b719faa9..b00d1f36a5c 100644 --- a/experimental/packages/otlp-exporter-base/test/common/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/common/util.test.ts @@ -21,6 +21,7 @@ import { parseHeaders, appendResourcePathToUrl, appendRootPathToUrlIfNeeded, + parseRetryAfterToMills, } from '../../src/util'; describe('utils', () => { @@ -117,3 +118,30 @@ describe('utils', () => { }); }); }); + +describe('parseRetryAfterToMills', () => { + // now: 2023-01-20T00:00:00.000Z + const tests = [ + [null, -1], + // duration + ['-100', -1], + ['1000', 1000 * 1000], + // future timestamp + ['Fri, 20 Jan 2023 00:00:01 GMT', 1000], + // Past timestamp + ['Fri, 19 Jan 2023 23:59:59 GMT', 0], + ] as [string | null, number][]; + + afterEach(() => { + sinon.restore(); + }); + + for (const [value, expect] of tests) { + it(`test ${value}`, () => { + sinon.useFakeTimers({ + now: new Date('2023-01-20T00:00:00.000Z'), + }); + assert.strictEqual(parseRetryAfterToMills(value), expect); + }); + } +});