-
Notifications
You must be signed in to change notification settings - Fork 201
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
Applicaiton Insights java agent doesn't work correctly in Quarkus apps (custom attributes or getting SpanId or TraceId) #1980
Comments
I think I found the problem and why the http instrumentation is skipped. In the logs the agent warns that it can't find the class javax.servlet.http.HttpServletRequest
So, it turns out that if you only have as a reference in maven resteasy you don't have a proper Servlet implementation only jaxrs. What I did was add quarkus-undertow as a dependency so the Java Agent can properly find all the dependencies, he needs to instrument the incoming http request. Thanks! |
@guerrerotook Confirmed. I can reproduce the same. The only thing is that in case you add |
hi @guerrerotook @mkowa42 thanks for the repro and investigations! I agree that Application Insights should work without having to modify your application (though I'm very glad you have found a workaround for now). I'm planning to look into this next week to see what we can do to make this experience better, and will post back here. |
Hi @trask, One additional observation regarding outgoing requests (dependency). If an outgoing JAX-RS request is implemented asynchronously. Then again it’s not correlated with the previous events. In my case it's implemented as a JAX-RS / Microprofile Client running inside the same Quarkus based application (which includes as the previously mentioned workaround still the Example JAX-RS/Microprofile client interface: @ApplicationScoped
@RegisterRestClient
@Path("/")
public interface AsyncApi {
@POST
@Path("/path")
@Consumes({"application/json"})
@Produces({"application/json"})
public CompletionStage<MyResponse> performAsync(Payload payload) throws ProcessingException;
} It might be related to Quarkus which is heavily using Vert.x (not only) for implementing the asynchronous calls. |
hey @guerrerotook and @mkowa42, I have sent a fix for this issue upstream to the OpenTelemetry Javaagent, and will pull that fix into our distribution once it's approved and merged upstream, I should be able to provide you with a SNAPSHOT build to test in a couple of days. |
@trask Thanks for letting us know the progress. Does the upstream PR also address the issue for async client requests I mentioned at #1980 (comment)? |
here's a SNAPSHOT build which includes the fix for server correlation: https://github.com/microsoft/ApplicationInsights-Java/suites/4658614061/artifacts/126616732 can you try it out with async client requests, and if it's still an issue, can you post a repro that we can use to debug? |
Hi @trask, great work. The fix is solving both problems. I tested my application after removing the servlet dependency ( Can you already say, when you plan to release a version containing this fix? Many thanks! |
current target is mid-January |
@trask, I saw the todays release of 3.2.5-BETA hoping to see the fix of this issue is part of that release. But it's not. I did also a quick test and it's not working. The snapshot build you have provided in Dec. (#1980 (comment)) worked already fine. Did I miss anything? |
Thank you for letting us know. upstream PR is for opentelemetry repo.. we might not pull that change in yet.. will double check with @trask and let you know soon. We will make a GA release in 2 weeks.. definitely will include it in the GA if not available in our BETA release. |
hi @mkowa42, I just ran 3.2.5-BETA against the repro in this issue and it's working for me. Can you confirm that the repro in this issue works for you? and if so, can you provide a separate repro for the issue you are seeing? thx! |
Hi @trask - thanks for the reply. If you ran the repro than it contains still the workaround as mentioned by @guerrerotook in comment (#1980 (comment)): Refer to https://github.com/guerrerotook/quarkus-telemetry-demo/blob/main/pom.xml#L36-L39 I think if you would try the same after removing the undertow extension you should get still the issue. This wasn't the case with the snapshot build in December (#1980 (comment) ). |
@mkowa42 I don't have that workaround in my local test. Can you confirm that the repro in this issue works for you? and if so, can you provide a separate repro for the issue you are seeing? thx! |
@mkowa42 please let us know, hoping to isolate the issue you are seeing and get that fixed before 3.2.5 GA if possible, thx |
Hi @trask - I downloaded a fresh copy of the 3.2.5-BETA and tested again my code. Now it's working! So it looks that I made a mistake the last time. Thank you again! |
awesome 🎉 |
Expected behavior
Based on the documentation, I should be able to add custom attributes to the current http request in my app. That doesn't work.
Also, in the documentation it being mentioned that I should be able to get the current SpanId or TraceId but it's always zero.
Actual behavior
Telemetry is being sent to Application Insights but without my custom attributes or I can't get the current SpanId or TraceId.
This is the response of the API where I output these lines of code:
This is the output of the API
Hello RESTEasy TraceId 00000000000000000000000000000000 SpanId 0000000000000000
To Reproduce
Create an empty Quarkus app using the RedHat code generator. Add the Application Insights java agent by adding these line of the code in the pom.xml in the section of the build plugin of quarkus maven.
Sample Application
Here you can find the same application https://github.com/guerrerotook/quarkus-telemetry-demo with the binary for the java agent. The only thing missing is a valid Application Insights key.
System information
Please provide the following information:
Logs
As you can see on the logs, the last span exported doesn't include the custom attribute there.
The text was updated successfully, but these errors were encountered: