feat: flagd manual otel interceptor for grpc

[Kavindu-Dodan]

This PR

Fixes #283 by introducing manual tracing [1] for flagd Java sdk

This PR introduce a new constructor to provide an OpenTelemetry SDK. Internally, this is used to intercept grpc communication and add simple span for evaluation.

 OpenFeatureAPI api = OpenFeatureAPI.getInstance();
api.setProvider(new FlagdProvider(optenTelemetry));

With a proper setup, we get distributed telemetry for flagd ecosystem

Note - a follow-up refactoring is ready to improve constructor behavior #294

[1] - https://opentelemetry.io/docs/instrumentation/java/manual/

[thiyagu06]

Just out of curiosity, can't this instrumentation be implemented using hooks?

[thisthat]

Just out of curiosity, can't this instrumentation be implemented using hooks?

Hey @thiyagu06, not really. With the hooks, you cannot instrument the provider to propagate the W3C context.

[thiyagu06]

Just out of curiosity, can't this instrumentation be implemented using hooks?

Hey @thiyagu06, not really. With the hooks, you cannot instrument the provider to propagate the W3C context.

Yes. got it. My concern here is, for an application authors who uses an auto instrumentation of open telemetry, creating an instance of OpenTelemetrySdk and passing it as an constructor argument api.setProvider(new FlagdProvider(telemetrySdk)); doesn't look good.

Instead, I would prefer to obtain the opentelemetry instance by calling GlobalOpenTelemetry.get() in providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java and perform the operation which are needed. any thoughts?

[thisthat]

My concern here is, for an application authors who uses an auto instrumentation of open telemetry, creating an instance of OpenTelemetrySdk and passing it as an constructor argument api.setProvider(new FlagdProvider(telemetrySdk)); doesn't look good.

Instead, I would prefer to obtain the opentelemetry instance by calling GlobalOpenTelemetry.get() in providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java and perform the operation which are needed. any thoughts?

Gotcha! Thanks for noticing it. I agree with you. We should not use the SDK part of OTel here and instead only rely on the API since we are a third party library 👍

[Kavindu-Dodan]

@thisthat & @thiyagu06

I re-checked the documentation and the recommended way it to pass down an instance of OpenTelemetry instead of OpenTelemetrySDK [1]. Usage of GlobalOpenTelemetry is discouraged.

As an aside, if you are writing library instrumentation, it is strongly recommended that you provide your users the ability to inject an instance of OpenTelemetry into your instrumentation code. If this is not possible for some reason, you can fall back to using an instance from the GlobalOpenTelemetry class. Note that you can’t force end-users to configure the global, so this is the most brittle option for library instrumentation.

I will update this and this will allow us to get rid of sdk dependency.

[1] - https://opentelemetry.io/docs/instrumentation/java/manual/#example

[toddbaert]

After reading your docs I think I agree with this solution. Thanks for taking the time.

I think you may want to add some documentation about this to the README, and maybe even some of the links from this comment.

Approving but I'd like to hear once again from @thiyagu06 or @thisthat .

[Kavindu-Dodan]

After reading your docs I think I agree with this solution. Thanks for taking the time.

I think you may want to add some document ion about this to the README, and maybe even some of the links from this comment.

Approving but I'd like to hear once again from @thiyagu06 or @thisthat .

Thank you 🙌

Regarding the documentation, yes agree with the input. I will include an explanation with the next PR #294 where I try to improve the overall usage of constructors :)

[toddbaert]

After reading your docs I think I agree with this solution. Thanks for taking the time.
I think you may want to add some document ion about this to the README, and maybe even some of the links from this comment.
Approving but I'd like to hear once again from @thiyagu06 or @thisthat .

Thank you raised_hands

Regarding the documentation, yes agree with the input. I will include an explanation with the next PR #294 where I try to improve the overall usage of constructors :)

Works for me!