From d019e2b13678eafc4ac3767499c4960c8311964c Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Mon, 26 Oct 2020 18:33:39 +0100 Subject: [PATCH] feat(tracing): support for truncatation of span attribute values --- .../src/common/attributes.ts | 61 +++++++++++++++- .../test/common/attributes.test.ts | 70 +++++++++++++++++++ packages/opentelemetry-tracing/src/Span.ts | 9 ++- packages/opentelemetry-tracing/src/types.ts | 2 + .../opentelemetry-tracing/test/Span.test.ts | 44 ++++++++++++ 5 files changed, 183 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-core/src/common/attributes.ts b/packages/opentelemetry-core/src/common/attributes.ts index 858a84227fd..dd94d69b424 100644 --- a/packages/opentelemetry-core/src/common/attributes.ts +++ b/packages/opentelemetry-core/src/common/attributes.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { AttributeValue, Attributes } from '@opentelemetry/api'; +import { AttributeValue, Attributes, Logger } from '@opentelemetry/api'; export function sanitizeAttributes(attributes: unknown): Attributes { const out: Attributes = {}; @@ -47,6 +47,65 @@ export function isAttributeValue(val: unknown): val is AttributeValue { return isValidPrimitiveAttributeValue(val); } +export function truncateValueIfTooLong( + value: AttributeValue, + limit: number | undefined, + logger: Logger +): AttributeValue { + if (limit == null) { + return value; + } + + if (typeof value === 'boolean' || typeof value === 'number') { + // these types reasonably can't exceed a limit + // if the limit was set to e.g. 2, then true with a length of 4 will exceed a limit + // however, the purpose of the limit is to prevent accidents such as values being thousands of characters long + return value; + } + + if (Array.isArray(value)) { + let accruedLength = 0; + + // roughly measure the size of a serialized array + // this is done to avoid running json stringify each time attributes are added + for (const element of value) { + if (element == undefined) { + accruedLength += 4; + } else if (typeof element === 'string') { + accruedLength += element.length; + } else { + accruedLength += element.toString().length; + } + + if (accruedLength > limit) { + break; + } + } + + if (accruedLength > limit) { + return truncateValueIfTooLong(JSON.stringify(value), limit, logger); + } else { + return value; + } + } + + if (typeof value !== 'string') { + logger.warn( + `truncateValueIfTooLong expected a value of type string, but received ${typeof value}.` + ); + + // while this is an undesireable situation, if a limit was set, then our goal is to prevent large payloads + return truncateValueIfTooLong(JSON.stringify(value), limit, logger); + } + + if (value.length > limit) { + logger.warn('Value was truncated.'); + return value.substring(0, limit); + } else { + return value; + } +} + function isHomogeneousAttributeValueArray(arr: unknown[]): boolean { let type: string | undefined; diff --git a/packages/opentelemetry-core/test/common/attributes.test.ts b/packages/opentelemetry-core/test/common/attributes.test.ts index b9ed2a1223a..8ab491129bd 100644 --- a/packages/opentelemetry-core/test/common/attributes.test.ts +++ b/packages/opentelemetry-core/test/common/attributes.test.ts @@ -14,10 +14,13 @@ * limitations under the License. */ +import { Logger } from '@opentelemetry/api'; import * as assert from 'assert'; +import * as sinon from 'sinon'; import { isAttributeValue, sanitizeAttributes, + truncateValueIfTooLong, } from '../../src/common/attributes'; describe('attributes', () => { @@ -103,4 +106,71 @@ describe('attributes', () => { assert.strictEqual(attributes.arr[0], 'unmodified'); }); }); + describe('#truncateValueIfTooLong', () => { + it('should not truncate any given value if the limit is not set', () => { + const fakeLogger: Logger = (null as unknown) as Logger; + assert.strictEqual( + truncateValueIfTooLong('a', undefined, fakeLogger), + 'a' + ); + assert.strictEqual(truncateValueIfTooLong(1, undefined, fakeLogger), 1); + assert.strictEqual( + truncateValueIfTooLong(true, undefined, fakeLogger), + true + ); + + const arrayRef: string[] = []; + assert.strictEqual( + truncateValueIfTooLong(arrayRef, undefined, fakeLogger), + arrayRef + ); + }); + + it('passes numbers and bools through regardless of limits', () => { + const fakeLogger: Logger = (null as unknown) as Logger; + assert.strictEqual(truncateValueIfTooLong(true, 3, fakeLogger), true); + assert.strictEqual(truncateValueIfTooLong(false, 3, fakeLogger), false); + assert.strictEqual(truncateValueIfTooLong(100, 3, fakeLogger), 100); + assert.strictEqual(truncateValueIfTooLong(1000, 3, fakeLogger), 1000); + }); + + it('truncates strings if they are longer then the limit', () => { + const fakeLogger: Logger = ({ warn: () => {} } as unknown) as Logger; + assert.strictEqual( + truncateValueIfTooLong('a'.repeat(10), 10, fakeLogger), + 'a'.repeat(10) + ); + assert.strictEqual( + truncateValueIfTooLong('a'.repeat(11), 10, fakeLogger), + 'a'.repeat(10) + ); + assert.strictEqual( + truncateValueIfTooLong('a'.repeat(100), 100, fakeLogger), + 'a'.repeat(100) + ); + assert.strictEqual( + truncateValueIfTooLong('a'.repeat(101), 100, fakeLogger), + 'a'.repeat(100) + ); + }); + + it('truncates arrays if they are longer then the limit', () => { + const fakeLogger: Logger = ({ warn: () => {} } as unknown) as Logger; + assert.strictEqual( + truncateValueIfTooLong(['abcd', 'abcd'], 5, fakeLogger), + '["abc' + ); + assert.strictEqual( + truncateValueIfTooLong([1000, 1000], 5, fakeLogger), + '[1000' + ); + }); + + it('warns if a value was truncated', () => { + const warnSpy = sinon.spy(); + const fakeLogger: Logger = ({ warn: warnSpy } as unknown) as Logger; + truncateValueIfTooLong('a'.repeat(6), 5, fakeLogger); + assert(warnSpy.calledWith('Value was truncated.')); + }); + }); }); diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index 8669f899dc6..c35ead5f13c 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -16,12 +16,13 @@ import * as api from '@opentelemetry/api'; import { - isAttributeValue, hrTime, hrTimeDuration, InstrumentationLibrary, + isAttributeValue, isTimeInput, timeInputToHrTime, + truncateValueIfTooLong, } from '@opentelemetry/core'; import { Resource } from '@opentelemetry/resources'; import { @@ -112,7 +113,11 @@ export class Span implements api.Span, ReadableSpan { delete this.attributes[attributeKeyToDelete]; } } - this.attributes[key] = value; + this.attributes[key] = truncateValueIfTooLong( + value, + this._traceParams.spanAttributeValueSizeLimit, + this._logger + ); return this; } diff --git a/packages/opentelemetry-tracing/src/types.ts b/packages/opentelemetry-tracing/src/types.ts index c90502006ed..7071ed3ecb4 100644 --- a/packages/opentelemetry-tracing/src/types.ts +++ b/packages/opentelemetry-tracing/src/types.ts @@ -74,6 +74,8 @@ export interface TraceParams { numberOfLinksPerSpan?: number; /** numberOfEventsPerSpan is number of message events per span */ numberOfEventsPerSpan?: number; + /** this field defines maximum length of attribute value before it is truncated */ + spanAttributeValueSizeLimit?: number; } /** Interface configuration for a buffer. */ diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index 6a9b2e6f3ef..deb7a70ed7e 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -255,6 +255,50 @@ describe('Span', () => { assert.strictEqual(span.attributes['foo149'], 'bar149'); }); + it('should truncate attribute values exceeding length limit', () => { + const tracerWithLimit = new BasicTracerProvider({ + logger: new NoopLogger(), + traceParams: { + spanAttributeValueSizeLimit: 100, + }, + }).getTracer('default'); + + const spanWithLimit = new Span( + tracerWithLimit, + name, + spanContext, + SpanKind.CLIENT + ); + const spanWithoutLimit = new Span( + tracer, + name, + spanContext, + SpanKind.CLIENT + ); + + spanWithLimit.setAttribute('attr under limit', 'a'.repeat(100)); + assert.strictEqual( + spanWithLimit.attributes['attr under limit'], + 'a'.repeat(100) + ); + spanWithoutLimit.setAttribute('attr under limit', 'a'.repeat(100)); + assert.strictEqual( + spanWithoutLimit.attributes['attr under limit'], + 'a'.repeat(100) + ); + + spanWithLimit.setAttribute('attr over limit', 'b'.repeat(101)); + assert.strictEqual( + spanWithLimit.attributes['attr over limit'], + 'b'.repeat(100) + ); + spanWithoutLimit.setAttribute('attr over limit', 'b'.repeat(101)); + assert.strictEqual( + spanWithoutLimit.attributes['attr over limit'], + 'b'.repeat(101) + ); + }); + it('should set an error status', () => { const span = new Span(tracer, name, spanContext, SpanKind.CLIENT); span.setStatus({