feat(shim-opentracing): update setTag based on new spec (#2194)

Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
open-telemetry · Jun 2, 2021 · 1d65b3e · 1d65b3e
1 parent 8bb9752
commit 1d65b3e
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 14 deletions.
diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts
@@ -19,6 +19,7 @@ import * as opentracing from 'opentracing';
 import {
   SpanAttributes,
   SpanAttributeValue,
+  SpanStatusCode,
   TextMapPropagator,
 } from '@opentelemetry/api';
 
@@ -298,7 +299,14 @@ export class SpanShim extends opentracing.Span {
    * @param keyValueMap set of KV pairs representing tags
    */
   override addTags(keyValueMap: SpanAttributes): this {
-    this._span.setAttributes(keyValueMap);
+    for (const [key, value] of Object.entries(keyValueMap)) {
+      if (this._setErrorAsSpanStatusCode(key, value)) {
+        continue;
+      }
+      if (value !== undefined) {
+        this._span.setAttribute(key, value);
+      }
+    }
     return this;
   }
 
@@ -309,11 +317,7 @@ export class SpanShim extends opentracing.Span {
    * @param value value for the tag
    */
   override setTag(key: string, value: SpanAttributeValue): this {
-    if (
-      key === opentracing.Tags.ERROR &&
-      (value === true || value === 'true')
-    ) {
-      this._span.setStatus({ code: api.SpanStatusCode.ERROR });
+    if (this._setErrorAsSpanStatusCode(key, value)) {
       return this;
     }
 
@@ -330,13 +334,40 @@ export class SpanShim extends opentracing.Span {
     return this;
   }
 
-  /*
-   * Returns the underlying {@link types.Span} that the shim
+  /**
+   * Returns the underlying {@link api.Span} that the shim
    * is wrapping.
    */
   getSpan(): api.Span {
     return this._span;
   }
+
+  private _setErrorAsSpanStatusCode(
+    key: string,
+    value: SpanAttributeValue | undefined
+  ): boolean {
+    if (key === opentracing.Tags.ERROR) {
+      const statusCode = SpanShim._mapErrorTag(value);
+      this._span.setStatus({ code: statusCode });
+      return statusCode !== SpanStatusCode.UNSET;
+    }
+    return false;
+  }
+
+  private static _mapErrorTag(
+    value: SpanAttributeValue | undefined
+  ): SpanStatusCode {
+    switch (value) {
+      case true:
+      case 'true':
+        return SpanStatusCode.ERROR;
+      case false:
+      case 'false':
+        return SpanStatusCode.OK;
+      default:
+        return SpanStatusCode.UNSET;
+    }
+  }
 }
 
 /**

diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
@@ -30,6 +30,7 @@ import {
   INVALID_SPAN_CONTEXT,
   propagation,
   ROOT_CONTEXT,
+  SpanStatusCode,
   trace,
 } from '@opentelemetry/api';
 import { performance } from 'perf_hooks';
@@ -291,13 +292,67 @@ describe('OpenTracing Shim', () => {
       otSpan = (span as SpanShim).getSpan() as Span;
     });
 
-    it('sets tags', () => {
-      span.setTag('hello', 'world');
-      assert.strictEqual(otSpan.attributes.hello, 'world');
+    describe('tags', () => {
+      it('sets tags', () => {
+        span.setTag('hello', 'world');
+        assert.strictEqual(otSpan.attributes.hello, 'world');
 
-      span.addTags({ hello: 'stars', from: 'earth' });
-      assert.strictEqual(otSpan.attributes.hello, 'stars');
-      assert.strictEqual(otSpan.attributes.from, 'earth');
+        span.addTags({ hello: 'stars', from: 'earth' });
+        assert.strictEqual(otSpan.attributes.hello, 'stars');
+        assert.strictEqual(otSpan.attributes.from, 'earth');
+      });
+
+      it('ignores undefined tags', () => {
+        span.addTags({ hello: 'stars', from: undefined });
+        assert.deepStrictEqual(otSpan.attributes, { hello: 'stars' });
+      });
+
+      it('maps error tag to status code', () => {
+        span.setTag('error', '');
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.UNSET);
+
+        span.setTag('error', true);
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.ERROR);
+
+        span.setTag('error', false);
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.OK);
+
+        span.setTag('error', 'true');
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.ERROR);
+
+        span.setTag('error', 'false');
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.OK);
+      });
+
+      it('sets unknown error tag as attribute', () => {
+        span.setTag('error', 'whoopsie');
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.UNSET);
+        assert.strictEqual(otSpan.attributes.error, 'whoopsie');
+      });
+
+      it('maps error tag to status code when adding multiple tags', () => {
+        span.addTags({ hello: 'stars', error: '' });
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.UNSET);
+
+        span.addTags({ hello: 'stars', error: true });
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.ERROR);
+
+        span.addTags({ hello: 'stars', error: false });
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.OK);
+
+        span.addTags({ hello: 'stars', error: 'true' });
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.ERROR);
+
+        span.addTags({ hello: 'stars', error: 'false' });
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.OK);
+      });
+
+      it('sets unknown error tag as attribute when adding multiple tags', () => {
+        span.addTags({ hello: 'stars', error: 'whoopsie' });
+        assert.strictEqual(otSpan.status.code, SpanStatusCode.UNSET);
+        assert.strictEqual(otSpan.attributes.hello, 'stars');
+        assert.strictEqual(otSpan.attributes.error, 'whoopsie');
+      });
     });
 
     it('logs KV pairs', () => {