feat: adding possibility of recording exception (#1372)

* feat: adding possibility of recording exception * chore: copy * chore: copy * chore: linting * chore: reviews * chore: updating exception type * chore: reviews * chore: reviews * chore: fixing test when waiting for files to be loaded
open-telemetry · Aug 24, 2020 · 2ee9f1a · 2ee9f1a
1 parent fb06b5b
commit 2ee9f1a
Show file tree

Hide file tree

Showing 13 changed files with 233 additions and 27 deletions.
diff --git a/packages/opentelemetry-api/src/common/Exception.ts b/packages/opentelemetry-api/src/common/Exception.ts
@@ -0,0 +1,47 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+interface ExceptionWithCode {
+  code: string;
+  name?: string;
+  message?: string;
+  stack?: string;
+}
+
+interface ExceptionWithMessage {
+  code?: string;
+  message: string;
+  name?: string;
+  stack?: string;
+}
+
+interface ExceptionWithName {
+  code?: string;
+  message?: string;
+  name: string;
+  stack?: string;
+}
+
+/**
+ * Defines Exception.
+ *
+ * string or an object with one of (message or name or code) and optional stack
+ */
+export type Exception =
+  | ExceptionWithCode
+  | ExceptionWithMessage
+  | ExceptionWithName
+  | string;
diff --git a/packages/opentelemetry-api/src/index.ts b/packages/opentelemetry-api/src/index.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+export * from './common/Exception';
 export * from './common/Logger';
 export * from './common/Time';
 export * from './context/propagation/getter';

diff --git a/packages/opentelemetry-api/src/trace/NoopSpan.ts b/packages/opentelemetry-api/src/trace/NoopSpan.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { Exception } from '../common/Exception';
 import { TimeInput } from '../common/Time';
 import { Attributes } from './attributes';
 import { Span } from './span';
@@ -76,6 +77,9 @@ export class NoopSpan implements Span {
   isRecording(): boolean {
     return false;
   }
+
+  // By default does nothing
+  recordException(exception: Exception, time?: TimeInput): void {}
 }
 
 export const NOOP_SPAN = new NoopSpan();
diff --git a/packages/opentelemetry-api/src/trace/span.ts b/packages/opentelemetry-api/src/trace/span.ts
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { Exception } from '../common/Exception';
 import { Attributes } from './attributes';
 import { SpanContext } from './span_context';
 import { Status } from './status';
@@ -114,4 +115,12 @@ export interface Span {
    *     with the `AddEvent` operation and attributes using `setAttributes`.
    */
   isRecording(): boolean;
+
+  /**
+   * Sets exception as a span event
+   * @param exception the exception the only accepted values are string or Error
+   * @param [time] the time to set as Span's event time. If not provided,
+   *     use the current time.
+   */
+  recordException(exception: Exception, time?: TimeInput): void;
 }
diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts
@@ -50,14 +50,17 @@ const mockResError = {
   statusCode: 400,
 };
 
+// send is lazy loading file so need to wait a bit
+const waitTimeMS = 20;
+
 describe('CollectorMetricExporter - node with proto over http', () => {
   let collectorExporter: CollectorMetricExporter;
   let collectorExporterConfig: collectorTypes.CollectorExporterConfigBase;
   let spyRequest: sinon.SinonSpy;
   let spyWrite: sinon.SinonSpy;
   let metrics: MetricRecord[];
   describe('export', () => {
-    beforeEach(done => {
+    beforeEach(() => {
       spyRequest = sinon.stub(http, 'request').returns(fakeRequest as any);
       spyWrite = sinon.stub(fakeRequest, 'write');
       collectorExporterConfig = {
@@ -86,11 +89,6 @@ describe('CollectorMetricExporter - node with proto over http', () => {
       metrics[2].aggregator.update(7);
       metrics[2].aggregator.update(14);
       metrics[3].aggregator.update(5);
-
-      // due to lazy loading ensure to wait to next tick
-      setImmediate(() => {
-        done();
-      });
     });
     afterEach(() => {
       spyRequest.restore();
@@ -108,7 +106,7 @@ describe('CollectorMetricExporter - node with proto over http', () => {
         assert.strictEqual(options.method, 'POST');
         assert.strictEqual(options.path, '/');
         done();
-      });
+      }, waitTimeMS);
     });
 
     it('should set custom headers', done => {
@@ -119,7 +117,7 @@ describe('CollectorMetricExporter - node with proto over http', () => {
         const options = args[0];
         assert.strictEqual(options.headers['foo'], 'bar');
         done();
-      });
+      }, waitTimeMS);
     });
 
     it('should successfully send metrics', done => {
@@ -154,7 +152,7 @@ describe('CollectorMetricExporter - node with proto over http', () => {
         ensureExportMetricsServiceRequestIsSet(json);
 
         done();
-      });
+      }, waitTimeMS);
     });
 
     it('should log the successful message', done => {
@@ -175,7 +173,7 @@ describe('CollectorMetricExporter - node with proto over http', () => {
           assert.strictEqual(responseSpy.args[0][0], 0);
           done();
         });
-      });
+      }, waitTimeMS);
     });
 
     it('should log the error message', done => {
@@ -195,7 +193,7 @@ describe('CollectorMetricExporter - node with proto over http', () => {
           assert.strictEqual(responseSpy.args[0][0], 1);
           done();
         });
-      });
+      }, waitTimeMS);
     });
   });
 });
diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorTraceExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorTraceExporter.test.ts
@@ -44,14 +44,17 @@ const mockResError = {
   statusCode: 400,
 };
 
+// send is lazy loading file so need to wait a bit
+const waitTimeMS = 20;
+
 describe('CollectorExporter - node with proto over http', () => {
   let collectorExporter: CollectorTraceExporter;
   let collectorExporterConfig: collectorTypes.CollectorExporterConfigBase;
   let spyRequest: sinon.SinonSpy;
   let spyWrite: sinon.SinonSpy;
   let spans: ReadableSpan[];
   describe('export', () => {
-    beforeEach(done => {
+    beforeEach(() => {
       spyRequest = sinon.stub(http, 'request').returns(fakeRequest as any);
       spyWrite = sinon.stub(fakeRequest, 'write');
       collectorExporterConfig = {
@@ -67,11 +70,6 @@ describe('CollectorExporter - node with proto over http', () => {
       collectorExporter = new CollectorTraceExporter(collectorExporterConfig);
       spans = [];
       spans.push(Object.assign({}, mockedReadableSpan));
-
-      // due to lazy loading ensure to wait to next tick
-      setImmediate(() => {
-        done();
-      });
     });
     afterEach(() => {
       spyRequest.restore();
@@ -89,7 +87,7 @@ describe('CollectorExporter - node with proto over http', () => {
         assert.strictEqual(options.method, 'POST');
         assert.strictEqual(options.path, '/');
         done();
-      });
+      }, waitTimeMS);
     });
 
     it('should set custom headers', done => {
@@ -100,7 +98,7 @@ describe('CollectorExporter - node with proto over http', () => {
         const options = args[0];
         assert.strictEqual(options.headers['foo'], 'bar');
         done();
-      });
+      }, waitTimeMS);
     });
 
     it('should successfully send the spans', done => {
@@ -121,7 +119,7 @@ describe('CollectorExporter - node with proto over http', () => {
         ensureExportTraceServiceRequestIsSet(json);
 
         done();
-      });
+      }, waitTimeMS);
     });
 
     it('should log the successful message', done => {
@@ -142,7 +140,7 @@ describe('CollectorExporter - node with proto over http', () => {
           assert.strictEqual(responseSpy.args[0][0], 0);
           done();
         });
-      });
+      }, waitTimeMS);
     });
 
     it('should log the error message', done => {
@@ -162,7 +160,7 @@ describe('CollectorExporter - node with proto over http', () => {
           assert.strictEqual(responseSpy.args[0][0], 1);
           done();
         });
-      });
+      }, waitTimeMS);
     });
   });
 });
diff --git a/packages/opentelemetry-plugin-http/package.json b/packages/opentelemetry-plugin-http/package.json
@@ -15,7 +15,8 @@
     "precompile": "tsc --version",
     "version:update": "node ../../scripts/version-update.js",
     "compile": "npm run version:update && tsc -p .",
-    "prepare": "npm run compile"
+    "prepare": "npm run compile",
+    "watch": "tsc -w"
   },
   "keywords": [
     "opentelemetry",

diff --git a/packages/opentelemetry-semantic-conventions/package.json b/packages/opentelemetry-semantic-conventions/package.json
@@ -14,7 +14,8 @@
     "precompile": "tsc --version",
     "version:update": "node ../../scripts/version-update.js",
     "compile": "npm run version:update && tsc -p .",
-    "prepare": "npm run compile"
+    "prepare": "npm run compile",
+    "watch": "tsc -w"
   },
   "keywords": [
     "opentelemetry",

diff --git a/packages/opentelemetry-semantic-conventions/src/trace/exception.ts b/packages/opentelemetry-semantic-conventions/src/trace/exception.ts
@@ -0,0 +1,23 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export const ExceptionAttribute = {
+  MESSAGE: 'exception.message',
+  STACKTRACE: 'exception.stacktrace',
+  TYPE: 'exception.type',
+};
+
+export const ExceptionEventName = 'exception';
diff --git a/packages/opentelemetry-semantic-conventions/src/trace/index.ts b/packages/opentelemetry-semantic-conventions/src/trace/index.ts
@@ -14,8 +14,9 @@
  * limitations under the License.
  */
 
+export * from './database';
+export * from './exception';
 export * from './general';
-export * from './rpc';
 export * from './http';
-export * from './database';
 export * from './os';
+export * from './rpc';
diff --git a/packages/opentelemetry-tracing/package.json b/packages/opentelemetry-tracing/package.json
@@ -77,6 +77,7 @@
     "@opentelemetry/api": "^0.10.2",
     "@opentelemetry/context-base": "^0.10.2",
     "@opentelemetry/core": "^0.10.2",
-    "@opentelemetry/resources": "^0.10.2"
+    "@opentelemetry/resources": "^0.10.2",
+    "@opentelemetry/semantic-conventions": "^0.10.2"
   }
 }
diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts
@@ -23,6 +23,10 @@ import {
   timeInputToHrTime,
 } from '@opentelemetry/core';
 import { Resource } from '@opentelemetry/resources';
+import {
+  ExceptionAttribute,
+  ExceptionEventName,
+} from '@opentelemetry/semantic-conventions';
 import { ReadableSpan } from './export/ReadableSpan';
 import { Tracer } from './Tracer';
 import { SpanProcessor } from './SpanProcessor';
@@ -178,6 +182,35 @@ export class Span implements api.Span, ReadableSpan {
     return true;
   }
 
+  recordException(exception: api.Exception, time: api.TimeInput = hrTime()) {
+    const attributes: api.Attributes = {};
+    if (typeof exception === 'string') {
+      attributes[ExceptionAttribute.MESSAGE] = exception;
+    } else if (exception) {
+      if (exception.code) {
+        attributes[ExceptionAttribute.TYPE] = exception.code;
+      } else if (exception.name) {
+        attributes[ExceptionAttribute.TYPE] = exception.name;
+      }
+      if (exception.message) {
+        attributes[ExceptionAttribute.MESSAGE] = exception.message;
+      }
+      if (exception.stack) {
+        attributes[ExceptionAttribute.STACKTRACE] = exception.stack;
+      }
+    }
+
+    // these are minimum requirements from spec
+    if (
+      attributes[ExceptionAttribute.TYPE] ||
+      attributes[ExceptionAttribute.MESSAGE]
+    ) {
+      this.addEvent(ExceptionEventName, attributes as api.Attributes, time);
+    } else {
+      this._logger.warn(`Failed to record an exception ${exception}`);
+    }
+  }
+
   get duration(): api.HrTime {
     return this._duration;
   }