From a241bf6195eb7c795ae7fa963b5aa94f7a579fb7 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 22 Nov 2024 11:45:54 +0100 Subject: [PATCH 1/5] fix(otlp-exporter-base): don't create blob before sending xhr --- .../browser/CollectorTraceExporter.test.ts | 23 +-- .../browser/CollectorMetricExporter.test.ts | 137 +++++++++--------- .../src/transport/xhr-transport.ts | 2 +- .../test/browser/xhr-transport.test.ts | 4 - 4 files changed, 86 insertions(+), 80 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 00e160dab49..1de1e9485b0 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 @@ -187,10 +187,9 @@ describe('OTLPTraceExporter - web', () => { assert.strictEqual(request.method, 'POST'); assert.strictEqual(request.url, 'http://foo.bar.com'); - const body = request.requestBody as Blob; - const decoder = new TextDecoder(); + const body = request.requestBody as Uint8Array; const json = JSON.parse( - decoder.decode(await body.arrayBuffer()) + new TextDecoder().decode(body) ) as IExportTraceServiceRequest; const span1 = json.resourceSpans?.[0].scopeSpans?.[0].spans?.[0]; @@ -340,14 +339,18 @@ describe('OTLPTraceExporter - web', () => { collectorTraceExporter.export(spans, () => {}); queueMicrotask(() => { - const [{ requestHeaders }] = server.requests; - - ensureHeadersContain(requestHeaders, customHeaders); - assert.strictEqual(stubBeacon.callCount, 0); - assert.strictEqual(stubOpen.callCount, 0); + try { + const [{ requestHeaders: requestHeaders }] = server.requests; - clock.restore(); - done(); + ensureHeadersContain(requestHeaders, customHeaders); + assert.strictEqual(stubBeacon.callCount, 0); + assert.strictEqual(stubOpen.callCount, 0); + done(); + } catch (e) { + done(e); + } finally { + clock.restore(); + } }); }); it('should log the timeout request error message', done => { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts index aa7270a6ffb..55dd0a8049f 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/CollectorMetricExporter.test.ts @@ -240,77 +240,84 @@ describe('OTLPMetricExporter - web', () => { collectorExporter.export(metrics, () => {}); queueMicrotask(async () => { - const request = server.requests[0]; - assert.strictEqual(request.method, 'POST'); - assert.strictEqual(request.url, 'http://foo.bar.com'); - - const body = request.requestBody; - const decoder = new TextDecoder(); - const json = JSON.parse( - decoder.decode(await body.arrayBuffer()) - ) as IExportMetricsServiceRequest; - // The order of the metrics is not guaranteed. - const counterIndex = metrics.scopeMetrics[0].metrics.findIndex( - it => it.descriptor.name === 'int-counter' - ); - const observableIndex = metrics.scopeMetrics[0].metrics.findIndex( - it => it.descriptor.name === 'double-observable-gauge2' - ); - const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex( - it => it.descriptor.name === 'int-histogram' - ); - - const metric1 = - json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; - const metric2 = - json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; - const metric3 = - json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; + try { + const request = server.requests[0]; + assert.strictEqual(request.method, 'POST'); + assert.strictEqual(request.url, 'http://foo.bar.com'); + + const body = request.requestBody; + const json = JSON.parse( + new TextDecoder().decode(body) + ) as IExportMetricsServiceRequest; + // The order of the metrics is not guaranteed. + const counterIndex = metrics.scopeMetrics[0].metrics.findIndex( + it => it.descriptor.name === 'int-counter' + ); + const observableIndex = metrics.scopeMetrics[0].metrics.findIndex( + it => it.descriptor.name === 'double-observable-gauge2' + ); + const histogramIndex = metrics.scopeMetrics[0].metrics.findIndex( + it => it.descriptor.name === 'int-histogram' + ); - assert.ok(typeof metric1 !== 'undefined', "metric doesn't exist"); - ensureCounterIsCorrect( - metric1, - metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0].endTime, - metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0] - .startTime - ); + const metric1 = + json.resourceMetrics[0].scopeMetrics[0].metrics[counterIndex]; + const metric2 = + json.resourceMetrics[0].scopeMetrics[0].metrics[observableIndex]; + const metric3 = + json.resourceMetrics[0].scopeMetrics[0].metrics[histogramIndex]; + + assert.ok(typeof metric1 !== 'undefined', "metric doesn't exist"); + ensureCounterIsCorrect( + metric1, + metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0] + .endTime, + metrics.scopeMetrics[0].metrics[counterIndex].dataPoints[0] + .startTime + ); - assert.ok( - typeof metric2 !== 'undefined', - "second metric doesn't exist" - ); - ensureObservableGaugeIsCorrect( - metric2, - metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] - .endTime, - metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] - .startTime, - 6, - 'double-observable-gauge2' - ); + assert.ok( + typeof metric2 !== 'undefined', + "second metric doesn't exist" + ); + ensureObservableGaugeIsCorrect( + metric2, + metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] + .endTime, + metrics.scopeMetrics[0].metrics[observableIndex].dataPoints[0] + .startTime, + 6, + 'double-observable-gauge2' + ); - assert.ok( - typeof metric3 !== 'undefined', - "third metric doesn't exist" - ); - ensureHistogramIsCorrect( - metric3, - metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] - .endTime, - metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] - .startTime, - [0, 100], - [0, 2, 0] - ); + assert.ok( + typeof metric3 !== 'undefined', + "third metric doesn't exist" + ); + ensureHistogramIsCorrect( + metric3, + metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] + .endTime, + metrics.scopeMetrics[0].metrics[histogramIndex].dataPoints[0] + .startTime, + [0, 100], + [0, 2, 0] + ); - const resource = json.resourceMetrics[0].resource; - assert.ok(typeof resource !== 'undefined', "resource doesn't exist"); - ensureWebResourceIsCorrect(resource); + const resource = json.resourceMetrics[0].resource; + assert.ok( + typeof resource !== 'undefined', + "resource doesn't exist" + ); + ensureWebResourceIsCorrect(resource); - assert.strictEqual(stubBeacon.callCount, 0); - ensureExportMetricsServiceRequestIsSet(json); + assert.strictEqual(stubBeacon.callCount, 0); + ensureExportMetricsServiceRequestIsSet(json); - done(); + done(); + } catch (e) { + done(e); + } }); }); diff --git a/experimental/packages/otlp-exporter-base/src/transport/xhr-transport.ts b/experimental/packages/otlp-exporter-base/src/transport/xhr-transport.ts index e2802aa521b..17c02096478 100644 --- a/experimental/packages/otlp-exporter-base/src/transport/xhr-transport.ts +++ b/experimental/packages/otlp-exporter-base/src/transport/xhr-transport.ts @@ -81,7 +81,7 @@ class XhrTransport implements IExporterTransport { }); }; - xhr.send(new Blob([data], { type: headers['Content-Type'] })); + xhr.send(data); }); } diff --git a/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts b/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts index 7fd82fc0544..1bbb6d9d4e1 100644 --- a/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts +++ b/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts @@ -65,10 +65,6 @@ describe('XhrTransport', function () { undefined ); assert.strictEqual(request.url, testTransportParameters.url); - assert.strictEqual( - (request.requestBody as unknown as Blob).type, - 'application/json' - ); ensureHeadersContain(request.requestHeaders, { foo: 'foo-value', bar: 'bar-value', From 57c1dc9bd41c66566cb4c7324e39d8aeb0e98518 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 22 Nov 2024 13:03:50 +0100 Subject: [PATCH 2/5] fixup! fix(otlp-exporter-base): don't create blob before sending xhr --- .../otlp-exporter-base/test/browser/xhr-transport.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts b/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts index 1bbb6d9d4e1..5928206469a 100644 --- a/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts +++ b/experimental/packages/otlp-exporter-base/test/browser/xhr-transport.test.ts @@ -65,6 +65,7 @@ describe('XhrTransport', function () { undefined ); assert.strictEqual(request.url, testTransportParameters.url); + assert.strictEqual(request.requestBody, testPayload); ensureHeadersContain(request.requestHeaders, { foo: 'foo-value', bar: 'bar-value', From 079511f50411fc19c07231691ae5f4cf4bd56248 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 22 Nov 2024 13:13:25 +0100 Subject: [PATCH 3/5] chore: add changelog entry --- experimental/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 1d41e62f082..47a93862bb1 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -26,6 +26,9 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) +* fix(otlp-exporter-base): don't create blob before sending xhr [#5193](https://github.com/open-telemetry/opentelemetry-js/pull/5193) + * improves compatibility with some unsupported runtimes + ### :books: (Refine Doc) ### :house: (Internal) From d8eee42a9464430dbd9ba917ea36695229ddcd0c Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 25 Nov 2024 09:51:53 +0100 Subject: [PATCH 4/5] chore: update changelog --- experimental/CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index da42448e364..5a13766d59a 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -23,12 +23,11 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) * feat(otlp-exporter-base): internally accept a http header provider function only [#5179](https://github.com/open-telemetry/opentelemetry-js/pull/5179) @pichlermarc +* refactor(otlp-exporter-base): don't create blob before sending xhr [#5193](https://github.com/open-telemetry/opentelemetry-js/pull/5193) + * improves compatibility with some unsupported runtimes ### :bug: (Bug Fix) -* fix(otlp-exporter-base): don't create blob before sending xhr [#5193](https://github.com/open-telemetry/opentelemetry-js/pull/5193) - * improves compatibility with some unsupported runtimes - ### :books: (Refine Doc) ### :house: (Internal) From 3ec05b01fdadcbb8a3b9fd7cabcd532abe07eb7b Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 25 Nov 2024 09:53:25 +0100 Subject: [PATCH 5/5] chore: add github handle to changelog --- experimental/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 5a13766d59a..7edb7aa07e1 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -23,7 +23,7 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) * feat(otlp-exporter-base): internally accept a http header provider function only [#5179](https://github.com/open-telemetry/opentelemetry-js/pull/5179) @pichlermarc -* refactor(otlp-exporter-base): don't create blob before sending xhr [#5193](https://github.com/open-telemetry/opentelemetry-js/pull/5193) +* refactor(otlp-exporter-base): don't create blob before sending xhr [#5193](https://github.com/open-telemetry/opentelemetry-js/pull/5193) @pichlermarc * improves compatibility with some unsupported runtimes ### :bug: (Bug Fix)