microsoft / ApplicationInsights-Java

Application Insights for Java
http://aka.ms/application-insights
Other
295 stars 200 forks source link

IllegalArgumentException in Tracestate #3874

Open kgusarov opened 1 week ago

kgusarov commented 1 week ago

Expected behavior

Application Insight Agent should be able to parse correct tracestate header where key starts with a digit (https://www.w3.org/TR/trace-context/):

Identifiers MUST begin with a lowercase letter or a digit

Actual behavior

java.lang.IllegalArgumentException: invalid string 9***@dt=f*** in tracestate
    at com.microsoft.applicationinsights.web.internal.correlation.tracecontext.Tracestate.<init>(Tracestate.java:43)

To Reproduce

Set tracestate header where one of keys starts with a digit

System information

Logs

N/A due to security requirements

trask commented 3 days ago

hi @kgusarov! can you include more of the stack trace so we can see where this call is coming from? thanks

kgusarov commented 2 days ago

Hi! Call is coming from our code:

Original Stack Trace:
        at com.microsoft.applicationinsights.web.internal.correlation.tracecontext.Tracestate.<init>(Tracestate.java:43)
        at com.microsoft.applicationinsights.web.internal.RequestTelemetryContext.getTracestate(RequestTelemetryContext.java:23)
        at xxx.yyy.zzz

Long story short - we need to store both tracestate and traceparent values in audit logs.

trask commented 2 days ago

thanks, can you try using opentelemetry-api and using Span.current().getSpanContext().getTraceState() instead?

    <dependency>
      <groupId>io.opentelemetry</groupId>
      <artifactId>opentelemetry-api</artifactId>
      <version>1.42.1</version>
    </dependency>
kgusarov commented 2 days ago

Hi!

Yes, I can try such a scenario but I would also like to be sure that if this doesn't only break our specific use case but also works always. I will probably be able to provide some feedback here tomorrow.

kgusarov commented 12 hours ago

Hi @trask ! By directly utilizing opentelemetry-api I don't see this error. However, I am currently looking into other strange behaviour which might be specific for our application - currently doing some additional research. Question from my side would be if the API that was failing in the beginning is not used somewhere internally?

trask commented 4 hours ago

Question from my side would be if the API that was failing in the beginning is not used somewhere internally?

internally everything has used OpenTelemetry since Application Insights Java version 3.0.0

com.microsoft.applicationinsights.web.internal.correlation.tracecontext.Tracestate is provided for backwards compatibility for people who were directly using that class in Application Insights Java 2.x prior to upgrading to 3.x

kgusarov commented 1 hour ago

These are good news! From our side we need to do some additional testing that might take additional time. Then I guess, I will be able to close this issue.