Skip to content

Use parent Call.Factory of OkHttpClient to be able to use OpenTelemetry - #1590

Closed
Cees Bos (cbos) wants to merge 1 commit into
microsoftgraph:devfrom
cbos:prepare_for_opentelemetry
Closed

Use parent Call.Factory of OkHttpClient to be able to use OpenTelemetry#1590
Cees Bos (cbos) wants to merge 1 commit into
microsoftgraph:devfrom
cbos:prepare_for_opentelemetry

Conversation

@cbos

@cbos Cees Bos (cbos) commented Oct 25, 2023

Copy link
Copy Markdown

See microsoftgraph/msgraph-sdk-java-core#1252
This change depends on that change.


For application insights it is good to be able to get insights in all incoming and outgoing requests.
OpenTelemetry is the standard for that.

I tried to use https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/okhttp/okhttp-3.0/library/README.md

  private Call.Factory createTracedClient(OpenTelemetry openTelemetry, @Nonnull final IAuthenticationProvider auth) {
        return OkHttpTelemetry.builder(openTelemetry).build().newCallFactory(createClient(auth));
    }

  private OkHttpClient createClient(@Nonnull final IAuthenticationProvider auth) {
        return HttpClients.createDefault(auth);
    }

// then create the GraphServiceClient
IAuthenticationProvider authenticationProvider = requestUrl -> cf;
GraphServiceClient
                .builder(Call.Factory.class, Request.class)
                .httpClient(createTracedClient(openTelemetry, authenticationProvider))
                .authenticationProvider(authenticationProvider)
                .buildClient();

This gives an exception at runtime

Caused by: java.lang.ClassCastException: class io.opentelemetry.instrumentation.okhttp.v3_0.TracingCallFactory cannot be cast to class okhttp3.OkHttpClient (io.opentelemetry.instrumentation.okhttp.v3_0.TracingCallFactory and okhttp3.OkHttpClient are in unnamed module of loader io.quarkus.bootstrap.classloading.QuarkusClassLoader @4df50bcc)
        at com.microsoft.graph.core.BaseClient$Builder.getHttpProvider(BaseClient.java:188)
        at com.microsoft.graph.core.BaseClient$Builder.buildClient(BaseClient.java:275)
        at com.microsoft.graph.requests.GraphServiceClient$Builder.buildClient(GraphServiceClient.java:153)

But there is no actual use of casting it to OkHttpClient, only the methods of Call.Factory are used.

As that is the minimal base required, I changed the code as well that in the generic it requires as least Call.Factory as base.

…ry (dependent on change in msgraph-sdk-java-core)
@cbos
Cees Bos (cbos) requested a review from a team as a code owner October 25, 2023 09:18

@baywet Vincent Biret (baywet) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering the remarks I've made in the core pull request, I don't think we need this pull request anymore.

@cbos

Copy link
Copy Markdown
Author

Considering the remarks I've made in the core pull request, I don't think we need this pull request anymore.

Correct, with the requested change this PR is no needed anymore.
But as soon as the core has a new release the dependency has to be updated, but I guess that part of the normal release cycle.

As mentioned on the PR, with these change OpenTelemetry can be used with MSGraph SDK to get traces for the requests done with MSGraph SDK library.

@cbos
Cees Bos (cbos) deleted the prepare_for_opentelemetry branch October 25, 2023 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants