Do not validate tracestate

[dyladan]

Right now we are parsing and validating tracestate on every span regardless of if it is used or not (we currently do not use it for anything except exporting to the collector). This is an unnecessary cost and complexity (parse does a split, reverse, and reduce). Further, we allow arbitrary keys to be overwritten by calling traceState.set despite the fact that we should not be modifying other vendor keys.

Propose that we remove all parsing and validation of tracestate until it is actually modified, and change our default behavior to simply propagate unchanged. Further, set should be changed such that it doesn't parse all values into a map, but simply searches the list for our key/value and splices it out, then sets a new value at the front.

There is currently an open issue on trace context to change the language from MAY to SHOULD NOT validate tracestate w3c/trace-context#384

[pichlermarc]

@dyladan do you think this may be relevant for SDK 2.0? Looks like it would signficantly change behavior.

I think the relevant code is still there:

opentelemetry-js/packages/opentelemetry-core/src/trace/TraceState.ts

Lines 72 to 89 in cd8e77b

	if (rawTraceState.length > MAX_TRACE_STATE_LEN) return;
	this._internalState = rawTraceState
	.split(LIST_MEMBERS_SEPARATOR)
	.reverse() // Store in reverse so new keys (.set(...)) will be placed at the beginning
	.reduce((agg: Map<string, string>, part: string) => {
	const listMember = part.trim(); // Optional Whitespace (OWS) handling
	const i = listMember.indexOf(LIST_MEMBER_KEY_VALUE_SPLITTER);
	if (i !== -1) {
	const key = listMember.slice(0, i);
	const value = listMember.slice(i + 1, part.length);
	if (validateKey(key) && validateValue(value)) {
	agg.set(key, value);
	} else {
	// TODO: Consider to add warning log
	}
	}
	return agg;
	}, new Map());

[david-luna]

@dyladan @pichlermarc I'll have a look into this

[trentm]

Maintainers were discussing this earlier today.

• It was noted that there is an implementation of TraceState in the api package as well: https://github.com/open-telemetry/opentelemetry-js/blob/main/api/src/trace/internal/tracestate-impl.ts This was added in Add TraceState implementation to API opentelemetry-js-api#147 (exported as createTraceState) in 2021 (I think as part of v1.1.0 of the api, not sure.)
• Having two implementations of TraceState is weird, undesirable.
• We need to decide which we want to maintain. Discussion seemed to be leaning towards deprecating the one in the api (we can't remove it because API is stable), and updating the one in core.

For now we decided to punt on this issue being in "SDK 2.0". The changes proposed here are not breaking, so can be done anytime after 2.0.

[trentm]

FWIW, I came back here because this PR implementing the latest sampling spec changes will be using TraceState if head sampling is being used: #5839

(Parsing the tracestate should still be lazy, I think.)