[ASP.NET Core] GRPC instrumentation support

[vishweshbankwar]

Issue

open-telemetry/opentelemetry-dotnet#5097 removes support for grpc instrumentation. This is done to unblock stable release of instrumentation library which will contain support for http server instrumentation. Semantic conventions for grpc is still in experimental state and hence the grpc instrumentation cannot be marked stable. https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md

Workaround

Going, forward the grpc instrumentation will be supported under an experimental feature flag. https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md#:~:text=Re%2Dintroduced%20support,enabled%20by%20default.

[cijothomas]

Will this be added back in the next non-stable release? Or as experimental feature in stable?

[vishweshbankwar]

The current plan is to bring back in the next non-stable release (Post GA).

[cartermp]

Why not just wait that extra month? There's no rush to push out all the (massively) breaking changes in HTTP instrumentation. I know of zero end-users who even know about this, let alone those who only just read the blog post about HTTP semconv stabilizing in November. And that blog post also only has a tiny number of views despite us trying to get the word out that things were going to change.

[vishweshbankwar]

@cartermp Users have an option to stay on the last released beta version which defaults to older conventions.

[cijothomas]

Why not just wait that extra month?

What benefit does waiting a month gives? gRPC conventions won't still be stable in a month, right?

Maybe we should make an opt-in experimental feature to re-enable the gRPC part, and make it available in the first stable release itself?

[cartermp]

Sorry, I crossed some streams here -- yes, gRPC isn't going stable in a month.

But I guess I don't understand what's going on anymore. If the goal is to stabilize the aspnetcore package, then adding gRPC support back in later makes it not stable anymore? What about new users who want the latest bug fixes (if they exist in the future) for aspnetcore instrumentation, and also want to use gRPC?

[cartermp]

I'll also reiterate that I don't think end-users have any hope at understanding what's happening here

[cijothomas]

If the goal is to stabilize the aspnetcore package, then adding gRPC support back in later makes it not stable anymore?

gRPC support will be under experimental feature flag only. The package will be stable, and offer aspnetcore instrumentation by default. Users who want to get gRPC feature, can opt-in for it, and it can remain opt-in until semantic conventions stabilize.

I think the first stable release should contain the exprimental+opt-in capability to support grpc attributes.

What about new users who want the latest bug fixes (if they exist in the future) for aspnetcore instrumentation, and also want to use gRPC?

Update to the newer versions having the bug fixes. For gRPC, same as my above comment : users can keep the opt-in feature for that.

[cijothomas]

I'll also reiterate that I don't think end-users have any hope at understanding what's happening here

Can't really see how comments like this are useful to anyone!
If you are trying to help, please help with specific feedback, that we can address.

[cartermp]

My specific feedback is just hold off on releasing for a moment. If the plan is to reintroduce gRPC instrumentation, why not just build what you're planning to build and release that? There's three points of churn here:

• gRPC instrumentation being removed entirely
• HTTP semantic conventions causing breaking changes downstream (these have the potential to be enormous in their own right)
• gRPC instrumentation being reintroduced, but as experimental only

Why do three when you can do two?

[cijothomas]

just hold off on releasing for a moment.

How would that help anyone? The 1st stable release of instrumentations are expected in under 2 weeks, so we should move forward and fix gRPC issue either by putting it under experimental feature flag in the stable release, or in the pre-release to be released on same time as stable.

My personal preference is to put gRPC support back under experimental feature flag, and release it as part of the 1st stable release.

[vishweshbankwar]

just hold off on releasing for a moment.

How would that help anyone? The 1st stable release of instrumentations are expected in under 2 weeks, so we should move forward and fix gRPC issue either by putting it under experimental feature flag in the stable release, or in the pre-release to be released on same time as stable.

My personal preference is to put gRPC support back under experimental feature flag, and release it as part of the 1st stable release.

I agree with this approach if gRPC instrumentation is what's needed then we can introduce an experimental flag OTEL_EXPERIMENTAL_ENABLE_GRPC_INSTRUMENTATION (or something like this). Note that users would still have to opt-in for it as opposed to having the instrumentation on by default.

Regarding

HTTP semantic conventions causing breaking changes downstream (these have the potential to be enormous in their own right)

This is an expected churn and was communicated well in advance. We have followed the formal transition plan defined in semantic conventions. All the languages are expected to go through it.

[cijothomas]

@vishweshbankwar Could you start/prepare a PR doing OTEL_EXPERIMENTAL_ENABLE_GRPC_INSTRUMENTATION, and bring it to todays SIG call? It'd be good to get it added and released as rc2 asap, and stick with stable release for dec mid.