microsoft / ApplicationInsights-Java

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

question - how to instrument Apache Wink Client or older REST clients? #3850

Closed avishnyakov closed 2 months ago

avishnyakov commented 2 months ago

Is your feature request related to a problem? Please describe. Hi team, we use AppInsights agent v3.4.4 with arguments for JVM, without touching legacy java codebase (eg, without adding Java package into codebase).

Works well, but we noticed that Apache Wink Client and some older REST clients don't get instrumentation. That results in "mystical" gaps in timeline for telemetry.

Can you please advice on options to gain visibility in such cases? Looks we need to pass incoming header values + generate new traceid for operation and then inject these into headers for older rest clients, a bit unsure how to do this properly. Eg, this:


// this is Apache Wink Client v1.1.2
RestClient client = new RestClient(clientConfig);
Resource resource = client.resource(uri);

resource.contentType(....)
resource.header('Authorization', ....)

// here we have access to current HTTP context: - Incoming requests come instrumented by AppInsights
// so current web API has access to incoming headers for App Insights
// App Insights instrumentation works well, however, the traceId values are NOT passed further when we make REST calls with Wink client.

// here, Apache Wink Client v1.1.2 is NOT passing incoming headers further, nor AppInsights instruments
// what additional headers we should add, so AppInsights tracing would work?

resource.header(???)
resource.header(???)

// actuall make post request 
resource.post(body);

Describe the solution you would like As above

Describe alternatives you have considered Reading Microsoft doco, could not find clear mapping on which exact header should be captured and then passes further.

Additional context

heyams commented 2 months ago

// App Insights instrumentation works well, however, the traceId values are NOT passed further when we make REST calls with Wink client.

you can wrap the following around making Apache Wink Client rest calls for local context propagation:

try (Scope Ignore = Span.current().makeCurrent()) {
   // making rest call here
}

let me know if that resolves your issue.

avishnyakov commented 2 months ago

@heyams appreciate quick direction, this is awesome.

We deal with legacy codebase, all sorts of things makes adding Java package non-trivial challenge. I'll look into Span.current().makeCurrent() implementation to see how it can be done natively without adding packages.

heyams commented 2 months ago

@avishnyakov here is a sample

https://github.com/Azure-Samples/ApplicationInsights-Java-Samples/blob/15b36b0ea4cf0926582f7bebcd45f745d0028bec/advanced/manual-trace-propagation/src/main/java/com/example/RequestProcessor.java

readme

dependencies you need

avishnyakov commented 2 months ago

Legend! 🚀

Already testing, recon this ticket can be closed.

avishnyakov commented 2 months ago

Sorry to re-open this. Did testing, unfortinaly header aren't getting propogated.

System A


import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Scope;

...

RestClient client = new RestClient(clientConfig);
Resource resource = client.resource()
...

try (Scope Ignore = Span.current().makeCurrent()) {
   resource.header("me-test", "Wink Client v1.1.2 - test"); 
  clientResponse = resource.post(...);
}

System B (where we want to see switching with System A)

avishnyakov commented 2 months ago

Run more testing, it's even more puzzling now - created (1) quick rest hello microservice with Wink 1.1.2-incubating (2) another microservice - AI does instrumentation for these rest calls. This is weird, because same calls in System A -> System B dont't get instrumented.

A bit confused on where challenge could be and how to narrow down problem.

So hello-java rest api

Quick testing for rest calls

    // sprint framework apis

    @GetMapping("/wink-call-get")
    public String getWinkGet() {
      ClientConfig clientConfig = new ClientConfig();
      RestClient client = new RestClient(clientConfig);

      resource.header("me-test", "Wink Client v1.1.2 - test"); 

       ClientResponse response = resource.get("another-microserve-here");

      return "OK/getWinkGet, status: " + response.getStatusCode(); 
    }
heyams commented 2 months ago

@avishnyakov can you share the repro app with me? I didn't find code where you add more header other than "me-test". Without looking at your log, i don't understand what is missing?

alternative, can you try this to add more values to context:

.with can be nested and have more.

 try (Scope scope = io.opentelemetry.context.Context.current() // Get the current context to add values
                .with("<ADDITIONAL_CONTEXT_KEY>", <"ADDITIONAL_CONTEXT_VALUE>") // Add additional context key-value pairs
                .makeCurrent()) {  // A scope is created when we set the new context as current
     // make calls
}
microsoft-github-policy-service[bot] commented 2 months ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.