chore: disallow null attribute values (#1555)

open-telemetry · Sep 28, 2020 · e298eec · e298eec
1 parent 7831bd6
commit e298eec
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 13 deletions.
diff --git a/packages/opentelemetry-api/src/trace/attributes.ts b/packages/opentelemetry-api/src/trace/attributes.ts
@@ -15,12 +15,10 @@
  */
 
 export interface Attributes {
-  [attributeKey: string]: AttributeValue;
+  [attributeKey: string]: AttributeValue | undefined;
 }
 
 export type AttributeValue =
-  | undefined
-  | null
   | string
   | number
   | boolean

diff --git a/packages/opentelemetry-api/src/trace/span.ts b/packages/opentelemetry-api/src/trace/span.ts
@@ -49,7 +49,7 @@ export interface Span {
    * @param key the key for this attribute.
    * @param value the value for this attribute.
    */
-  setAttribute(key: string, value: AttributeValue): this;
+  setAttribute(key: string, value?: AttributeValue): this;
 
   /**
    * Sets attributes to the span.

diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts
@@ -88,9 +88,9 @@ export class Span implements api.Span, ReadableSpan {
     return this.spanContext;
   }
 
-  setAttribute(key: string, value: AttributeValue): this;
+  setAttribute(key: string, value?: AttributeValue): this;
   setAttribute(key: string, value: unknown): this {
-    if (this._isSpanEnded()) return this;
+    if (value == null || this._isSpanEnded()) return this;
     if (key.length === 0) {
       this._logger.warn(`Invalid attribute key: ${key}`);
       return this;
@@ -100,11 +100,6 @@ export class Span implements api.Span, ReadableSpan {
       return this;
     }
 
-    if (value == null) {
-      delete this.attributes[key];
-      return this;
-    }
-
     if (
       Object.keys(this.attributes).length >=
       this._traceParams.numberOfAttributesPerSpan!

diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts
@@ -182,9 +182,7 @@ describe('Span', () => {
     const span = new Span(tracer, name, spanContext, SpanKind.CLIENT);
 
     span.setAttribute('overwrite', 'initial value');
-    span.setAttribute('remove', 'initial value');
     span.setAttribute('overwrite', 'overwritten value');
-    span.setAttribute('remove', null);
 
     assert.deepStrictEqual(span.attributes, {
       overwrite: 'overwritten value',