feat(opentelemetry-sdk-trace-base): implemented option to limit lengt…

…h of values of attributes (#2418) * chore(opentelemetry-sdk-trace-base): implemented option to limit length of values of attributes Signed-off-by: Banothu Ramesh Naik <[email protected]> * fix(opentelemetry-sdk-trace-base): pr suggested changes and code improvements Signed-off-by: Banothu Ramesh Naik <[email protected]> * fix(opentelemetry-sdk-trace-base): pr suggested changes Signed-off-by: Banothu Ramesh Naik <[email protected]> * fix(opentelemetry-sdk-trace-base): pr suggested changes on test cases Signed-off-by: Banothu Ramesh Naik <[email protected]> * fix(opentelemetry-sdk-trace-base): pr suggested changes Signed-off-by: Banothu Ramesh Naik <[email protected]> * fix(opentelemetry-sdk-trace-base): pr suggested changes Signed-off-by: Banothu Ramesh Naik <[email protected]> Co-authored-by: Daniel Dyla <[email protected]>
open-telemetry · Aug 23, 2021 · 78a78c0 · 78a78c0
1 parent 5bb9fcc
commit 78a78c0
Show file tree

Hide file tree

Showing 7 changed files with 302 additions and 126 deletions.
diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts
@@ -28,6 +28,7 @@ const ENVIRONMENT_NUMBERS_KEYS = [
   'OTEL_BSP_MAX_EXPORT_BATCH_SIZE',
   'OTEL_BSP_MAX_QUEUE_SIZE',
   'OTEL_BSP_SCHEDULE_DELAY',
+  'OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT',
   'OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT',
   'OTEL_SPAN_EVENT_COUNT_LIMIT',
   'OTEL_SPAN_LINK_COUNT_LIMIT',
@@ -117,6 +118,7 @@ export const DEFAULT_ENVIRONMENT: Required<ENVIRONMENT> = {
   OTEL_PROPAGATORS: ['tracecontext', 'baggage'],
   OTEL_RESOURCE_ATTRIBUTES: '',
   OTEL_SERVICE_NAME: '',
+  OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: Infinity,
   OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 128,
   OTEL_SPAN_EVENT_COUNT_LIMIT: 128,
   OTEL_SPAN_LINK_COUNT_LIMIT: 128,

diff --git a/packages/opentelemetry-core/test/utils/environment.test.ts b/packages/opentelemetry-core/test/utils/environment.test.ts
@@ -84,6 +84,7 @@ describe('environment', () => {
         OTEL_LOG_LEVEL: 'ERROR',
         OTEL_NO_PATCH_MODULES: 'a,b,c',
         OTEL_RESOURCE_ATTRIBUTES: '<attrs>',
+        OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: 100,
         OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: 10,
         OTEL_SPAN_EVENT_COUNT_LIMIT: 20,
         OTEL_SPAN_LINK_COUNT_LIMIT: 30,
@@ -93,6 +94,7 @@ describe('environment', () => {
       const env = getEnv();
       assert.deepStrictEqual(env.OTEL_NO_PATCH_MODULES, ['a', 'b', 'c']);
       assert.strictEqual(env.OTEL_LOG_LEVEL, DiagLogLevel.ERROR);
+      assert.strictEqual(env.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT, 100);
       assert.strictEqual(env.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, 10);
       assert.strictEqual(env.OTEL_SPAN_EVENT_COUNT_LIMIT, 20);
       assert.strictEqual(env.OTEL_SPAN_LINK_COUNT_LIMIT, 30);

diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts
@@ -57,6 +57,7 @@ export class Span implements api.Span, ReadableSpan {
   private _duration: api.HrTime = [-1, -1];
   private readonly _spanProcessor: SpanProcessor;
   private readonly _spanLimits: SpanLimits;
+  private readonly _attributeValueLengthLimit: number;
 
   /** Constructs a new Span instance. */
   constructor(
@@ -80,6 +81,7 @@ export class Span implements api.Span, ReadableSpan {
     this._spanLimits = parentTracer.getSpanLimits();
     this._spanProcessor = parentTracer.getActiveSpanProcessor();
     this._spanProcessor.onStart(this, context);
+    this._attributeValueLengthLimit = this._spanLimits.attributeValueLengthLimit || 0;
   }
 
   spanContext(): api.SpanContext {
@@ -105,7 +107,7 @@ export class Span implements api.Span, ReadableSpan {
     ) {
       return this;
     }
-    this.attributes[key] = value;
+    this.attributes[key] = this._truncateToSize(value);
     return this;
   }
 
@@ -235,4 +237,49 @@ export class Span implements api.Span, ReadableSpan {
     }
     return this._ended;
   }
+
+  // Utility function to truncate given value within size
+  // for value type of string, will truncate to given limit
+  // for type of non-string, will return same value
+  private _truncateToLimitUtil(value: string, limit: number): string {
+    if (value.length <= limit) {
+      return value;
+    }
+    return value.substr(0, limit);
+  }
+
+  /**
+   * If the given attribute value is of type string and has more characters than given {@code attributeValueLengthLimit} then
+   * return string with trucated to {@code attributeValueLengthLimit} characters
+   *
+   * If the given attribute value is array of strings then
+   * return new array of strings with each element truncated to {@code attributeValueLengthLimit} characters
+   *
+   * Otherwise return same Attribute {@code value}
+   *
+   * @param value Attribute value
+   * @returns truncated attribute value if required, otherwise same value
+   */
+  private _truncateToSize(value: SpanAttributeValue): SpanAttributeValue {
+    const limit = this._attributeValueLengthLimit;
+    // Check limit
+    if (limit <= 0) {
+      // Negative values are invalid, so do not truncate
+      api.diag.warn(`Attribute value limit must be positive, got ${limit}`);
+      return value;
+    }
+
+    // String
+    if (typeof value === 'string') {
+      return this._truncateToLimitUtil(value, limit);
+    }
+
+    // Array of strings
+    if (Array.isArray(value)) {
+      return (value as []).map(val => typeof val === 'string' ? this._truncateToLimitUtil(val, limit) : val);
+    }
+
+    // Other types, no need to apply value length limit
+    return value;
+  }
 }
diff --git a/packages/opentelemetry-sdk-trace-base/src/config.ts b/packages/opentelemetry-sdk-trace-base/src/config.ts
@@ -38,6 +38,7 @@ export const DEFAULT_CONFIG = {
   sampler: buildSamplerFromEnv(env),
   forceFlushTimeoutMillis: 30000,
   spanLimits: {
+    attributeValueLengthLimit: getEnv().OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT,
     attributeCountLimit: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
     linkCountLimit: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT,
     eventCountLimit: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT,

diff --git a/packages/opentelemetry-sdk-trace-base/src/types.ts b/packages/opentelemetry-sdk-trace-base/src/types.ts
@@ -63,6 +63,8 @@ export interface SDKRegistrationConfig {
 
 /** Global configuration of trace service */
 export interface SpanLimits {
+  /** attributeValueLengthLimit is maximum allowed attribute value size */
+  attributeValueLengthLimit?: number;
   /** attributeCountLimit is number of attributes per span */
   attributeCountLimit?: number;
   /** linkCountLimit is number of links per span */

diff --git a/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts
@@ -64,74 +64,97 @@ describe('BasicTracerProvider', () => {
   });
 
   describe('constructor', () => {
-    it('should construct an instance without any options', () => {
-      const provider = new BasicTracerProvider();
-      assert.ok(provider instanceof BasicTracerProvider);
-    });
+    describe('when options not defined', () => {
+      it('should construct an instance', () => {
+        const tracer = new BasicTracerProvider();
+        assert.ok(tracer instanceof BasicTracerProvider);
+      });
 
-    it('should construct an instance with sampler', () => {
-      const provider = new BasicTracerProvider({
-        sampler: new AlwaysOnSampler(),
+      it('should use noop span processor by default', () => {
+        const tracer = new BasicTracerProvider();
+        assert.ok(tracer.activeSpanProcessor instanceof NoopSpanProcessor);
       });
-      assert.ok(provider instanceof BasicTracerProvider);
     });
 
-    it('should construct an instance with default span limits', () => {
-      const tracer = new BasicTracerProvider({}).getTracer('default');
-      assert.deepStrictEqual(tracer.getSpanLimits(), {
-        attributeCountLimit: 128,
-        eventCountLimit: 128,
-        linkCountLimit: 128,
+    describe('when "sampler" option defined', () => {
+      it('should have an instance with sampler', () => {
+        const tracer = new BasicTracerProvider({
+          sampler: new AlwaysOnSampler(),
+        });
+        assert.ok(tracer instanceof BasicTracerProvider);
       });
     });
 
-    it('should construct an instance with customized attributeCountLimit span limits', () => {
-      const tracer = new BasicTracerProvider({
-        spanLimits: {
-          attributeCountLimit: 100,
-        },
-      }).getTracer('default');
-      assert.deepStrictEqual(tracer.getSpanLimits(), {
-        attributeCountLimit: 100,
-        eventCountLimit: 128,
-        linkCountLimit: 128,
+    describe('spanLimits', () => {
+      describe('when not defined default values', () => {
+        it('should have tracer with default values', () => {
+          const tracer = new BasicTracerProvider({}).getTracer('default');
+          assert.deepStrictEqual(tracer.getSpanLimits(), {
+            attributeValueLengthLimit: Infinity,
+            attributeCountLimit: 128,
+            eventCountLimit: 128,
+            linkCountLimit: 128,
+          });
+        });
       });
-    });
 
-    it('should construct an instance with customized eventCountLimit span limits', () => {
-      const tracer = new BasicTracerProvider({
-        spanLimits: {
-          eventCountLimit: 300,
-        },
-      }).getTracer('default');
-      assert.deepStrictEqual(tracer.getSpanLimits(), {
-        attributeCountLimit: 128,
-        eventCountLimit: 300,
-        linkCountLimit: 128,
+      describe('when "attributeCountLimit" is defined', () => {
+        it('should have tracer with defined value', () => {
+          const tracer = new BasicTracerProvider({
+            spanLimits: {
+              attributeCountLimit: 100,
+            },
+          }).getTracer('default');
+          const spanLimits = tracer.getSpanLimits();
+          assert.strictEqual(spanLimits.attributeCountLimit, 100);
+        });
       });
-    });
 
-    it('should construct an instance with customized linkCountLimit span limits', () => {
-      const tracer = new BasicTracerProvider({
-        spanLimits: {
-          linkCountLimit: 10,
-        },
-      }).getTracer('default');
-      assert.deepStrictEqual(tracer.getSpanLimits(), {
-        attributeCountLimit: 128,
-        eventCountLimit: 128,
-        linkCountLimit: 10,
+      describe('when "attributeValueLengthLimit" is defined', () => {
+        it('should have tracer with defined value', () => {
+          const tracer = new BasicTracerProvider({
+            spanLimits: {
+              attributeValueLengthLimit: 10,
+            },
+          }).getTracer('default');
+          const spanLimits = tracer.getSpanLimits();
+          assert.strictEqual(spanLimits.attributeValueLengthLimit, 10);
+        });
+
+        it('should have tracer with negative "attributeValueLengthLimit" value', () => {
+          const tracer = new BasicTracerProvider({
+            spanLimits: {
+              attributeValueLengthLimit: -10,
+            },
+          }).getTracer('default');
+          const spanLimits = tracer.getSpanLimits();
+          assert.strictEqual(spanLimits.attributeValueLengthLimit, -10);
+        });
       });
-    });
 
-    it('should construct an instance of BasicTracerProvider', () => {
-      const tracer = new BasicTracerProvider();
-      assert.ok(tracer instanceof BasicTracerProvider);
-    });
+      describe('when "eventCountLimit" is defined', () => {
+        it('should have tracer with defined value', () => {
+          const tracer = new BasicTracerProvider({
+            spanLimits: {
+              eventCountLimit: 300,
+            },
+          }).getTracer('default');
+          const spanLimits = tracer.getSpanLimits();
+          assert.strictEqual(spanLimits.eventCountLimit, 300);
+        });
+      });
 
-    it('should use noop span processor by default', () => {
-      const tracer = new BasicTracerProvider();
-      assert.ok(tracer.activeSpanProcessor instanceof NoopSpanProcessor);
+      describe('when "linkCountLimit" is defined', () => {
+        it('should have tracer with defined value', () => {
+          const tracer = new BasicTracerProvider({
+            spanLimits: {
+              linkCountLimit: 10,
+            },
+          }).getTracer('default');
+          const spanLimits = tracer.getSpanLimits();
+          assert.strictEqual(spanLimits.linkCountLimit, 10);
+        });
+      });
     });
   });