From ac578e9a8d165233365064ac65bc2673d900d348 Mon Sep 17 00:00:00 2001 From: Daniel Gomez Blanco Date: Wed, 11 May 2022 20:54:22 +0100 Subject: [PATCH 1/2] fix(opentelemetry-instrumentation-http): handle null ports in options (#2948) Co-authored-by: Daniel Dyla --- experimental/CHANGELOG.md | 1 + .../opentelemetry-instrumentation-http/src/utils.ts | 5 ++--- .../test/functionals/utils.test.ts | 12 ++++++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index f2228b384f..b330cb043f 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 ### :bug: (Bug Fix) +* fix(opentelemetry-instrumentation-http): use correct origin when port is `null` #2948 @danielgblanco * fix(otlp-exporter-base): include esm and esnext in package files #2952 @dyladan ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 68d7be7588..e1e98209db 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -263,9 +263,8 @@ export const getRequestInfo = ( if (!pathname && optionsParsed.path) { pathname = url.parse(optionsParsed.path).pathname || '/'; } - origin = `${optionsParsed.protocol || 'http:'}//${ - optionsParsed.host || `${optionsParsed.hostname}:${optionsParsed.port}` - }`; + const hostname = optionsParsed.host || (optionsParsed.port != null ? `${optionsParsed.hostname}${optionsParsed.port}` : optionsParsed.hostname); + origin = `${optionsParsed.protocol || 'http:'}//${hostname}`; } const headers = optionsParsed.headers ?? {}; diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index 062f160bdc..52211abadc 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -72,11 +72,23 @@ describe('Utility', () => { ...urlParsed, pathname: undefined, }; + const urlParsedWithUndefinedHostAndPort = { + ...urlParsed, + host: undefined, + port: undefined, + }; + const urlParsedWithUndefinedHostAndNullPort = { + ...urlParsed, + host: undefined, + port: null, + }; const whatWgUrl = new url.URL(webUrl); for (const param of [ webUrl, urlParsed, urlParsedWithoutPathname, + urlParsedWithUndefinedHostAndPort, + urlParsedWithUndefinedHostAndNullPort, whatWgUrl, ]) { const result = utils.getRequestInfo(param); From 479321cdcc44355a28c3477e9d5baee9634cfbdb Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Wed, 11 May 2022 15:29:58 -0500 Subject: [PATCH 2/2] feat(otlp-exporter): add timeout env var (#2738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gerhard Stöbich Co-authored-by: Rauno Viskus Co-authored-by: Valentin Marchaud Co-authored-by: Daniel Dyla --- experimental/CHANGELOG.md | 1 + .../exporter-trace-otlp-grpc/README.md | 41 +++- .../test/OTLPTraceExporter.test.ts | 30 ++- .../exporter-trace-otlp-http/README.md | 38 +++- .../browser/CollectorTraceExporter.test.ts | 58 ++++- .../test/node/CollectorTraceExporter.test.ts | 198 +++++++++++++++++- .../exporter-trace-otlp-proto/README.md | 37 +++- .../test/OTLPTraceExporter.test.ts | 125 ++++++++++- .../test/node/CollectorMetricExporter.test.ts | 45 ++-- .../test/OTLPMetricExporter.test.ts | 125 ++++++----- .../src/OTLPExporterBase.ts | 4 + .../browser/OTLPExporterBrowserBase.ts | 2 +- .../src/platform/browser/util.ts | 15 ++ .../src/platform/node/util.ts | 59 ++++-- .../packages/otlp-exporter-base/src/types.ts | 3 + .../packages/otlp-exporter-base/src/util.ts | 41 ++++ .../test/browser/util.test.ts | 37 +++- .../otlp-exporter-base/test/node/util.test.ts | 86 ++++++++ .../otlp-grpc-exporter-base/src/types.ts | 1 + .../otlp-grpc-exporter-base/src/util.ts | 3 +- .../src/utils/environment.ts | 6 + .../test/utils/environment.test.ts | 4 + 22 files changed, 825 insertions(+), 134 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index b330cb043f..0027b4114c 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -63,6 +63,7 @@ All notable changes to experimental packages in this project will be documented * feat(proto): add @opentelemetry/otlp-transformer package with hand-rolled transformation #2746 @dyladan * feat(sdk-metrics-base): shutdown and forceflush on MeterProvider #2890 @legendecas * feat(sdk-metrics-base): return the same meter for identical input to getMeter #2901 @legendecas +* feat(otlp-exporter): add [OTEL_EXPORTER_OTLP_TIMEOUT](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options) env var to otlp exporters #2738 @svetlanabrennan * feat(sdk-metrics-base): hoist async instrument callback invocations #2822 @legendecas ### :bug: (Bug Fix) diff --git a/experimental/packages/exporter-trace-otlp-grpc/README.md b/experimental/packages/exporter-trace-otlp-grpc/README.md index fa58300b9b..0ff58a0afa 100644 --- a/experimental/packages/exporter-trace-otlp-grpc/README.md +++ b/experimental/packages/exporter-trace-otlp-grpc/README.md @@ -108,6 +108,27 @@ provider.register(); Note, that this will only work if TLS is also configured on the server. +## Exporter Timeout Configuration + +The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. + ++ To override the default timeout duration, provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + + ```js + const collectorOptions = { + timeoutMillis: 15000, + // url is optional and can be omitted - default is localhost:4317 + url: ':', + metadata, // // an optional grpc.Metadata object to be sent with each request + }; + + const exporter = new OTLPTraceExporter(collectorOptions); + ``` + + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. + +## Exporter Compression Configuration + By default no compression will be used. To use compression, set it programmatically in `collectorOptions` or with environment variables. Supported compression options: `gzip` and `none`. ```js @@ -126,13 +147,13 @@ const exporter = new OTLPTraceExporter(collectorOptions); ## Environment Variable Configuration -Set compression with environment variables. - -```shell -OTEL_EXPORTER_OTLP_TRACES_COMPRESSION=gzip -``` - - > Compression set programatically in `collectorOptions` takes precedence over compression set with environment variables. `OTEL_EXPORTER_OTLP_TRACES_COMPRESSION` takes precedence and overrides `OTEL_EXPORTER_OTLP_COMPRESSION`. + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TRACES_COMPRESSION | The compression type to use on OTLP trace requests. Options include gzip. By default no compression will be used. | + | OTEL_EXPORTER_OTLP_COMPRESSION | The compression type to use on OTLP trace, metric, and log requests. Options include gzip. By default no compression will be used. | + > The per-signal environment variables (`OTEL_EXPORTER_OTLP_TRACES_TIMEOUT`) takes precedence and non-per-signal environment variable (`OTEL_EXPORTER_OTLP_TIMEOUT`). ## Running opentelemetry-collector locally to see the traces @@ -141,9 +162,9 @@ OTEL_EXPORTER_OTLP_TRACES_COMPRESSION=gzip ## Useful links -- For more information on OpenTelemetry, visit: -- For more about OpenTelemetry JavaScript: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ++ For more information on OpenTelemetry, visit: ++ For more about OpenTelemetry JavaScript: ++ For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ## License diff --git a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts index 5eb2fc585d..03e8be4c6e 100644 --- a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts @@ -34,6 +34,7 @@ import { ensureResourceIsCorrect, mockedReadableSpan, } from './traceHelper'; +import * as core from '@opentelemetry/core'; import { CompressionAlgorithm } from '@opentelemetry/otlp-exporter-base'; import { GrpcCompressionAlgorithm } from '@opentelemetry/otlp-grpc-exporter-base'; import { IExportTraceServiceRequest, IResourceSpans } from '@opentelemetry/otlp-transformer'; @@ -197,7 +198,34 @@ const testCollectorExporter = (params: TestParams) => ensureMetadataIsCorrect(reqMetadata, params?.metadata); done(); - }, 200); + }, 500); + }); + it('should log deadline exceeded error', done => { + const credentials = params.useTLS + ? grpc.credentials.createSsl( + fs.readFileSync('./test/certs/ca.crt'), + fs.readFileSync('./test/certs/client.key'), + fs.readFileSync('./test/certs/client.crt') + ) + : undefined; + + const collectorExporterWithTimeout = new OTLPTraceExporter({ + url: 'grpcs://' + address, + credentials, + metadata: params.metadata, + timeoutMillis: 100, + }); + + const responseSpy = sinon.spy(); + const spans = [Object.assign({}, mockedReadableSpan)]; + collectorExporterWithTimeout.export(spans, responseSpy); + + setTimeout(() => { + const result = responseSpy.args[0][0] as core.ExportResult; + assert.strictEqual(result.code, core.ExportResultCode.FAILED); + assert.strictEqual(responseSpy.args[0][0].error.details, 'Deadline exceeded'); + done(); + }, 300); }); }); describe('export - with gzip compression', () => { diff --git a/experimental/packages/exporter-trace-otlp-http/README.md b/experimental/packages/exporter-trace-otlp-http/README.md index 8621e73291..64d422924d 100644 --- a/experimental/packages/exporter-trace-otlp-http/README.md +++ b/experimental/packages/exporter-trace-otlp-http/README.md @@ -107,6 +107,38 @@ OTEL_EXPORTER_OTLP_METRICS_ENDPOINT=https://metric-service:4318/v1/metrics For more details, see [OpenTelemetry Specification on Protocol Exporter][opentelemetry-spec-protocol-exporter]. +## Exporter Timeout Configuration + +The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. + +To override the default timeout duration, use the following options: + ++ Set with environment variables: + + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + + > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + ++ Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + + ```js + const collectorOptions = { + timeoutMillis: 15000, + url: '', // url is optional and can be omitted - default is http://localhost:4318/v1/traces + headers: { + foo: 'bar' + }, // an optional object containing custom headers to be sent with each request will only work with http + concurrencyLimit: 10, // an optional limit on pending requests + }; + + const exporter = new OTLPTraceExporter(collectorOptions); + ``` + + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. + ## Running opentelemetry-collector locally to see the traces 1. Go to `examples/otlp-exporter-node` @@ -114,9 +146,9 @@ For more details, see [OpenTelemetry Specification on Protocol Exporter][opentel ## Useful links -- For more information on OpenTelemetry, visit: -- For more about OpenTelemetry JavaScript: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ++ For more information on OpenTelemetry, visit: ++ For more about OpenTelemetry JavaScript: ++ For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ## License 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 b6835b0151..7970005c08 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 @@ -14,13 +14,13 @@ * limitations under the License. */ +import * as core from '@opentelemetry/core'; import { diag, DiagLogger, DiagLogLevel } from '@opentelemetry/api'; import { ExportResultCode } from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { OTLPTraceExporter } from '../../src/platform/browser/index'; - import { ensureSpanIsCorrect, ensureExportTraceServiceRequestIsSet, @@ -28,7 +28,7 @@ import { ensureHeadersContain, mockedReadableSpan, } from '../traceHelper'; -import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base'; +import { OTLPExporterConfigBase, OTLPExporterError } from '@opentelemetry/otlp-exporter-base'; import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; describe('OTLPTraceExporter - web', () => { @@ -176,7 +176,12 @@ describe('OTLPTraceExporter - web', () => { describe('when "sendBeacon" is NOT available', () => { let server: any; + let clock: sinon.SinonFakeTimers; beforeEach(() => { + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + (window.navigator as any).sendBeacon = false; collectorTraceExporter = new OTLPTraceExporter( collectorExporterConfig @@ -191,7 +196,7 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + queueMicrotask(() => { const request = server.requests[0]; assert.strictEqual(request.method, 'POST'); assert.strictEqual(request.url, 'http://foo.bar.com'); @@ -211,9 +216,9 @@ describe('OTLPTraceExporter - web', () => { ensureWebResourceIsCorrect(resource); assert.strictEqual(stubBeacon.callCount, 0); - ensureExportTraceServiceRequestIsSet(json); + clock.restore(); done(); }); }); @@ -236,15 +241,15 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + queueMicrotask(() => { const request = server.requests[0]; request.respond(200); - const response: any = spyLoggerDebug.args[2][0]; assert.strictEqual(response, 'xhr success'); assert.strictEqual(spyLoggerError.args.length, 0); - assert.strictEqual(stubBeacon.callCount, 0); + + clock.restore(); done(); }); }); @@ -256,9 +261,11 @@ describe('OTLPTraceExporter - web', () => { done(); }); - setTimeout(() => { + queueMicrotask(() => { const request = server.requests[0]; request.respond(400); + clock.restore(); + done(); }); }); @@ -266,11 +273,12 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + queueMicrotask(() => { const request = server.requests[0]; request.respond(200); assert.strictEqual(stubBeacon.callCount, 0); + clock.restore(); done(); }); }); @@ -373,7 +381,12 @@ describe('OTLPTraceExporter - web', () => { }); describe('when "sendBeacon" is available', () => { + let clock: sinon.SinonFakeTimers; beforeEach(() => { + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + collectorTraceExporter = new OTLPTraceExporter( collectorExporterConfig ); @@ -382,20 +395,26 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + queueMicrotask(() => { const [{ requestHeaders }] = server.requests; ensureHeadersContain(requestHeaders, customHeaders); assert.strictEqual(stubBeacon.callCount, 0); assert.strictEqual(stubOpen.callCount, 0); + clock.restore(); done(); }); }); }); describe('when "sendBeacon" is NOT available', () => { + let clock: sinon.SinonFakeTimers; beforeEach(() => { + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + (window.navigator as any).sendBeacon = false; collectorTraceExporter = new OTLPTraceExporter( collectorExporterConfig @@ -406,13 +425,30 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => { }); - setTimeout(() => { + queueMicrotask(() => { const [{ requestHeaders }] = server.requests; ensureHeadersContain(requestHeaders, customHeaders); assert.strictEqual(stubBeacon.callCount, 0); assert.strictEqual(stubOpen.callCount, 0); + clock.restore(); + done(); + }); + }); + it('should log the timeout request error message', done => { + const responseSpy = sinon.spy(); + collectorTraceExporter.export(spans, responseSpy); + clock.tick(10000); + clock.restore(); + + 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.message, 'Request Timeout'); + 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 bfe9d4ccb4..3e9da9370e 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 @@ -35,6 +35,7 @@ import { ensureSpanIsCorrect, mockedReadableSpan } from '../traceHelper'; +import { nextTick } from 'process'; import { MockedResponse } from './nodeHelpers'; import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; @@ -163,7 +164,11 @@ 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'); const options = args[0]; assert.strictEqual(options.hostname, 'foo.bar.com'); @@ -177,7 +182,12 @@ 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'); + const options = args[0]; assert.strictEqual(options.headers['foo'], 'bar'); done(); @@ -188,7 +198,12 @@ 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'); + const options = args[0]; assert.strictEqual(options.headers['Content-Encoding'], undefined); done(); @@ -199,7 +214,12 @@ 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'); + const options = args[0]; const agent = options.agent; assert.strictEqual(agent.keepAlive, true); @@ -209,14 +229,34 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('different http export requests should use the same agent', done => { - collectorExporter.export(spans, () => { }); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => { }); - setTimeout(() => { + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); + clock.restore(); + + nextTick(() => { + const clock = sinon.useFakeTimers(); + collectorExporter.export(spans, () => { }); + + const mockRes2 = new MockedResponse(200); + const args2 = stubRequest.args[1]; + const callback2 = args2[1]; + + callback2(mockRes); + mockRes2.send('success'); + const [firstExportAgent, secondExportAgent] = stubRequest.args.map( a => a[0].agent ); + assert.strictEqual(firstExportAgent, secondExportAgent); + clock.restore(); done(); }); }); @@ -242,6 +282,13 @@ describe('OTLPTraceExporter - node with json over http', () => { }); collectorExporter.export(spans, () => { }); + + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); }); it('should log the successful message', done => { @@ -254,6 +301,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; const callback = args[1]; + callback(mockRes); mockRes.send('success'); setTimeout(() => { @@ -277,6 +325,7 @@ describe('OTLPTraceExporter - node with json over http', () => { const callback = args[1]; callback(mockResError); mockResError.send('failed'); + setTimeout(() => { const result = responseSpy.args[0][0] as core.ExportResult; assert.strictEqual(result.code, core.ExportResultCode.FAILED); @@ -312,7 +361,6 @@ describe('OTLPTraceExporter - node with json over http', () => { }); it('should successfully send the spans', done => { - collectorExporter.export(spans, () => { }); let buff = Buffer.from(''); fakeRequest.on('end', () => { @@ -325,15 +373,22 @@ describe('OTLPTraceExporter - node with json over http', () => { ensureExportTraceServiceRequestIsSet(json); assert.ok(spySetHeader.calledWith('Content-Encoding', 'gzip')); - done(); }); fakeRequest.on('data', chunk => { buff = Buffer.concat([buff, chunk]); }); - }); + collectorExporter.export(spans,() => { }); + + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); + }); }); describe('OTLPTraceExporter - node (getDefaultUrl)', () => { @@ -357,4 +412,137 @@ describe('OTLPTraceExporter - node with json over http', () => { }); }); }); + describe('export - with timeout', () => { + beforeEach(() => { + fakeRequest = new Stream.PassThrough(); + stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); + spySetHeader = sinon.spy(); + (fakeRequest as any).setHeader = spySetHeader; + (fakeRequest as any).abort = sinon.spy(); + collectorExporterConfig = { + headers: { + foo: 'bar', + }, + hostname: 'foo', + attributes: {}, + url: 'http://foo.bar.com', + keepAlive: true, + httpAgentOptions: { keepAliveMsecs: 2000 }, + timeoutMillis: 100, + }; + collectorExporter = new OTLPTraceExporter(collectorExporterConfig); + spans = []; + spans.push(Object.assign({}, mockedReadableSpan)); + }); + it('should log the timeout request error message', done => { + const responseSpy = sinon.spy(); + collectorExporter.export(spans, responseSpy); + + setTimeout(() => { + fakeRequest.emit('error', { code: 'ECONNRESET'}); + + 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.message, 'Request Timeout'); + + done(); + }); + }, 300); + }); + }); +}); + +describe('export - real http request destroyed before response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + + const server = http.createServer((_, res) => { + setTimeout(() => { + res.statusCode = 200; + res.end(); + }, 200); + }); + before(done => { + server.listen(8081, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message when timeout is 1', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 1, + }; + 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); + }); + it('should log the timeout request error message when timeout is 100', done => { + collectorExporterConfig = { + url: 'http://localhost:8081', + timeoutMillis: 100, + }; + 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 - 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); + }); }); diff --git a/experimental/packages/exporter-trace-otlp-proto/README.md b/experimental/packages/exporter-trace-otlp-proto/README.md index 5e5d2ed6e8..8fd0765c4a 100644 --- a/experimental/packages/exporter-trace-otlp-proto/README.md +++ b/experimental/packages/exporter-trace-otlp-proto/README.md @@ -39,6 +39,37 @@ provider.register(); ``` +## Exporter Timeout Configuration + +The OTLPTraceExporter has a timeout configuration option which is the maximum time, in milliseconds, the OTLP exporter will wait for each batch export. The default value is 10000ms. + +To override the default timeout duration, use the following options: + ++ Set with environment variables: + + | Environment variable | Description | + |----------------------|-------------| + | OTEL_EXPORTER_OTLP_TRACES_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace batch. Default is 10000. | + | OTEL_EXPORTER_OTLP_TIMEOUT | The maximum waiting time, in milliseconds, allowed to send each OTLP trace and metric batch. Default is 10000. | + + > `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` takes precedence and overrides `OTEL_EXPORTER_OTLP_TIMEOUT`. + ++ Provide `timeoutMillis` to OTLPTraceExporter with `collectorOptions`: + + ```js + const collectorOptions = { + timeoutMillis: 15000, + url: '', // url is optional and can be omitted - default is http://localhost:4318/v1/traces + headers: { + foo: 'bar' + }, //an optional object containing custom headers to be sent with each request will only work with http + }; + + const exporter = new OTLPTraceExporter(collectorOptions); + ``` + + > Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables. + ## Running opentelemetry-collector locally to see the traces 1. Go to examples/otlp-exporter-node @@ -47,9 +78,9 @@ provider.register(); ## Useful links -- For more information on OpenTelemetry, visit: -- For more about OpenTelemetry JavaScript: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ++ For more information on OpenTelemetry, visit: ++ For more about OpenTelemetry JavaScript: ++ For help or feedback on this project, join us in [GitHub Discussions][discussions-url] ## License diff --git a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts index 56f6aba69c..e68a02e762 100644 --- a/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-proto/test/OTLPTraceExporter.test.ts @@ -20,7 +20,7 @@ import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import * as assert from 'assert'; import * as http from 'http'; import * as sinon from 'sinon'; -import { Stream } from 'stream'; +import { Stream, PassThrough } from 'stream'; import * as zlib from 'zlib'; import { OTLPTraceExporter } from '../src'; import { @@ -29,21 +29,22 @@ import { mockedReadableSpan, MockedResponse, } from './traceHelper'; -import { CompressionAlgorithm, OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; +import { CompressionAlgorithm, OTLPExporterNodeConfigBase, OTLPExporterError } from '@opentelemetry/otlp-exporter-base'; import { getExportRequestProto } from '@opentelemetry/otlp-proto-exporter-base'; import { IExportTraceServiceRequest } from '@opentelemetry/otlp-transformer'; -const fakeRequest = { - end: function () { }, - on: function () { }, - write: function () { }, -}; +let fakeRequest: PassThrough; describe('OTLPTraceExporter - node with proto over http', () => { let collectorExporter: OTLPTraceExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase; let spans: ReadableSpan[]; + afterEach(() => { + fakeRequest = new Stream.PassThrough(); + sinon.restore(); + }); + describe('when configuring via environment', () => { const envSource = process.env; it('should use url defined in env', () => { @@ -115,10 +116,14 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should open the connection', done => { collectorExporter.export(spans, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + 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(); return fakeRequest as any; }); @@ -127,8 +132,12 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should set custom headers', done => { collectorExporter.export(spans, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + 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(); return fakeRequest as any; }); @@ -137,9 +146,13 @@ describe('OTLPTraceExporter - node with proto over http', () => { it('should have keep alive and keepAliveMsecs option set', done => { collectorExporter.export(spans, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + 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(); return fakeRequest as any; }); @@ -167,7 +180,10 @@ describe('OTLPTraceExporter - node with proto over http', () => { buff = Buffer.concat([buff, chunk]); }); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => { }); + clock.tick(200); + clock.restore(); }); it('should log the successful message', done => { @@ -252,7 +268,96 @@ describe('OTLPTraceExporter - node with proto over http', () => { buff = Buffer.concat([buff, chunk]); }); + const clock = sinon.useFakeTimers(); collectorExporter.export(spans, () => { }); + clock.tick(200); + clock.restore(); + }); + }); +}); + + +describe('export - real http request destroyed before response received', () => { + let collectorExporter: OTLPTraceExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let spans: ReadableSpan[]; + const server = http.createServer((_, res) => { + setTimeout(() => { + res.statusCode = 200; + res.end(); + }, 200); + }); + before(done => { + server.listen(8080, done); + }); + 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', + timeoutMillis: 100, + }; + 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(); + }); + }); +}); + +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(8080, done); + }); + after(done => { + server.close(done); + }); + it('should log the timeout request error message', done => { + collectorExporterConfig = { + url: 'http://localhost:8080', + timeoutMillis: 300, + }; + 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(); }); }); }); 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 0cd82e3178..9d730d5a2e 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 @@ -39,17 +39,11 @@ import { } from '../metricsHelper'; import { MockedResponse } from './nodeHelpers'; import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; +import { Stream, PassThrough } from 'stream'; import { OTLPExporterError, OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; -const fakeRequest = { - end: function () { - }, - on: function () { - }, - write: function () { - }, -}; +let fakeRequest: PassThrough; const address = 'localhost:1501'; @@ -57,7 +51,6 @@ describe('OTLPMetricExporter - node with json over http', () => { let collectorExporter: OTLPMetricExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions; let stubRequest: sinon.SinonStub; - let stubWrite: sinon.SinonStub; let metrics: ResourceMetrics; beforeEach(async () => { @@ -65,6 +58,7 @@ describe('OTLPMetricExporter - node with json over http', () => { }); afterEach(async () => { + fakeRequest = new Stream.PassThrough(); await shutdown(); sinon.restore(); }); @@ -153,7 +147,6 @@ describe('OTLPMetricExporter - node with json over http', () => { describe('export', () => { beforeEach(async () => { stubRequest = sinon.stub(http, 'request').returns(fakeRequest as any); - stubWrite = sinon.stub(fakeRequest, 'end'); collectorExporterConfig = { headers: { foo: 'bar', @@ -188,7 +181,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; assert.strictEqual(options.hostname, 'foo.bar.com'); @@ -203,7 +200,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; assert.strictEqual(options.headers['foo'], 'bar'); done(); @@ -215,7 +216,11 @@ describe('OTLPMetricExporter - node with json over http', () => { }); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = stubRequest.args[0]; + const callback = args[1]; + callback(mockRes); + mockRes.send('success'); const options = args[0]; const agent = options.agent; assert.strictEqual(agent.keepAlive, true); @@ -225,12 +230,15 @@ describe('OTLPMetricExporter - node with json over http', () => { }); it('should successfully send metrics', done => { + let buff = Buffer.from(''); + collectorExporter.export(metrics, () => { }); - setTimeout(() => { - const writeArgs = stubWrite.args[0]; - const json = JSON.parse(writeArgs[0]) as IExportMetricsServiceRequest; + fakeRequest.on('end', () => { + const responseBody = buff.toString(); + + const json = JSON.parse(responseBody) as IExportMetricsServiceRequest; const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; @@ -262,6 +270,17 @@ describe('OTLPMetricExporter - node with json over http', () => { done(); }); + + fakeRequest.on('data', chunk => { + buff = Buffer.concat([buff, chunk]); + }); + + const mockRes = new MockedResponse(200); + const args = stubRequest.args[0]; + const callback = args[1]; + + callback(mockRes); + mockRes.send('success'); }); it('should log the successful message', done => { 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 6f8b03e00b..8aec57134e 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 @@ -34,23 +34,22 @@ import { } from './metricsHelper'; import { AggregationTemporality, ResourceMetrics } from '@opentelemetry/sdk-metrics-base'; import { OTLPMetricExporterOptions } from '@opentelemetry/exporter-metrics-otlp-http'; +import { Stream, PassThrough } from 'stream'; import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { IExportMetricsServiceRequest } from '@opentelemetry/otlp-transformer'; -const fakeRequest = { - end: function () { - }, - on: function () { - }, - write: function () { - }, -}; +let fakeRequest: PassThrough; describe('OTLPMetricExporter - node with proto over http', () => { let collectorExporter: OTLPMetricExporter; let collectorExporterConfig: OTLPExporterNodeConfigBase & OTLPMetricExporterOptions; let metrics: ResourceMetrics; + afterEach(() => { + fakeRequest = new Stream.PassThrough(); + sinon.restore(); + }); + describe('when configuring via environment', () => { const envSource = process.env; it('should use url defined in env', () => { @@ -138,10 +137,14 @@ describe('OTLPMetricExporter - node with proto over http', () => { collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + 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(); return fakeRequest as any; }); @@ -151,8 +154,13 @@ describe('OTLPMetricExporter - node with proto over http', () => { collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + 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(); return fakeRequest as any; }); @@ -162,59 +170,70 @@ describe('OTLPMetricExporter - node with proto over http', () => { collectorExporter.export(metrics, () => { }); - sinon.stub(http, 'request').callsFake((options: any) => { + 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(); return fakeRequest as any; }); }); it('should successfully send metrics', done => { - sinon.stub(http, 'request').returns({ - write: () => {}, - on: () => {}, - end: (...writeArgs: any[]) => { - const ExportTraceServiceRequestProto = getExportRequestProto(); - const data = ExportTraceServiceRequestProto?.decode(writeArgs[0]); - const json = data?.toJSON() as IExportMetricsServiceRequest; - - const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; - const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; - const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; - - assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); - ensureExportedCounterIsCorrect( - metric1, - metric1.sum?.dataPoints[0].timeUnixNano, - metric1.sum?.dataPoints[0].startTimeUnixNano - ); - assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); - ensureExportedObservableGaugeIsCorrect( - metric2, - metric2.gauge?.dataPoints[0].timeUnixNano, - metric2.gauge?.dataPoints[0].startTimeUnixNano - ); - assert.ok( - typeof metric3 !== 'undefined', - "value recorder doesn't exist" - ); - ensureExportedHistogramIsCorrect( - metric3, - metric3.histogram?.dataPoints[0].timeUnixNano, - metric3.histogram?.dataPoints[0].startTimeUnixNano, - [0, 100], - ['0', '2', '0'] - ); - - ensureExportMetricsServiceRequestIsSet(json); - done(); - }, - } as any); + const fakeRequest = new Stream.PassThrough(); + sinon.stub(http, 'request').returns(fakeRequest as any); - collectorExporter.export(metrics, result => { - done(result.error); + let buff = Buffer.from(''); + + fakeRequest.on('end', () => { + const ExportTraceServiceRequestProto = getExportRequestProto(); + const data = ExportTraceServiceRequestProto?.decode(buff); + const json = data?.toJSON() as IExportMetricsServiceRequest; + + const metric1 = json.resourceMetrics[0].scopeMetrics[0].metrics[0]; + const metric2 = json.resourceMetrics[0].scopeMetrics[0].metrics[1]; + const metric3 = json.resourceMetrics[0].scopeMetrics[0].metrics[2]; + + assert.ok(typeof metric1 !== 'undefined', "counter doesn't exist"); + ensureExportedCounterIsCorrect( + metric1, + metric1.sum?.dataPoints[0].timeUnixNano, + metric1.sum?.dataPoints[0].startTimeUnixNano + ); + assert.ok(typeof metric2 !== 'undefined', "observable gauge doesn't exist"); + ensureExportedObservableGaugeIsCorrect( + metric2, + metric2.gauge?.dataPoints[0].timeUnixNano, + metric2.gauge?.dataPoints[0].startTimeUnixNano + ); + assert.ok( + typeof metric3 !== 'undefined', + "value recorder doesn't exist" + ); + ensureExportedHistogramIsCorrect( + metric3, + metric3.histogram?.dataPoints[0].timeUnixNano, + metric3.histogram?.dataPoints[0].startTimeUnixNano, + [0, 100], + ['0', '2', '0'] + ); + + ensureExportMetricsServiceRequestIsSet(json); + done(); }); + + fakeRequest.on('data', chunk => { + buff = Buffer.concat([buff, chunk]); + }); + + const clock = sinon.useFakeTimers(); + collectorExporter.export(metrics, () => { }); + clock.tick(200); + clock.restore(); }); it('should log the successful message', done => { diff --git a/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts b/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts index 421ceae2e9..5cc2ae5338 100644 --- a/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts +++ b/experimental/packages/otlp-exporter-base/src/OTLPExporterBase.ts @@ -21,6 +21,7 @@ import { OTLPExporterConfigBase, ExportServiceError, } from './types'; +import { configureExporterTimeout } from './util'; /** * Collector Exporter abstract base class @@ -33,6 +34,7 @@ export abstract class OTLPExporterBase< public readonly url: string; public readonly hostname: string | undefined; public readonly attributes?: SpanAttributes; + public readonly timeoutMillis: number; protected _concurrencyLimit: number; protected _sendingPromises: Promise[] = []; protected _shutdownOnce: BindOnceFuture; @@ -56,6 +58,8 @@ export abstract class OTLPExporterBase< ? config.concurrencyLimit : Infinity; + this.timeoutMillis = configureExporterTimeout(config.timeoutMillis); + // platform dependent this.onInit(config); } diff --git a/experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts b/experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts index 708702300b..46948167b2 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts @@ -78,7 +78,7 @@ export abstract class OTLPExporterBrowserBase< const promise = new Promise((resolve, reject) => { if (this._useXHR) { - sendWithXhr(body, this.url, this._headers, resolve, reject); + sendWithXhr(body, this.url, this._headers, this.timeoutMillis, resolve, reject); } else { sendWithBeacon(body, this.url, { type: 'application/json' }, resolve, reject); } 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 af47f4bebf..4844037056 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/browser/util.ts @@ -55,9 +55,17 @@ export function sendWithXhr( body: string, url: string, headers: Record, + exporterTimeout: number, onSuccess: () => void, onError: (error: OTLPExporterError) => void ): void { + let reqIsDestroyed: boolean; + + const exporterTimer = setTimeout(() => { + reqIsDestroyed = true; + xhr.abort(); + }, exporterTimeout); + const xhr = new XMLHttpRequest(); xhr.open('POST', url); @@ -78,13 +86,20 @@ export function sendWithXhr( 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 + ); + onError(error); } else { const error = new OTLPExporterError( `Failed to export with XHR (status: ${xhr.status})`, xhr.status ); + clearTimeout(exporterTimer); onError(error); } } 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 392063cea7..f5b391866e 100644 --- a/experimental/packages/otlp-exporter-base/src/platform/node/util.ts +++ b/experimental/packages/otlp-exporter-base/src/platform/node/util.ts @@ -42,7 +42,20 @@ export function sendWithHttp( onSuccess: () => void, onError: (error: OTLPExporterError) => void ): void { + const exporterTimeout = collector.timeoutMillis; const parsedUrl = new url.URL(collector.url); + let reqIsDestroyed: boolean; + const nodeVersion = Number(process.versions.node.split('.')[0]); + + const exporterTimer = setTimeout(() => { + reqIsDestroyed = true; + // req.abort() was deprecated since v14 + if (nodeVersion >= 14) { + req.destroy(); + } else { + req.abort(); + } + }, exporterTimeout); const options: http.RequestOptions | https.RequestOptions = { hostname: parsedUrl.hostname, @@ -61,24 +74,44 @@ export function sendWithHttp( const req = request(options, (res: http.IncomingMessage) => { let responseData = ''; res.on('data', chunk => (responseData += chunk)); - res.on('end', () => { - if (res.statusCode && res.statusCode < 299) { - diag.debug(`statusCode: ${res.statusCode}`, responseData); - onSuccess(); - } else { - const error = new OTLPExporterError( - res.statusMessage, - res.statusCode, - responseData + + res.on('aborted', () => { + if (reqIsDestroyed) { + const err = new OTLPExporterError( + 'Request Timeout' ); - onError(error); + onError(err); } }); - }); + 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); + } + }); + }); - req.on('error', (error: Error) => { - onError(error); + req.on('error', (error: Error | any) => { + if (reqIsDestroyed) { + const err = new OTLPExporterError( + 'Request Timeout', error.code + ); + onError(err); + } else { + clearTimeout(exporterTimer); + onError(error); + } }); switch (collector.compression) { diff --git a/experimental/packages/otlp-exporter-base/src/types.ts b/experimental/packages/otlp-exporter-base/src/types.ts index ffe91fda80..5233f940cd 100644 --- a/experimental/packages/otlp-exporter-base/src/types.ts +++ b/experimental/packages/otlp-exporter-base/src/types.ts @@ -52,4 +52,7 @@ export interface OTLPExporterConfigBase { attributes?: SpanAttributes; url?: string; concurrencyLimit?: number; + /** Maximum time the OTLP exporter will wait for each batch export. + * The default value is 10000ms. */ + timeoutMillis?: number; } diff --git a/experimental/packages/otlp-exporter-base/src/util.ts b/experimental/packages/otlp-exporter-base/src/util.ts index b018000110..47629d9a8e 100644 --- a/experimental/packages/otlp-exporter-base/src/util.ts +++ b/experimental/packages/otlp-exporter-base/src/util.ts @@ -15,6 +15,9 @@ */ import { diag } from '@opentelemetry/api'; +import { getEnv } from '@opentelemetry/core'; + +const DEFAULT_TRACE_TIMEOUT = 10000; /** * Parses headers from config leaving only those that have defined values @@ -39,3 +42,41 @@ export function appendResourcePathToUrlIfNotPresent(url: string, path: string): return url + path; } + +/** + * Configure exporter trace timeout value from passed in value or environment variables + * @param timeoutMillis + * @returns timeout value in milliseconds + */ + +export function configureExporterTimeout(timeoutMillis: number | undefined): number { + if (typeof timeoutMillis === 'number') { + if (timeoutMillis <= 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return invalidTimeout(timeoutMillis, DEFAULT_TRACE_TIMEOUT); + } + return timeoutMillis; + } else { + return getExporterTimeoutFromEnv(); + } +} + +function getExporterTimeoutFromEnv(): number { + const definedTimeout = + Number(getEnv().OTEL_EXPORTER_OTLP_TRACES_TIMEOUT ?? + getEnv().OTEL_EXPORTER_OTLP_TIMEOUT); + + if (definedTimeout <= 0) { + // OTLP exporter configured timeout - using default value of 10000ms + return invalidTimeout(definedTimeout, DEFAULT_TRACE_TIMEOUT); + } else { + return definedTimeout; + } +} + +// OTLP exporter configured timeout - using default value of 10000ms +export function invalidTimeout(timeout: number, defaultTimeout: number): number { + diag.warn('Timeout must be greater than 0', timeout); + + return defaultTimeout; +} diff --git a/experimental/packages/otlp-exporter-base/test/browser/util.test.ts b/experimental/packages/otlp-exporter-base/test/browser/util.test.ts index 106e04b4da..e72b8618f5 100644 --- a/experimental/packages/otlp-exporter-base/test/browser/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/browser/util.test.ts @@ -16,6 +16,7 @@ import * as sinon from 'sinon'; import { sendWithXhr } from '../../src/platform/browser/util'; +import { nextTick } from 'process'; import { ensureHeadersContain } from '../testHelper'; describe('util - browser', () => { @@ -39,7 +40,12 @@ describe('util - browser', () => { describe('when XMLHTTPRequest is used', () => { let expectedHeaders: Record; + let clock: sinon.SinonFakeTimers; beforeEach(()=>{ + // fakeTimers is used to replace the next setTimeout which is + // located in sendWithXhr function called by the export method + clock = sinon.useFakeTimers(); + expectedHeaders = { // ;charset=utf-8 is applied by sinon.fakeServer 'Content-Type': 'application/json;charset=utf-8', @@ -51,21 +57,24 @@ describe('util - browser', () => { const explicitContentType = { 'Content-Type': 'application/json', }; - sendWithXhr(body, url, explicitContentType, onSuccessStub, onErrorStub); + const exporterTimeout = 10000; + sendWithXhr(body, url, explicitContentType, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain "Accept" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); @@ -74,21 +83,25 @@ describe('util - browser', () => { describe('and empty headers are set', () => { beforeEach(()=>{ const emptyHeaders = {}; - sendWithXhr(body, url, emptyHeaders, onSuccessStub, onErrorStub); + // use default exporter timeout + const exporterTimeout = 10000; + sendWithXhr(body, url, emptyHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain "Accept" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); @@ -97,29 +110,33 @@ describe('util - browser', () => { let customHeaders: Record; beforeEach(()=>{ customHeaders = { aHeader: 'aValue', bHeader: 'bValue' }; - sendWithXhr(body, url, customHeaders, onSuccessStub, onErrorStub); + const exporterTimeout = 10000; + sendWithXhr(body, url, customHeaders, exporterTimeout, onSuccessStub, onErrorStub); }); it('Request Headers should contain "Content-Type" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain "Accept" header', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, expectedHeaders); + clock.restore(); done(); }); }); it('Request Headers should contain custom headers', done => { - setTimeout(() => { + nextTick(() => { const { requestHeaders } = server.requests[0]; ensureHeadersContain(requestHeaders, customHeaders); + clock.restore(); done(); }); }); 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 c920f4e874..b50e1329ca 100644 --- a/experimental/packages/otlp-exporter-base/test/node/util.test.ts +++ b/experimental/packages/otlp-exporter-base/test/node/util.test.ts @@ -15,8 +15,94 @@ */ import * as assert from 'assert'; +import { configureExporterTimeout, invalidTimeout } from '../../src/util'; import { CompressionAlgorithm} from '../../src/platform/node/types'; import { configureCompression} from '../../src/platform/node/util'; +import { diag } from '@opentelemetry/api'; +import * as sinon from 'sinon'; + +describe('configureExporterTimeout', () => { + const envSource = process.env; + it('should use timeoutMillis parameter as export timeout value', () => { + const exporterTimeout = configureExporterTimeout(9000); + assert.strictEqual(exporterTimeout, 9000); + }); + it('should use default trace export timeout env variable value when timeoutMillis parameter is undefined', () => { + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use default trace export timeout env variable value when timeoutMillis parameter is negative', () => { + const exporterTimeout = configureExporterTimeout(-18000); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use trace export timeout value defined in env', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '15000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 15000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use default trace export timeout env variable value when trace export timeout value defined in env is negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-15000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use default trace export timeout when timeoutMillis parameter is negative', () => { + const exporterTimeout = configureExporterTimeout(-15000); + assert.strictEqual(exporterTimeout, 10000); + }); + it('should use timeoutMillis parameter over trace export timeout value defined in env', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '11000'; + const exporterTimeout = configureExporterTimeout(9000); + assert.strictEqual(exporterTimeout, 9000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + }); + it('should use default value when both timeoutMillis parameter and export timeout values defined in env are negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-11000'; + envSource.OTEL_EXPORTER_OTLP_TIMEOUT = '-9000'; + const exporterTimeout = configureExporterTimeout(-5000); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + delete envSource.OTEL_EXPORTER_OTLP_TIMEOUT; + }); + it('should use default value export timeout value defined in env are negative', () => { + envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT = '-11000'; + envSource.OTEL_EXPORTER_OTLP_TIMEOUT = '-9000'; + const exporterTimeout = configureExporterTimeout(undefined); + assert.strictEqual(exporterTimeout, 10000); + delete envSource.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT; + delete envSource.OTEL_EXPORTER_OTLP_TIMEOUT; + }); + it('should warn user about invalid timeout', () => { + const spyLoggerWarn = sinon.stub(diag, 'warn'); + configureExporterTimeout(-15000); + const args = spyLoggerWarn.args[0]; + assert.strictEqual(args[0], 'Timeout must be greater than 0'); + assert.strictEqual(args[1], -15000); + sinon.restore(); + }); +}); + +describe('invalidTimeout', () => { + it('should warn user about invalid timeout', () => { + const spyLoggerWarn = sinon.stub(diag, 'warn'); + invalidTimeout(-9000, 10000); + const args = spyLoggerWarn.args[0]; + assert.strictEqual(args[0], 'Timeout must be greater than 0'); + assert.strictEqual(args[1], -9000); + sinon.restore(); + }); + it('diag warn was called', () => { + const spyLoggerWarn = sinon.stub(diag, 'warn'); + invalidTimeout(-9000, 10000); + assert(spyLoggerWarn.calledOnce); + sinon.restore(); + }); + it('should return default timeout', () => { + const defaultTimeout = invalidTimeout(-9000, 10000); + assert.strictEqual(defaultTimeout, 10000); + }); +}); describe('configureCompression', () => { const envSource = process.env; diff --git a/experimental/packages/otlp-grpc-exporter-base/src/types.ts b/experimental/packages/otlp-grpc-exporter-base/src/types.ts index b015a4aef2..74c1e7ccf0 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/types.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/types.ts @@ -34,6 +34,7 @@ export interface ServiceClient extends grpc.Client { export: ( request: any, metadata: grpc.Metadata, + options: grpc.CallOptions, callback: Function ) => {}; } diff --git a/experimental/packages/otlp-grpc-exporter-base/src/util.ts b/experimental/packages/otlp-grpc-exporter-base/src/util.ts index 7165f77d5a..ad25335a79 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/util.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/util.ts @@ -84,10 +84,12 @@ export function send( ): void { if (collector.serviceClient) { const serviceRequest = collector.convert(objects); + const deadline = Date.now() + collector.timeoutMillis; collector.serviceClient.export( serviceRequest, collector.metadata || new grpc.Metadata(), + {deadline: deadline}, (err: ExportServiceError) => { if (err) { diag.error('Service request', serviceRequest); @@ -134,7 +136,6 @@ function toGrpcCompression(compression: CompressionAlgorithm): GrpcCompressionAl return GrpcCompressionAlgorithm.NONE; } - /** * These values are defined by grpc client */ diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index b43c85e6a5..bb3b123ffe 100644 --- a/packages/opentelemetry-core/src/utils/environment.ts +++ b/packages/opentelemetry-core/src/utils/environment.ts @@ -34,6 +34,9 @@ const ENVIRONMENT_NUMBERS_KEYS = [ 'OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT', 'OTEL_SPAN_EVENT_COUNT_LIMIT', 'OTEL_SPAN_LINK_COUNT_LIMIT', + 'OTEL_EXPORTER_OTLP_TIMEOUT', + 'OTEL_EXPORTER_OTLP_TRACES_TIMEOUT', + 'OTEL_EXPORTER_OTLP_METRICS_TIMEOUT', 'OTEL_EXPORTER_JAEGER_AGENT_PORT', ] as const; @@ -123,6 +126,9 @@ export const DEFAULT_ENVIRONMENT: Required = { OTEL_EXPORTER_OTLP_HEADERS: '', OTEL_EXPORTER_OTLP_TRACES_HEADERS: '', OTEL_EXPORTER_OTLP_METRICS_HEADERS: '', + OTEL_EXPORTER_OTLP_TIMEOUT: 10000, + OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 10000, + OTEL_EXPORTER_OTLP_METRICS_TIMEOUT: 10000, OTEL_EXPORTER_ZIPKIN_ENDPOINT: 'http://localhost:9411/api/v2/spans', OTEL_LOG_LEVEL: DiagLogLevel.INFO, OTEL_NO_PATCH_MODULES: [], diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts index e336eeeca5..ce18b42476 100644 --- a/packages/opentelemetry-core/test/utils/environment.test.ts +++ b/packages/opentelemetry-core/test/utils/environment.test.ts @@ -93,6 +93,8 @@ describe('environment', () => { OTEL_SPAN_LINK_COUNT_LIMIT: 30, OTEL_TRACES_SAMPLER: 'always_on', OTEL_TRACES_SAMPLER_ARG: '0.5', + OTEL_EXPORTER_OTLP_TIMEOUT: 15000, + OTEL_EXPORTER_OTLP_TRACES_TIMEOUT: 12000, }); const env = getEnv(); assert.deepStrictEqual(env.OTEL_NO_PATCH_MODULES, ['a', 'b', 'c']); @@ -128,6 +130,8 @@ describe('environment', () => { assert.strictEqual(env.OTEL_BSP_SCHEDULE_DELAY, 50); assert.strictEqual(env.OTEL_TRACES_SAMPLER, 'always_on'); assert.strictEqual(env.OTEL_TRACES_SAMPLER_ARG, '0.5'); + assert.strictEqual(env.OTEL_EXPORTER_OTLP_TIMEOUT, 15000); + assert.strictEqual(env.OTEL_EXPORTER_OTLP_TRACES_TIMEOUT, 12000); }); it('should parse OTEL_LOG_LEVEL despite casing', () => {