feat(instrumentation-http): monitor error events with events.errorMon…

…itor (#3402) Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
open-telemetry · Nov 13, 2022 · b4f04ae · b4f04ae
1 parent 76073ee
commit b4f04ae
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 46 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -8,6 +8,8 @@ All notable changes to experimental packages in this project will be documented
 
 ### :rocket: (Enhancement)
 
+* feat(instrumentation-http): monitor error events with events.errorMonitor [#3402](https://github.com/open-telemetry/opentelemetry-js/pull/3402) @legendecas
+
 ### :bug: (Bug Fix)
 
 ### :books: (Refine Doc)

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -54,6 +54,7 @@ import {
   safeExecuteInTheMiddle,
 } from '@opentelemetry/instrumentation';
 import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
+import { errorMonitor } from 'events';
 
 /**
  * Http instrumentation instrumentation for Opentelemetry
@@ -361,7 +362,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
 
           this._closeHttpSpan(span, SpanKind.CLIENT, startTime, metricAttributes);
         });
-        response.on('error', (error: Err) => {
+        response.on(errorMonitor, (error: Err) => {
           this._diag.debug('outgoingRequest on error()', error);
           utils.setSpanWithError(span, error);
           const code = utils.parseResponseStatus(SpanKind.CLIENT, response.statusCode);
@@ -376,7 +377,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         this._closeHttpSpan(span, SpanKind.CLIENT, startTime, metricAttributes);
       }
     });
-    request.on('error', (error: Err) => {
+    request.on(errorMonitor, (error: Err) => {
       this._diag.debug('outgoingRequest on request error()', error);
       utils.setSpanWithError(span, error);
       this._closeHttpSpan(span, SpanKind.CLIENT, startTime, metricAttributes);

diff --git a/...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
@@ -670,21 +670,21 @@ describe('HttpInstrumentation', () => {
           );
           req.setTimeout(10, () => {
             req.abort();
-            reject('timeout');
+          });
+          // Instrumentation should not swallow error event.
+          assert.strictEqual(req.listeners('error').length, 0);
+          req.on('error', err => {
+            reject(err);
           });
           return req.end();
         });
 
-        try {
-          await promiseRequest;
-          assert.fail();
-        } catch (error) {
-          const spans = memoryExporter.getFinishedSpans();
-          const [span] = spans;
-          assert.strictEqual(spans.length, 1);
-          assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
-          assert.ok(Object.keys(span.attributes).length >= 6);
-        }
+        await assert.rejects(promiseRequest, /Error: socket hang up/);
+        const spans = memoryExporter.getFinishedSpans();
+        const [span] = spans;
+        assert.strictEqual(spans.length, 1);
+        assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
+        assert.ok(Object.keys(span.attributes).length >= 6);
       });
 
       it('should have 1 ended span when request is aborted after receiving response', async () => {
@@ -701,28 +701,29 @@ describe('HttpInstrumentation', () => {
             (resp: http.IncomingMessage) => {
               let data = '';
               resp.on('data', chunk => {
-                req.destroy(Error());
+                req.destroy(Error('request destroyed'));
                 data += chunk;
               });
               resp.on('end', () => {
                 resolve(data);
               });
             }
           );
+          // Instrumentation should not swallow error event.
+          assert.strictEqual(req.listeners('error').length, 0);
+          req.on('error', err => {
+            reject(err);
+          });
 
           return req.end();
         });
 
-        try {
-          await promiseRequest;
-          assert.fail();
-        } catch (error) {
-          const spans = memoryExporter.getFinishedSpans();
-          const [span] = spans;
-          assert.strictEqual(spans.length, 1);
-          assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
-          assert.ok(Object.keys(span.attributes).length > 7);
-        }
+        await assert.rejects(promiseRequest, /Error: request destroyed/);
+        const spans = memoryExporter.getFinishedSpans();
+        const [span] = spans;
+        assert.strictEqual(spans.length, 1);
+        assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
+        assert.ok(Object.keys(span.attributes).length > 7);
       });
 
       it("should have 1 ended client span when request doesn't listening response", done => {

diff --git a/...imental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts b/...imental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts
@@ -639,21 +639,21 @@ describe('HttpsInstrumentation', () => {
           );
           req.setTimeout(10, () => {
             req.abort();
-            reject('timeout');
+          });
+          // Instrumentation should not swallow error event.
+          assert.strictEqual(req.listeners('error').length, 0);
+          req.on('error', err => {
+            reject(err);
           });
           return req.end();
         });
 
-        try {
-          await promiseRequest;
-          assert.fail();
-        } catch (error) {
-          const spans = memoryExporter.getFinishedSpans();
-          const [span] = spans;
-          assert.strictEqual(spans.length, 1);
-          assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
-          assert.ok(Object.keys(span.attributes).length >= 6);
-        }
+        await assert.rejects(promiseRequest, /Error: socket hang up/);
+        const spans = memoryExporter.getFinishedSpans();
+        const [span] = spans;
+        assert.strictEqual(spans.length, 1);
+        assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
+        assert.ok(Object.keys(span.attributes).length >= 6);
       });
 
       it('should have 1 ended span when request is aborted after receiving response', async () => {
@@ -670,28 +670,29 @@ describe('HttpsInstrumentation', () => {
             (resp: http.IncomingMessage) => {
               let data = '';
               resp.on('data', chunk => {
-                req.destroy(Error());
+                req.destroy(Error('request destroyed'));
                 data += chunk;
               });
               resp.on('end', () => {
                 resolve(data);
               });
             }
           );
+          // Instrumentation should not swallow error event.
+          assert.strictEqual(req.listeners('error').length, 0);
+          req.on('error', err => {
+            reject(err);
+          });
 
           return req.end();
         });
 
-        try {
-          await promiseRequest;
-          assert.fail();
-        } catch (error) {
-          const spans = memoryExporter.getFinishedSpans();
-          const [span] = spans;
-          assert.strictEqual(spans.length, 1);
-          assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
-          assert.ok(Object.keys(span.attributes).length > 7);
-        }
+        await assert.rejects(promiseRequest, /Error: request destroyed/);
+        const spans = memoryExporter.getFinishedSpans();
+        const [span] = spans;
+        assert.strictEqual(spans.length, 1);
+        assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
+        assert.ok(Object.keys(span.attributes).length > 7);
       });
 
       it("should have 1 ended span when response is listened by using req.on('response')", done => {