search

openzipkin / zipkin-reporter-java

Shared library for reporting zipkin spans on transports such as http or kafka
Apache License 2.0
126 stars 70 forks source link

allow passing client to sender builder #200

Closed arun0009 closed 3 years ago

arun0009 commented 3 years ago

Allow passing custom OkHttpClient to OkHttpSender builder so we can configure a proxy, httpLoggingInterceptor, etc on OkHttpClient.

jorgheymans commented 3 years ago

Is there an overlap with #195 here ? How would an upgrade to okhttp4 impact this ? It seems they have gone to great length to stay compatible but there are some differences. https://square.github.io/okhttp/upgrading_to_okhttp_4/

arun0009 commented 3 years ago

It's not an overlap, with current implementation irrelevant of OkHttp version, I can't configure OkHttpClient with for e.g. proxy etc.

arun0009 commented 3 years ago

@anuraaga - can you please review?

oldNoakes commented 3 years ago

Giving this PR a nudge - unless there is a different way of supporting proxied reporting that doesn't use environment level setup. Thanks.

shakuzen commented 3 years ago

Making customizations to the OkHttp client builder should already be possible and an example is given in the JavaDoc already. For the example of configuring a proxy, here's some code that should work:

Proxy proxy = new Proxy(Proxy.Type.HTTP, new InetSocketAddress("proxy.company.com", 8099));
OkHttpSender.Builder okHttpSenderBuilder = OkHttpSender.newBuilder();
okHttpSenderBuilder.clientBuilder().proxy(proxy);
OkHttpSender sender = okHttpSenderBuilder.build();

We test such customizations work in this test. Am I missing something? It doesn't seem like we need new API for the use cases mentioned.