fix: sanitize attributes inputs (#2881)

Co-authored-by: Rauno Viskus <Rauno56@users.noreply.github.com>

CHANGELOG.md

@@ -10,6 +10,8 @@ All notable changes to this project will be documented in this file.
 
 ### :bug: (Bug Fix)
 
+* fix: sanitize attributes inputs #2881 @legendecas
+
 ### :books: (Refine Doc)
 
 * docs(sdk): update earliest support node version #2860 @svetlanabrennan

packages/opentelemetry-core/src/common/attributes.ts

@@ -13,30 +13,40 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { SpanAttributeValue, SpanAttributes } from '@opentelemetry/api';
 
-export function sanitizeAttributes(attributes: unknown): SpanAttributes {
-  const out: SpanAttributes = {};
+import { diag, AttributeValue, Attributes } from '@opentelemetry/api';
 
-  if (attributes == null || typeof attributes !== 'object') {
+export function sanitizeAttributes(attributes: unknown): Attributes {
+  const out: Attributes = {};
+
+  if (typeof attributes !== 'object' || attributes == null) {
     return out;
   }
 
-  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-  for (const [k, v] of Object.entries(attributes!)) {
-    if (isAttributeValue(v)) {
-      if (Array.isArray(v)) {
-        out[k] = v.slice();
-      } else {
-        out[k] = v;
-      }
+  for (const [key, val] of Object.entries(attributes)) {
+    if (!isAttributeKey(key)) {
+      diag.warn(`Invalid attribute key: ${key}`);
+      continue;
+    }
+    if (!isAttributeValue(val)) {
+      diag.warn(`Invalid attribute value set for key: ${key}`);
+      continue;
+    }
+    if (Array.isArray(val)) {
+      out[key] = val.slice();
+    } else {
+      out[key] = val;
     }
   }
 
   return out;
 }
 
-export function isAttributeValue(val: unknown): val is SpanAttributeValue {
+export function isAttributeKey(key: unknown): key is string {
+  return typeof key === 'string' && key.length > 0;
+}
+
+export function isAttributeValue(val: unknown): val is AttributeValue {
   if (val == null) {
     return true;
   }

packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts

@@ -15,7 +15,7 @@
  */
 
 import {
-  SpanAttributes,
+  Attributes,
   Context,
   isSpanContextValid,
   Link,
@@ -64,7 +64,7 @@ export class ParentBasedSampler implements Sampler {
     traceId: string,
     spanName: string,
     spanKind: SpanKind,
-    attributes: SpanAttributes,
+    attributes: Attributes,
     links: Link[]
   ): SamplingResult {
     const parentContext = trace.getSpanContext(context);

packages/opentelemetry-exporter-zipkin/src/transform.ts

@@ -66,9 +66,9 @@ export function toZipkinSpan(
   return zipkinSpan;
 }
 
-/** Converts OpenTelemetry SpanAttributes and SpanStatus to Zipkin Tags format. */
+/** Converts OpenTelemetry Attributes and SpanStatus to Zipkin Tags format. */
 export function _toZipkinTags(
-  attributes: api.SpanAttributes,
+  attributes: api.Attributes,
   status: api.SpanStatus,
   statusCodeTagName: string,
   statusErrorTagName: string,

packages/opentelemetry-resources/src/Resource.ts

@@ -68,7 +68,7 @@ export class Resource {
   merge(other: Resource | null): Resource {
     if (!other || !Object.keys(other.attributes).length) return this;
 
-    // SpanAttributes from resource overwrite attributes from other resource.
+    // Attributes from resource overwrite attributes from other resource.
     const mergedAttributes = Object.assign(
       {},
       this.attributes,

packages/opentelemetry-sdk-trace-base/src/Span.ts

@@ -22,6 +22,7 @@ import {
   InstrumentationLibrary,
   isTimeInput,
   timeInputToHrTime,
+  sanitizeAttributes,
 } from '@opentelemetry/core';
 import { Resource } from '@opentelemetry/resources';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
@@ -30,7 +31,7 @@ import { TimedEvent } from './TimedEvent';
 import { Tracer } from './Tracer';
 import { SpanProcessor } from './SpanProcessor';
 import { SpanLimits } from './types';
-import { SpanAttributeValue, Context } from '@opentelemetry/api';
+import { AttributeValue, Context } from '@opentelemetry/api';
 import { ExceptionEventName } from './enums';
 
 /**
@@ -42,7 +43,7 @@ export class Span implements api.Span, ReadableSpan {
   private readonly _spanContext: api.SpanContext;
   readonly kind: api.SpanKind;
   readonly parentSpanId?: string;
-  readonly attributes: api.SpanAttributes = {};
+  readonly attributes: api.Attributes = {};
   readonly links: api.Link[] = [];
   readonly events: TimedEvent[] = [];
   readonly startTime: api.HrTime;
@@ -88,7 +89,7 @@ export class Span implements api.Span, ReadableSpan {
     return this._spanContext;
   }
 
-  setAttribute(key: string, value?: SpanAttributeValue): this;
+  setAttribute(key: string, value?: AttributeValue): this;
   setAttribute(key: string, value: unknown): this {
     if (value == null || this._isSpanEnded()) return this;
     if (key.length === 0) {
@@ -111,7 +112,7 @@ export class Span implements api.Span, ReadableSpan {
     return this;
   }
 
-  setAttributes(attributes: api.SpanAttributes): this {
+  setAttributes(attributes: api.Attributes): this {
     for (const [k, v] of Object.entries(attributes)) {
       this.setAttribute(k, v);
     }
@@ -127,7 +128,7 @@ export class Span implements api.Span, ReadableSpan {
    */
   addEvent(
     name: string,
-    attributesOrStartTime?: api.SpanAttributes | api.TimeInput,
+    attributesOrStartTime?: api.Attributes | api.TimeInput,
     startTime?: api.TimeInput
   ): this {
     if (this._isSpanEnded()) return this;
@@ -148,9 +149,11 @@ export class Span implements api.Span, ReadableSpan {
     if (typeof startTime === 'undefined') {
       startTime = hrTime();
     }
+
+    const attributes = sanitizeAttributes(attributesOrStartTime);
     this.events.push({
       name,
-      attributes: attributesOrStartTime as api.SpanAttributes,
+      attributes,
       time: timeInputToHrTime(startTime),
     });
     return this;
@@ -193,7 +196,7 @@ export class Span implements api.Span, ReadableSpan {
   }
 
   recordException(exception: api.Exception, time: api.TimeInput = hrTime()): void {
-    const attributes: api.SpanAttributes = {};
+    const attributes: api.Attributes = {};
     if (typeof exception === 'string') {
       attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
     } else if (exception) {
@@ -217,7 +220,7 @@ export class Span implements api.Span, ReadableSpan {
       attributes[SemanticAttributes.EXCEPTION_TYPE] ||
       attributes[SemanticAttributes.EXCEPTION_MESSAGE]
     ) {
-      this.addEvent(ExceptionEventName, attributes as api.SpanAttributes, time);
+      this.addEvent(ExceptionEventName, attributes, time);
     } else {
       api.diag.warn(`Failed to record an exception ${exception}`);
     }
@@ -260,7 +263,7 @@ export class Span implements api.Span, ReadableSpan {
    * @param value Attribute value
    * @returns truncated attribute value if required, otherwise same value
    */
-  private _truncateToSize(value: SpanAttributeValue): SpanAttributeValue {
+  private _truncateToSize(value: AttributeValue): AttributeValue {
     const limit = this._attributeValueLengthLimit;
     // Check limit
     if (limit <= 0) {

packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { HrTime, SpanAttributes } from '@opentelemetry/api';
+import { HrTime, Attributes } from '@opentelemetry/api';
 
 /**
  * Represents a timed event.
@@ -25,5 +25,5 @@ export interface TimedEvent {
   /** The name of the event. */
   name: string;
   /** The attributes of the event. */
-  attributes?: SpanAttributes;
+  attributes?: Attributes;
 }

packages/opentelemetry-sdk-trace-base/src/Tracer.ts

@@ -92,7 +92,12 @@ export class Tracer implements api.Tracer {
     }
 
     const spanKind = options.kind ?? api.SpanKind.INTERNAL;
-    const links = options.links ?? [];
+    const links = (options.links ?? []).map(link => {
+      return {
+        context: link.context,
+        attributes: sanitizeAttributes(link.attributes),
+      };
+    });
     const attributes = sanitizeAttributes(options.attributes);
     // make sampling decision
     const samplingResult = this._sampler.shouldSample(
@@ -124,8 +129,10 @@ export class Tracer implements api.Tracer {
       links,
       options.startTime
     );
-    // Set default attributes
-    span.setAttributes(Object.assign(attributes, samplingResult.attributes));
+    // Set initial span attributes. The attributes object may have been mutated
+    // by the sampler, so we sanitize the merged attributes before setting them.
+    const initAttributes = sanitizeAttributes(Object.assign(attributes, samplingResult.attributes));
+    span.setAttributes(initAttributes);
     return span;
   }
 

packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts

@@ -17,7 +17,7 @@
 import {
   SpanKind,
   SpanStatus,
-  SpanAttributes,
+  Attributes,
   HrTime,
   Link,
   SpanContext,
@@ -34,7 +34,7 @@ export interface ReadableSpan {
   readonly startTime: HrTime;
   readonly endTime: HrTime;
   readonly status: SpanStatus;
-  readonly attributes: SpanAttributes;
+  readonly attributes: Attributes;
   readonly links: Link[];
   readonly events: TimedEvent[];
   readonly duration: HrTime;