From 873d6ab15433f1cfbffe70c1e91dc8853f643feb Mon Sep 17 00:00:00 2001
From: Vera Reynolds <verareynolds@honeycomb.io>
Date: Mon, 10 May 2021 19:56:34 -0600
Subject: [PATCH] feat(shim-opentracing): map error tag to status code in
 addTags

---
 .../src/shim.ts                               | 33 ++++++++++++++-----
 .../test/Shim.test.ts                         | 29 ++++++++++++++++
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts
index 582b9bb8964..6f27d29bbe8 100644
--- a/packages/opentelemetry-shim-opentracing/src/shim.ts
+++ b/packages/opentelemetry-shim-opentracing/src/shim.ts
@@ -300,7 +300,14 @@ export class SpanShim extends opentracing.Span {
    * @param keyValueMap set of KV pairs representing tags
    */
   addTags(keyValueMap: SpanAttributes): this {
-    this._span.setAttributes(keyValueMap);
+    for (const [key, value] of Object.entries(keyValueMap)) {
+      if (this._setSpanStatusCode(key, value)) {
+        continue;
+      }
+      if (value !== undefined) {
+        this._span.setAttribute(key, value);
+      }
+    }
     return this;
   }
 
@@ -311,12 +318,8 @@ export class SpanShim extends opentracing.Span {
    * @param value value for the tag
    */
   setTag(key: string, value: SpanAttributeValue): this {
-    if (key === opentracing.Tags.ERROR) {
-      const statusCode = SpanShim._mapErrorTag(value);
-      this._span.setStatus({ code: statusCode });
-      if (statusCode !== SpanStatusCode.UNSET) {
-        return this;
-      }
+    if (this._setSpanStatusCode(key, value)) {
+      return this;
     }
 
     this._span.setAttribute(key, value);
@@ -340,7 +343,21 @@ export class SpanShim extends opentracing.Span {
     return this._span;
   }
 
-  private static _mapErrorTag(value: SpanAttributeValue): SpanStatusCode {
+  private _setSpanStatusCode(
+    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':
diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
index 1f8309fba74..3708d95b78f 100644
--- a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
+++ b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
@@ -304,6 +304,11 @@ describe('OpenTracing Shim', () => {
         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);
@@ -326,6 +331,30 @@ describe('OpenTracing Shim', () => {
         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', () => {