feat(plugin-http): sync. specs for statuscode (open-telemetry#719)

test: fix and add tests closes open-telemetry#642 Signed-off-by: Olivier Albertini <[email protected]>
srikanthccv · Jan 28, 2020 · 53dda64 · 53dda64
1 parent 69947da
commit 53dda64
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 34 deletions.
diff --git a/packages/opentelemetry-plugin-http/README.md b/packages/opentelemetry-plugin-http/README.md
@@ -55,6 +55,7 @@ Http plugin has few options available to choose from. You can set the following:
 | [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all incoming requests that match paths |
 | [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all outgoing requests that match urls |
 | [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `string` | The primary server name of the matched virtual host. |
+
 ## Useful links
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
 - For more about OpenTelemetry JavaScript: <https://github.com/open-telemetry/opentelemetry-js>

diff --git a/packages/opentelemetry-plugin-http/src/types.ts b/packages/opentelemetry-plugin-http/src/types.ts
@@ -78,3 +78,7 @@ export interface Err extends Error {
   syscall?: string;
   stack?: string;
 }
+
+export interface SpecialHttpStatusCodeMapping {
+  [custom: number]: number;
+}
diff --git a/packages/opentelemetry-plugin-http/src/utils.ts b/packages/opentelemetry-plugin-http/src/utils.ts
@@ -23,12 +23,30 @@ import {
   OutgoingHttpHeaders,
   ServerResponse,
 } from 'http';
-import { IgnoreMatcher, Err, ParsedRequestOptions } from './types';
+import {
+  IgnoreMatcher,
+  Err,
+  ParsedRequestOptions,
+  SpecialHttpStatusCodeMapping,
+} from './types';
 import { AttributeNames } from './enums/AttributeNames';
 import * as url from 'url';
 import { Socket } from 'net';
 
 export const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request';
+
+export const HTTP_STATUS_SPECIAL_CASES: SpecialHttpStatusCodeMapping = {
+  401: CanonicalCode.UNAUTHENTICATED,
+  403: CanonicalCode.PERMISSION_DENIED,
+  404: CanonicalCode.NOT_FOUND,
+  429: CanonicalCode.RESOURCE_EXHAUSTED,
+  501: CanonicalCode.UNIMPLEMENTED,
+  503: CanonicalCode.UNAVAILABLE,
+  504: CanonicalCode.DEADLINE_EXCEEDED,
+  598: CanonicalCode.INTERNAL,
+  599: CanonicalCode.INTERNAL,
+};
+
 /**
  * Get an absolute url
  */
@@ -58,37 +76,40 @@ export const getAbsoluteUrl = (
   return `${protocol}//${host}${path}`;
 };
 /**
- * Parse status code from HTTP response.
+ * Parse status code from HTTP response. [More details](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md#status)
  */
 export const parseResponseStatus = (
   statusCode: number
 ): Omit<Status, 'message'> => {
-  if (statusCode < 200 || statusCode > 504) {
+  // search for special case
+  const code: number | undefined = HTTP_STATUS_SPECIAL_CASES[statusCode];
+
+  if (code !== undefined) {
+    return { code };
+  }
+
+  // 0xx are unknown
+  if (statusCode < 100) {
     return { code: CanonicalCode.UNKNOWN };
-  } else if (statusCode >= 200 && statusCode < 400) {
+  }
+
+  // 1xx, 2xx, 3xx are OK
+  if (statusCode < 400) {
     return { code: CanonicalCode.OK };
-  } else {
-    switch (statusCode) {
-      case 400:
-        return { code: CanonicalCode.INVALID_ARGUMENT };
-      case 504:
-        return { code: CanonicalCode.DEADLINE_EXCEEDED };
-      case 404:
-        return { code: CanonicalCode.NOT_FOUND };
-      case 403:
-        return { code: CanonicalCode.PERMISSION_DENIED };
-      case 401:
-        return { code: CanonicalCode.UNAUTHENTICATED };
-      case 429:
-        return { code: CanonicalCode.RESOURCE_EXHAUSTED };
-      case 501:
-        return { code: CanonicalCode.UNIMPLEMENTED };
-      case 503:
-        return { code: CanonicalCode.UNAVAILABLE };
-      default:
-        return { code: CanonicalCode.UNKNOWN };
-    }
   }
+
+  // 4xx are client errors
+  if (statusCode < 500) {
+    return { code: CanonicalCode.INVALID_ARGUMENT };
+  }
+
+  // 5xx are internal errors
+  if (statusCode < 512) {
+    return { code: CanonicalCode.INTERNAL };
+  }
+
+  // All other codes are unknown
+  return { code: CanonicalCode.UNKNOWN };
 };
 
 /**

diff --git a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts
@@ -280,7 +280,19 @@ describe('HttpPlugin', () => {
         assert.strictEqual(spans.length, 0);
       });
 
-      const httpErrorCodes = [400, 401, 403, 404, 429, 501, 503, 504, 500, 505];
+      const httpErrorCodes = [
+        400,
+        401,
+        403,
+        404,
+        429,
+        501,
+        503,
+        504,
+        500,
+        505,
+        597,
+      ];
 
       for (let i = 0; i < httpErrorCodes.length; i++) {
         it(`should test span for GET requests with http error ${httpErrorCodes[i]}`, async () => {

diff --git a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts
@@ -14,17 +14,17 @@
  * limitations under the License.
  */
 
+import { NoopLogger } from '@opentelemetry/core';
+import { NoopScopeManager } from '@opentelemetry/scope-base';
+import { BasicTracerRegistry, Span } from '@opentelemetry/tracing';
+import { CanonicalCode, SpanKind } from '@opentelemetry/types';
 import * as assert from 'assert';
+import * as http from 'http';
 import * as sinon from 'sinon';
 import * as url from 'url';
-import { CanonicalCode, SpanKind } from '@opentelemetry/types';
-import { NoopScopeManager } from '@opentelemetry/scope-base';
+import { AttributeNames } from '../../src';
 import { IgnoreMatcher } from '../../src/types';
 import * as utils from '../../src/utils';
-import * as http from 'http';
-import { Span, BasicTracerRegistry } from '@opentelemetry/tracing';
-import { AttributeNames } from '../../src';
-import { NoopLogger } from '@opentelemetry/core';
 
 describe('Utility', () => {
   describe('parseResponseStatus()', () => {
@@ -36,18 +36,25 @@ describe('Utility', () => {
     });
 
     it('should return OK for Success HTTP status code', () => {
-      for (let index = 200; index < 400; index++) {
+      for (let index = 100; index < 400; index++) {
         const status = utils.parseResponseStatus(index);
         assert.deepStrictEqual(status, { code: CanonicalCode.OK });
       }
     });
 
     it('should not return OK for Bad HTTP status code', () => {
-      for (let index = 400; index <= 504; index++) {
+      for (let index = 400; index <= 600; index++) {
         const status = utils.parseResponseStatus(index);
         assert.notStrictEqual(status.code, CanonicalCode.OK);
       }
     });
+    it('should handle special HTTP status codes', () => {
+      for (const key in utils.HTTP_STATUS_SPECIAL_CASES) {
+        const status = utils.parseResponseStatus(key as any);
+        const canonicalCode = utils.HTTP_STATUS_SPECIAL_CASES[key];
+        assert.deepStrictEqual(status.code, canonicalCode);
+      }
+    });
   });
   describe('hasExpectHeader()', () => {
     it('should throw if no option', () => {