open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.68k stars 772 forks source link

TraceState does not check max items when calling set() or serialize() #4979

Open WinterBear opened 3 weeks ago

WinterBear commented 3 weeks ago

Calling the constructor for TraceState correctly limits the number of entries to 32 as specified in https://www.w3.org/TR/trace-context/#tracestate-header-field-values

However the logic is not reapplied when setting or serializing the TraceState , so parsing a TraceState with 32 keys then setting an additional key and calling serialize() produces a string with 33 entries.

let traceState = new TraceState('a=1,b=2,c=3,d=4,e=5,f=6,g=7,h=8,i=9,j=10,k=11,l=12,m=13,n=14,o=15,p=16,' +
'q=17,r=18,s=19,t=20,u=21,v=22,w=23,x=24,y=25,z=26,aa=27,ab=28,ac=29,ad=30,ae=31,af=32,ag=33');

traceState = traceState.set("testkey", "testvalue");
traceState = traceState.set("testkey2", "testvalue2");
traceState = traceState.set("testkey3", "testvalue3");

traceState.serialize(); // Produces testkey3=testvalue3,testkey2=testvalue2,testkey=testvalue,af=32,ae=31,ad=30,ac=29,ab=28,aa=27,z=26,y=25,x=24,w=23,v=22,u=21,t=20,s=19,r=18,q=17,p=16,o=15,n=14,m=13,l=12,k=11,j=10,i=9,h=8,g=7,f=6,e=5,d=4,c=3,b=2,a=1
// 33rd entry is correctly removed during construction, but entries added after this point do not trigger any kind of truncation

I also noticed a couple of other oddities, such as simply constructing, setting a new entry then serializing a TraceState reverses the order of the entries from the original string, and that there does not appear to be any logic to prioritise the removal of entries longer than 128 chars as outlined in https://www.w3.org/TR/trace-context/#tracestate-limits - but these are both less obtrusive problems.