From 78a78c093c2df24b66c47af4e037da9a6098fedb Mon Sep 17 00:00:00 2001 From: Banothu Ramesh Naik Date: Tue, 24 Aug 2021 03:07:50 +0530 Subject: [PATCH] feat(opentelemetry-sdk-trace-base): implemented option to limit length 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 * fix(opentelemetry-sdk-trace-base): pr suggested changes and code improvements Signed-off-by: Banothu Ramesh Naik * fix(opentelemetry-sdk-trace-base): pr suggested changes Signed-off-by: Banothu Ramesh Naik * fix(opentelemetry-sdk-trace-base): pr suggested changes on test cases Signed-off-by: Banothu Ramesh Naik * fix(opentelemetry-sdk-trace-base): pr suggested changes Signed-off-by: Banothu Ramesh Naik * fix(opentelemetry-sdk-trace-base): pr suggested changes Signed-off-by: Banothu Ramesh Naik Co-authored-by: Daniel Dyla --- .../src/utils/environment.ts | 2 + .../test/utils/environment.test.ts | 2 + .../opentelemetry-sdk-trace-base/src/Span.ts | 49 +++- .../src/config.ts | 1 + .../opentelemetry-sdk-trace-base/src/types.ts | 2 + .../test/common/BasicTracerProvider.test.ts | 131 ++++++---- .../test/common/Span.test.ts | 241 ++++++++++++------ 7 files changed, 302 insertions(+), 126 deletions(-) diff --git a/packages/opentelemetry-core/src/utils/environment.ts b/packages/opentelemetry-core/src/utils/environment.ts index 78b7f1f248..b6e9dc5db4 100644 --- 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 = { 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 index a993f10083..3f3a709b62 100644 --- 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: '', + 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 index 8ffad7ab0a..32226332dc 100644 --- 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 index 433a0161da..38c869cfa7 100644 --- 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 index d8329aa567..91a330d9c9 100644 --- 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 index f824049a84..220b71d6cf 100644 --- 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); + }); + }); }); }); diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts index bd2072bf00..21e5eb37ff 100644 --- a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts +++ b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts @@ -37,6 +37,7 @@ const performanceTimeOrigin = hrTime(); describe('Span', () => { const tracer = new BasicTracerProvider({ spanLimits: { + attributeValueLengthLimit: 100, attributeCountLimit: 100, eventCountLimit: 100, }, @@ -213,83 +214,186 @@ describe('Span', () => { }); }); - it('should set an attribute', () => { - const span = new Span( - tracer, - ROOT_CONTEXT, - name, - spanContext, - SpanKind.CLIENT - ); + describe('setAttribute', () => { + describe('when default options set', () => { + it('should set an attribute', () => { + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); + + span.setAttribute('string', 'string'); + span.setAttribute('number', 0); + span.setAttribute('bool', true); + span.setAttribute('array', ['str1', 'str2']); + span.setAttribute('array', [1, 2]); + span.setAttribute('array', [true, false]); + + //@ts-expect-error invalid attribute type object + span.setAttribute('object', { foo: 'bar' }); + //@ts-expect-error invalid attribute inhomogenous array + span.setAttribute('non-homogeneous-array', [0, '']); + // This empty length attribute should not be set + span.setAttribute('', 'empty-key'); + + assert.deepStrictEqual(span.attributes, { + string: 'string', + number: 0, + bool: true, + 'array': ['str1', 'str2'], + 'array': [1, 2], + 'array': [true, false], + }); + }); - span.setAttribute('string', 'string'); - span.setAttribute('number', 0); - span.setAttribute('bool', true); - span.setAttribute('array', ['str1', 'str2']); - span.setAttribute('array', [1, 2]); - span.setAttribute('array', [true, false]); + it('should be able to overwrite attributes', () => { + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); - //@ts-expect-error invalid attribute type object - span.setAttribute('object', { foo: 'bar' }); - //@ts-expect-error invalid attribute inhomogenous array - span.setAttribute('non-homogeneous-array', [0, '']); + span.setAttribute('overwrite', 'initial value'); + span.setAttribute('overwrite', 'overwritten value'); - assert.deepStrictEqual(span.attributes, { - string: 'string', - number: 0, - bool: true, - 'array': ['str1', 'str2'], - 'array': [1, 2], - 'array': [true, false], + assert.deepStrictEqual(span.attributes, { + overwrite: 'overwritten value', + }); + }); }); - }); - it('should overwrite attributes', () => { - const span = new Span( - tracer, - ROOT_CONTEXT, - name, - spanContext, - SpanKind.CLIENT - ); + describe('when spanLimits options set', () => { + describe('when "attributeCountLimit" option defined', () => { + const tracer = new BasicTracerProvider({ + spanLimits: { + // Setting count limit + attributeCountLimit: 100, + }, + }).getTracer('default'); - span.setAttribute('overwrite', 'initial value'); - span.setAttribute('overwrite', 'overwritten value'); + 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 this limit', () => { + assert.strictEqual(Object.keys(span.attributes).length, 100); + assert.strictEqual(span.attributes['foo0'], 'bar0'); + assert.strictEqual(span.attributes['foo99'], 'bar99'); + assert.strictEqual(span.attributes['foo149'], undefined); + }); + }); - assert.deepStrictEqual(span.attributes, { - overwrite: 'overwritten value', + describe('when "attributeValueLengthLimit" option defined', () => { + const tracer = new BasicTracerProvider({ + spanLimits: { + // Setting attribute value length limit + attributeValueLengthLimit: 5, + }, + }).getTracer('default'); + + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); + + it('should truncate value which length exceeds this limit', () => { + span.setAttribute('attr-with-more-length', 'abcdefgh'); + assert.strictEqual(span.attributes['attr-with-more-length'], 'abcde'); + }); + + it('should truncate value of arrays which exceeds this limit', () => { + span.setAttribute('attr-array-of-strings', ['abcdefgh', 'abc', 'abcde', '']); + span.setAttribute('attr-array-of-bool', [true, false]); + assert.deepStrictEqual(span.attributes['attr-array-of-strings'], ['abcde', 'abc', 'abcde', '']); + assert.deepStrictEqual(span.attributes['attr-array-of-bool'], [true, false]); + }); + + it('should not truncate value which length not exceeds this limit', () => { + span.setAttribute('attr-with-less-length', 'abc'); + assert.strictEqual(span.attributes['attr-with-less-length'], 'abc'); + }); + + it('should return same value for non-string values', () => { + span.setAttribute('attr-non-string', true); + assert.strictEqual(span.attributes['attr-non-string'], true); + }); + }); + + describe('when "attributeValueLengthLimit" option is invalid', () => { + const tracer = new BasicTracerProvider({ + spanLimits: { + // Setting invalid attribute value length limit + attributeValueLengthLimit: -5, + }, + }).getTracer('default'); + + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); + + it('should not truncate any value', () => { + span.setAttribute('attr-not-truncate', 'abcdefgh'); + span.setAttribute('attr-array-of-strings', ['abcdefgh', 'abc', 'abcde']); + assert.deepStrictEqual(span.attributes['attr-not-truncate'], 'abcdefgh'); + assert.deepStrictEqual(span.attributes['attr-array-of-strings'], ['abcdefgh', 'abc', 'abcde']); + }); + }); }); }); - it('should set attributes', () => { - const span = new Span( - tracer, - ROOT_CONTEXT, - name, - spanContext, - SpanKind.CLIENT - ); + describe('setAttributes', () => { + it('should be able to set multiple attributes', () => { + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); - span.setAttributes({ - string: 'string', - number: 0, - bool: true, - 'array': ['str1', 'str2'], - 'array': [1, 2], - 'array': [true, false], - //@ts-expect-error invalid attribute type object - object: { foo: 'bar' }, - //@ts-expect-error invalid attribute inhomogenous array - 'non-homogeneous-array': [0, ''], - }); + span.setAttributes({ + string: 'string', + number: 0, + bool: true, + 'array': ['str1', 'str2'], + 'array': [1, 2], + 'array': [true, false], + //@ts-expect-error invalid attribute type object + object: { foo: 'bar' }, + //@ts-expect-error invalid attribute inhomogenous array + 'non-homogeneous-array': [0, ''], + // This empty length attribute should not be set + '': 'empty-key', + }); - assert.deepStrictEqual(span.attributes, { - string: 'string', - number: 0, - bool: true, - 'array': ['str1', 'str2'], - 'array': [1, 2], - 'array': [true, false], + assert.deepStrictEqual(span.attributes, { + string: 'string', + number: 0, + bool: true, + 'array': ['str1', 'str2'], + 'array': [1, 2], + 'array': [true, false], + }); }); }); @@ -330,7 +434,7 @@ describe('Span', () => { span.end(); }); - it('should drop extra attributes and events', () => { + it('should drop extra events', () => { const span = new Span( tracer, ROOT_CONTEXT, @@ -339,17 +443,12 @@ describe('Span', () => { SpanKind.CLIENT ); for (let i = 0; i < 150; i++) { - span.setAttribute('foo' + i, 'bar' + i); span.addEvent('sent' + i); } span.end(); assert.strictEqual(span.events.length, 100); - assert.strictEqual(Object.keys(span.attributes).length, 100); assert.strictEqual(span.events[span.events.length - 1].name, 'sent149'); - assert.strictEqual(span.attributes['foo0'], 'bar0'); - assert.strictEqual(span.attributes['foo99'], 'bar99'); - assert.strictEqual(span.attributes['sent100'], undefined); }); it('should set an error status', () => {