fix(instrumentation-http): set conditionally required error.type attribute

experimental/CHANGELOG.md

@@ -12,6 +12,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 ### :bug: Bug Fixes
 
+* fix(instrumentation-http): set `error.type` on spans whose status code makes them an error [#7061](https://github.com/open-telemetry/opentelemetry-js/pull/7061) @mwear
+
 ### :books: Documentation
 
 ### :house: Internal

experimental/packages/opentelemetry-instrumentation-http/src/http.ts

@@ -66,6 +66,7 @@ import {
   isURLLike,
   headerCapture,
   isValidOptionsType,
+  parseErrorType,
   parseResponseStatus,
   setSpanWithError,
 } from './utils';
@@ -432,6 +433,13 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
             status = {
               code: parseResponseStatus(SpanKind.CLIENT, response.statusCode),
             };
+            const errorType = parseErrorType(
+              SpanKind.CLIENT,
+              response.statusCode
+            );
+            if (errorType !== undefined) {
+              span.setAttribute(ATTR_ERROR_TYPE, errorType);
+            }
           }
 
           span.setStatus(status);
@@ -817,6 +825,11 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       code: parseResponseStatus(SpanKind.SERVER, response.statusCode),
     });
 
+    const errorType = parseErrorType(SpanKind.SERVER, response.statusCode);
+    if (errorType !== undefined) {
+      span.setAttribute(ATTR_ERROR_TYPE, errorType);
+    }
+
     const route = attributes[ATTR_HTTP_ROUTE];
     if (route) {
       span.updateName(`${request.method || 'GET'} ${route}`);

experimental/packages/opentelemetry-instrumentation-http/src/utils.ts

@@ -137,6 +137,27 @@ export const parseResponseStatus = (
   return SpanStatusCode.ERROR;
 };
 
+/**
+ * Returns the `error.type` value for a response status code, or undefined when
+ * the code is not an error for this span kind. Semconv asks for the status code
+ * as a string once a response was received.
+ */
+export const parseErrorType = (
+  kind: SpanKind,
+  statusCode?: unknown
+): string | undefined => {
+  const lowerBound = kind === SpanKind.CLIENT ? 400 : 500;
+  if (
+    typeof statusCode === 'number' &&
+    statusCode >= lowerBound &&
+    statusCode < 600
+  ) {
+    return String(statusCode);
+  }
+
+  return undefined;
+};
+
 /**
  * Check whether the given obj match pattern
  * @param constant e.g URL of request
@@ -529,12 +550,9 @@ export const getOutgoingStableRequestMetricAttributesOnResponse = (
   const statusCode = spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
   if (statusCode) {
     metricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] = statusCode;
-    if (
-      typeof statusCode === 'number' &&
-      statusCode >= 400 &&
-      statusCode < 600
-    ) {
-      metricAttributes[ATTR_ERROR_TYPE] ??= String(statusCode);
+    const errorType = parseErrorType(SpanKind.CLIENT, statusCode);
+    if (errorType !== undefined) {
+      metricAttributes[ATTR_ERROR_TYPE] ??= errorType;
     }
   }
   return metricAttributes;
@@ -860,12 +878,9 @@ export const getIncomingStableRequestMetricAttributesOnResponse = (
   const statusCode = spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
   if (statusCode) {
     metricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] = statusCode;
-    if (
-      typeof statusCode === 'number' &&
-      statusCode >= 500 &&
-      statusCode < 600
-    ) {
-      metricAttributes[ATTR_ERROR_TYPE] ??= String(statusCode);
+    const errorType = parseErrorType(SpanKind.SERVER, statusCode);
+    if (errorType !== undefined) {
+      metricAttributes[ATTR_ERROR_TYPE] ??= errorType;
     }
   }
 

experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts

@@ -20,6 +20,7 @@ import {
 } from '@opentelemetry/sdk-trace';
 import {
   ATTR_CLIENT_ADDRESS,
+  ATTR_ERROR_TYPE,
   ATTR_HTTP_REQUEST_METHOD,
   ATTR_HTTP_RESPONSE_STATUS_CODE,
   ATTR_HTTP_ROUTE,
@@ -344,6 +345,10 @@ describe('HttpInstrumentation', () => {
           if (request.url?.includes('/withQuery')) {
             assert.match(request.url, /withQuery\?foo=bar$/);
           }
+          const status = request.url?.match(/\/status\/(\d+)/);
+          if (status) {
+            response.statusCode = Number(status[1]);
+          }
           response.end('Test Server Response');
         });
 
@@ -418,6 +423,43 @@ describe('HttpInstrumentation', () => {
         assert.strictEqual(span.name, 'GET TheRoute');
       });
 
+      it('should set error.type to the status code on a failing span', async () => {
+        await httpRequest.get(
+          `${protocol}://${hostname}:${serverPort}/status/500`
+        );
+        const spans = memoryExporter.getFinishedSpans();
+        const incomingSpan = spans.find(s => s.kind === SpanKind.SERVER);
+        const outgoingSpan = spans.find(s => s.kind === SpanKind.CLIENT);
+        assert.ok(incomingSpan);
+        assert.ok(outgoingSpan);
+
+        for (const span of [incomingSpan, outgoingSpan]) {
+          assert.strictEqual(span.status.code, SpanStatusCode.ERROR);
+          assert.strictEqual(span.attributes[ATTR_ERROR_TYPE], '500');
+        }
+      });
+
+      it('should treat 4xx as an error on the client span only', async () => {
+        await httpRequest.get(
+          `${protocol}://${hostname}:${serverPort}/status/404`
+        );
+        const spans = memoryExporter.getFinishedSpans();
+        const incomingSpan = spans.find(s => s.kind === SpanKind.SERVER);
+        const outgoingSpan = spans.find(s => s.kind === SpanKind.CLIENT);
+        assert.ok(incomingSpan);
+        assert.ok(outgoingSpan);
+
+        assert.strictEqual(incomingSpan.status.code, SpanStatusCode.UNSET);
+        assert.strictEqual(
+          incomingSpan.attributes[ATTR_ERROR_TYPE],
+          undefined,
+          "a 4xx is the caller's error, not the server's"
+        );
+
+        assert.strictEqual(outgoingSpan.status.code, SpanStatusCode.ERROR);
+        assert.strictEqual(outgoingSpan.attributes[ATTR_ERROR_TYPE], '404');
+      });
+
       const httpErrorCodes = [
         400, 401, 403, 404, 429, 501, 503, 504, 500, 505, 597,
       ];

experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts

@@ -60,6 +60,52 @@ describe('Utility', () => {
     });
   });
 
+  describe('parseErrorType()', () => {
+    it('should return the status code as a string for an error', () => {
+      assert.strictEqual(utils.parseErrorType(SpanKind.CLIENT, 404), '404');
+      assert.strictEqual(utils.parseErrorType(SpanKind.CLIENT, 500), '500');
+      assert.strictEqual(utils.parseErrorType(SpanKind.SERVER, 500), '500');
+    });
+
+    it('should return undefined for a successful status code', () => {
+      for (let index = 100; index < 400; index++) {
+        assert.strictEqual(
+          utils.parseErrorType(SpanKind.CLIENT, index),
+          undefined
+        );
+        assert.strictEqual(
+          utils.parseErrorType(SpanKind.SERVER, index),
+          undefined
+        );
+      }
+    });
+
+    it('should treat 4xx as an error on a client span only', () => {
+      for (let index = 400; index < 500; index++) {
+        assert.strictEqual(
+          utils.parseErrorType(SpanKind.CLIENT, index),
+          String(index)
+        );
+        assert.strictEqual(
+          utils.parseErrorType(SpanKind.SERVER, index),
+          undefined
+        );
+      }
+    });
+
+    it('should return undefined when no status code was received', () => {
+      assert.strictEqual(
+        utils.parseErrorType(SpanKind.CLIENT, undefined),
+        undefined
+      );
+      assert.strictEqual(
+        utils.parseErrorType(SpanKind.CLIENT, '500'),
+        undefined
+      );
+      assert.strictEqual(utils.parseErrorType(SpanKind.CLIENT, 600), undefined);
+    });
+  });
+
   describe('getRequestInfo()', () => {
     it('should get options object', () => {
       const webUrl = 'http://u:p@google.fr/aPath?qu=ry';

experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts

@@ -7,6 +7,7 @@ import { isValidSpanId, SpanKind } from '@opentelemetry/api';
 import { hrTimeToNanoseconds } from '@opentelemetry/core';
 import type { ReadableSpan } from '@opentelemetry/sdk-trace';
 import {
+  ATTR_ERROR_TYPE,
   ATTR_HTTP_REQUEST_METHOD,
   ATTR_HTTP_RESPONSE_STATUS_CODE,
   ATTR_NETWORK_PEER_ADDRESS,
@@ -79,6 +80,14 @@ export const assertSpan = (
     }
   );
 
+  // A forced status comes from an exception, which carries its own error.type.
+  if (!validations.forceStatus) {
+    assert.strictEqual(
+      span.attributes[ATTR_ERROR_TYPE],
+      utils.parseErrorType(span.kind, validations.httpStatusCode)
+    );
+  }
+
   assert.ok(span.endTime, 'must be finished');
   assert.ok(hrTimeToNanoseconds(span.duration), 'must have positive duration');