fix: span attribute count and value limits (#2671) (#2678)

Co-authored-by: Rauno Viskus <[email protected]> Co-authored-by: Daniel Dyla <[email protected]> Co-authored-by: Valentin Marchaud <[email protected]> Co-authored-by: legendecas <[email protected]>
open-telemetry · Jan 12, 2022 · d315b0a · d315b0a
1 parent 82e39c4
commit d315b0a
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 8 deletions.
diff --git a/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts b/packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts
@@ -37,6 +37,7 @@ import { NoopSpanProcessor } from './export/NoopSpanProcessor';
 import { SDKRegistrationConfig, TracerConfig } from './types';
 import { SpanExporter } from './export/SpanExporter';
 import { BatchSpanProcessor } from './platform';
+import { reconfigureLimits } from './utility';
 
 export type PROPAGATOR_FACTORY = () => TextMapPropagator;
 export type EXPORTER_FACTORY = () => SpanExporter;
@@ -73,7 +74,7 @@ export class BasicTracerProvider implements TracerProvider {
   readonly resource: Resource;
 
   constructor(config: TracerConfig = {}) {
-    const mergedConfig = merge({}, DEFAULT_CONFIG, config);
+    const mergedConfig = merge({}, DEFAULT_CONFIG, reconfigureLimits(config));
     this.resource = mergedConfig.resource ?? Resource.empty();
     this.resource = Resource.default().merge(this.resource);
     this._config = Object.assign({}, mergedConfig, {

diff --git a/packages/opentelemetry-sdk-trace-base/src/utility.ts b/packages/opentelemetry-sdk-trace-base/src/utility.ts
@@ -14,8 +14,6 @@
  * limitations under the License.
  */
 
-import { DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT, DEFAULT_ATTRIBUTE_COUNT_LIMIT } from '@opentelemetry/core';
-
 import { Sampler } from '@opentelemetry/api';
 import { buildSamplerFromEnv, DEFAULT_CONFIG } from './config';
 import { SpanLimits, TracerConfig, GeneralLimits } from './types';
@@ -52,21 +50,32 @@ export function mergeConfig(userConfig: TracerConfig): TracerConfig & {
     userConfig.spanLimits || {}
   );
 
+  return target;
+}
+
+/**
+ * When general limits are provided and model specific limits are not,
+ * configures the model specific limits by using the values from the general ones.
+ * @param userConfig User provided tracer configuration
+ */
+export function reconfigureLimits(userConfig: TracerConfig): TracerConfig {
+  const spanLimits = Object.assign({}, userConfig.spanLimits);
+
   /**
    * When span attribute count limit is not defined, but general attribute count limit is defined
    * Then, span attribute count limit will be same as general one
    */
-  if (target.spanLimits.attributeCountLimit === DEFAULT_ATTRIBUTE_COUNT_LIMIT && target.generalLimits.attributeCountLimit !== DEFAULT_ATTRIBUTE_COUNT_LIMIT) {
-    target.spanLimits.attributeCountLimit = target.generalLimits.attributeCountLimit;
+  if (spanLimits.attributeCountLimit == null && userConfig.generalLimits?.attributeCountLimit != null) {
+    spanLimits.attributeCountLimit = userConfig.generalLimits.attributeCountLimit;
   }
 
   /**
    * When span attribute value length limit is not defined, but general attribute value length limit is defined
    * Then, span attribute value length limit will be same as general one
    */
-  if (target.spanLimits.attributeValueLengthLimit === DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT && target.generalLimits.attributeValueLengthLimit !== DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT) {
-    target.spanLimits.attributeValueLengthLimit = target.generalLimits.attributeValueLengthLimit;
+  if (spanLimits.attributeValueLengthLimit == null && userConfig.generalLimits?.attributeValueLengthLimit != null) {
+    spanLimits.attributeValueLengthLimit = userConfig.generalLimits.attributeValueLengthLimit;
   }
 
-  return target;
+  return Object.assign({}, userConfig, { spanLimits });
 }
diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
@@ -23,6 +23,8 @@ import {
   TraceFlags,
 } from '@opentelemetry/api';
 import {
+  DEFAULT_ATTRIBUTE_COUNT_LIMIT,
+  DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
   hrTime,
   hrTimeDuration,
   hrTimeToMilliseconds,
@@ -486,6 +488,38 @@ describe('Span', () => {
         });
       });
 
+      describe('when span "attributeCountLimit" set to the default value and general "attributeCountLimit" option defined', () => {
+        const tracer = new BasicTracerProvider({
+          generalLimits: {
+            // Setting count limit
+            attributeCountLimit: 10,
+          },
+          spanLimits: {
+            attributeCountLimit: DEFAULT_ATTRIBUTE_COUNT_LIMIT,
+          }
+        }).getTracer('default');
+
+        const span = new Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+        for (let i = 0; i < 150; i++) {
+          span.setAttribute('foo' + i, 'bar' + i);
+        }
+        span.end();
+
+        it('should remove / drop all remaining values after the number of values exceeds the span limit', () => {
+          assert.strictEqual(Object.keys(span.attributes).length, DEFAULT_ATTRIBUTE_COUNT_LIMIT);
+          assert.strictEqual(span.attributes['foo0'], 'bar0');
+          assert.strictEqual(span.attributes['foo10'], 'bar10');
+          assert.strictEqual(span.attributes['foo127'], 'bar127');
+          assert.strictEqual(span.attributes['foo128'], undefined);
+        });
+      });
+
       describe('when "attributeValueLengthLimit" option defined', () => {
         const tracer = new BasicTracerProvider({
           generalLimits: {
@@ -528,6 +562,44 @@ describe('Span', () => {
           assert.strictEqual(span.attributes['attr-non-string'], true);
         });
       });
+
+      describe('when span "attributeValueLengthLimit" set to the default value and general "attributeValueLengthLimit" option defined', () => {
+        const tracer = new BasicTracerProvider({
+          generalLimits: {
+            // Setting attribute value length limit
+            attributeValueLengthLimit: 10,
+          },
+          spanLimits: {
+            // Setting attribute value length limit
+            attributeValueLengthLimit: DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
+          },
+        }).getTracer('default');
+
+        const span = new Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+
+        it('should not truncate value', () => {
+          span.setAttribute('attr-with-more-length', 'abcdefghijklmn');
+          assert.strictEqual(span.attributes['attr-with-more-length'], 'abcdefghijklmn');
+        });
+
+        it('should not truncate value of arrays', () => {
+          span.setAttribute('attr-array-of-strings', ['abcdefghijklmn', 'abc', 'abcde', '']);
+          span.setAttribute('attr-array-of-bool', [true, false]);
+          assert.deepStrictEqual(span.attributes['attr-array-of-strings'], ['abcdefghijklmn', 'abc', 'abcde', '']);
+          assert.deepStrictEqual(span.attributes['attr-array-of-bool'], [true, false]);
+        });
+
+        it('should return same value for non-string values', () => {
+          span.setAttribute('attr-non-string', true);
+          assert.strictEqual(span.attributes['attr-non-string'], true);
+        });
+      });
     });
   });