-
Notifications
You must be signed in to change notification settings - Fork 821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(tracing): support for truncatation of span attribute values #1624
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,47 @@ export function isAttributeValue(val: unknown): val is AttributeValue { | |
return isValidPrimitiveAttributeValue(val); | ||
} | ||
|
||
export function truncateValueIfTooLong( | ||
value: AttributeValue, | ||
limit: number | null, | ||
truncationWarningCallback = () => {} | ||
): AttributeValue { | ||
if (limit === null) { | ||
return value; | ||
} | ||
|
||
if (limit < 32) { | ||
throw new Error('Value size limit cannot be lower than 32.'); | ||
} | ||
|
||
if (typeof value === 'boolean' || typeof value === 'number') { | ||
// these types can't exceed the attribute value size limit | ||
return value; | ||
} | ||
|
||
if (Array.isArray(value)) { | ||
// note: this is potentially incompatible with a given exporter | ||
const serialized = JSON.stringify(value); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will be probably refactored to truncate each of the array element individually right ? |
||
|
||
if (serialized.length > limit) { | ||
return truncateValueIfTooLong( | ||
serialized, | ||
limit, | ||
truncationWarningCallback | ||
); | ||
} | ||
|
||
return value; | ||
} | ||
|
||
if (value.length > limit) { | ||
truncationWarningCallback(); | ||
return value.substring(0, limit); | ||
} | ||
|
||
return value; | ||
} | ||
|
||
function isHomogeneousAttributeValueArray(arr: unknown[]): boolean { | ||
let type: string | undefined; | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 { | ||||||
|
@@ -56,6 +57,7 @@ export class Span implements api.Span, ReadableSpan { | |||||
endTime: api.HrTime = [0, 0]; | ||||||
private _ended = false; | ||||||
private _duration: api.HrTime = [-1, -1]; | ||||||
private _hasTruncated = false; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
private readonly _logger: api.Logger; | ||||||
private readonly _spanProcessor: SpanProcessor; | ||||||
private readonly _traceParams: TraceParams; | ||||||
|
@@ -112,7 +114,18 @@ export class Span implements api.Span, ReadableSpan { | |||||
delete this.attributes[attributeKeyToDelete]; | ||||||
} | ||||||
} | ||||||
this.attributes[key] = value; | ||||||
|
||||||
console.log('limit', this._traceParams.spanAttributeValueSizeLimit); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be removed ? |
||||||
this.attributes[key] = truncateValueIfTooLong( | ||||||
value, | ||||||
this._traceParams.spanAttributeValueSizeLimit || null, | ||||||
this._hasTruncated | ||||||
? undefined | ||||||
: () => { | ||||||
this._hasTruncated = true; | ||||||
this._logger.warn(`Span attribute value truncated at key: ${key}.`); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
); | ||||||
return this; | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -55,6 +55,15 @@ export class Tracer implements api.Tracer { | |||||
this.resource = _tracerProvider.resource; | ||||||
this.instrumentationLibrary = instrumentationLibrary; | ||||||
this.logger = config.logger || new ConsoleLogger(config.logLevel); | ||||||
|
||||||
const configuredAttributeLimit = | ||||||
this._traceParams.spanAttributeValueSizeLimit || null; | ||||||
if (configuredAttributeLimit !== null && configuredAttributeLimit < 32) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
this.logger.warn( | ||||||
'OTEL_SPAN_ATTRIBUTE_VALUE_SIZE_LIMIT was set to a value lower than 32, which is not allowed, limit of 32 will be applied.' | ||||||
); | ||||||
this._traceParams.spanAttributeValueSizeLimit = 32; | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should never throw any error, it can warn user but never throw.
with regards to limit I posted a reply on spec issue