From ef798042d96676229cb3c57e19a62ea49bf46e9b Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Tue, 20 Oct 2020 17:45:59 +0200 Subject: [PATCH] feat(exporter-collector): log upstream error #1459 --- .../src/platform/node/util.ts | 25 +++++++++++-------- .../src/types.ts | 4 ++- .../test/helper.ts | 20 +++++++++++++++ .../test/node/CollectorMetricExporter.test.ts | 22 ++++++++-------- .../test/node/CollectorTraceExporter.test.ts | 20 +++++++-------- 5 files changed, 59 insertions(+), 32 deletions(-) diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/util.ts b/packages/opentelemetry-exporter-collector/src/platform/node/util.ts index 0e20cfc8ce2..29d46e29436 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/util.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/util.ts @@ -50,16 +50,21 @@ export function sendWithHttp( const request = parsedUrl.protocol === 'http:' ? http.request : https.request; const req = request(options, (res: http.IncomingMessage) => { - if (res.statusCode && res.statusCode < 299) { - collector.logger.debug(`statusCode: ${res.statusCode}`); - onSuccess(); - } else { - const error = new collectorTypes.CollectorExporterError( - res.statusMessage, - res.statusCode - ); - onError(error); - } + let data = ''; + res.on('data', chunk => (data += chunk)); + res.on('end', () => { + if (res.statusCode && res.statusCode < 299) { + collector.logger.debug(`statusCode: ${res.statusCode}`, data); + onSuccess(); + } else { + const error = new collectorTypes.CollectorExporterError( + res.statusMessage, + res.statusCode, + data + ); + onError(error); + } + }); }); req.on('error', (error: Error) => { diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index ba8c474cafa..694c60d8d9a 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/packages/opentelemetry-exporter-collector/src/types.ts @@ -310,9 +310,11 @@ export namespace opentelemetryProto { export class CollectorExporterError extends Error { readonly code?: number; readonly name: string = 'CollectorExporterError'; + readonly data?: string; - constructor(message?: string, code?: number) { + constructor(message?: string, code?: number, data?: string) { super(message); + this.data = data; this.code = code; } } diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts index e11257138e7..584ed2b67f8 100644 --- a/packages/opentelemetry-exporter-collector/test/helper.ts +++ b/packages/opentelemetry-exporter-collector/test/helper.ts @@ -22,6 +22,7 @@ import { InstrumentationLibrary } from '@opentelemetry/core'; import * as assert from 'assert'; import { opentelemetryProto } from '../src/types'; import * as collectorTypes from '../src/types'; +import { Stream } from 'stream'; const meterProvider = new MeterProvider({ interval: 30000, @@ -666,3 +667,22 @@ export function ensureHeadersContain( ); }); } + +export class MockedResponse extends Stream { + constructor(private _code: number, private _msg?: string) { + super(); + } + + send(data: string) { + this.emit('data', data); + this.emit('end'); + } + + get statusCode() { + return this._code; + } + + get statusMessage() { + return this._msg; + } +} diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts index 85f447cbabb..86d180e6cd2 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts @@ -31,6 +31,7 @@ import { mockValueRecorder, ensureValueRecorderIsCorrect, ensureObserverIsCorrect, + MockedResponse, } from '../helper'; import { MetricRecord } from '@opentelemetry/metrics'; @@ -40,14 +41,6 @@ const fakeRequest = { write: function () {}, }; -const mockRes = { - statusCode: 200, -}; - -const mockResError = { - statusCode: 400, -}; - const address = 'localhost:1501'; describe('CollectorMetricExporter - node with json over http', () => { @@ -178,9 +171,11 @@ describe('CollectorMetricExporter - node with json over http', () => { collectorExporter.export(metrics, responseSpy); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = spyRequest.args[0]; const callback = args[1]; callback(mockRes); + mockRes.send('success'); setTimeout(() => { const response: any = spyLoggerDebug.args[1][0]; assert.strictEqual(response, 'statusCode: 200'); @@ -205,12 +200,17 @@ describe('CollectorMetricExporter - node with json over http', () => { collectorExporter.export(metrics, responseSpy); setTimeout(() => { + const mockRes = new MockedResponse(400); const args = spyRequest.args[0]; const callback = args[1]; - callback(mockResError); + callback(mockRes); + mockRes.send('failed'); setTimeout(() => { - const response = spyLoggerError.args[0][0] as string; - assert.ok(response.includes('"code":"400"')); + const error = JSON.parse( + spyLoggerError.args[0][0] + ) as collectorTypes.CollectorExporterError; + assert.strictEqual(error.code, '400'); + assert.strictEqual(error.data, 'failed'); assert.strictEqual(responseSpy.args[0][0], 1); done(); }); diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts index 0429cb57f1b..0328ed0be35 100644 --- a/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts +++ b/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts @@ -23,6 +23,7 @@ import * as sinon from 'sinon'; import { CollectorTraceExporter } from '../../src/platform/node'; import { CollectorExporterConfigBase } from '../../src/types'; import * as collectorTypes from '../../src/types'; +import { MockedResponse } from '../helper'; import { ensureExportTraceServiceRequestIsSet, @@ -36,13 +37,6 @@ const fakeRequest = { write: function () {}, }; -const mockRes = { - statusCode: 200, -}; - -const mockResError = { - statusCode: 400, -}; const address = 'localhost:1501'; describe('CollectorTraceExporter - node with json over http', () => { @@ -144,9 +138,11 @@ describe('CollectorTraceExporter - node with json over http', () => { collectorExporter.export(spans, responseSpy); setTimeout(() => { + const mockRes = new MockedResponse(200); const args = spyRequest.args[0]; const callback = args[1]; callback(mockRes); + mockRes.send('success'); setTimeout(() => { const response: any = spyLoggerDebug.args[1][0]; assert.strictEqual(response, 'statusCode: 200'); @@ -171,13 +167,17 @@ describe('CollectorTraceExporter - node with json over http', () => { collectorExporter.export(spans, responseSpy); setTimeout(() => { + const mockResError = new MockedResponse(400); const args = spyRequest.args[0]; const callback = args[1]; callback(mockResError); + mockResError.send('failed'); setTimeout(() => { - const response = spyLoggerError.args[0][0] as string; - - assert.ok(response.includes('"code":"400"')); + const error = JSON.parse( + spyLoggerError.args[0][0] + ) as collectorTypes.CollectorExporterError; + assert.strictEqual(error.code, '400'); + assert.strictEqual(error.data, 'failed'); assert.strictEqual(responseSpy.args[0][0], 1); done(); });