-
Notifications
You must be signed in to change notification settings - Fork 773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[OTLP exporter] Remove dependency on deprecated gRPC library #4860
[OTLP exporter] Remove dependency on deprecated gRPC library #4860
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4860 +/- ##
==========================================
- Coverage 83.91% 83.87% -0.04%
==========================================
Files 293 293
Lines 12028 12023 -5
==========================================
- Hits 10093 10084 -9
- Misses 1935 1939 +4
|
|
Unsupported scenarios OTLP over GRPC on
I was trying it on Win10 and it is really not working. |
Unfortunately, we want to avoid exposing either |
Yes, as I suspected, definitely a breaking change. We'll discuss in the SIG meeting tomorrow. Whether we drop it now or not we need to begin to consider what our plan is if, in the event the library becomes unmaintained, a security issue is found. |
This may be a moment to bring #2959 into consideration. |
This crossed my mind, however, changing the default to OTLP/HTTP would expand the impact to all users rather than just users running on older versions of Windows. Nonetheless, we'll discuss today. |
@@ -1,6 +1,6 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
<PropertyGroup> | |||
<TargetFrameworks>$(DefaultTargetFrameworks);netstandard2.1</TargetFrameworks> | |||
<TargetFrameworks>$(DefaultTargetFrameworks)</TargetFrameworks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<DefaultTargetFrameworks>net6.0;netstandard2.0;net462</DefaultTargetFrameworks>
For quick/easy ref while reviewing PR
<DefaultTargetFrameworks>net6.0;netstandard2.0;net462</DefaultTargetFrameworks> |
Historically, this repo has dropped support for certain .NET frameworks when the frameworks went out of support itself, and this was done with a minor version bump. This PR is similar to that : OTLP Exporter in .NET Framework needs a particular grpc library, and that library itself is going out of support. Its reasonable for OTel .NET to drop support (and do minor version bump only) as well. (Similar is lack of Prometheus exporter for .NET Framework that can be used in production. There is no underlying library support for that, and we are not building it ourselves...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with this direction. Not marking as explicitly approved, as we need to update changelogs/readme etc. to reflect this.
This change looks good to me as well. It is better to do it sooner so that it gives time to users to take appropriate mitigation steps (use http based version or different OS). An announcement for this change could be a good option when we ship the first pre-release version. |
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
@alanwest, there is new announcement about Grpc.Core package. The maintenance mode is extended about at least one year from now (October 2024). See https://groups.google.com/g/grpc-io/c/iEalUhV4VrU?pli=1 I suppose we can wait a bit before merging these changes. |
Interesting, thanks for the heads up. Given the strong support expressed in removing the dependency, I'd still be a fan of removing it sooner than later giving folks more time to adjust. I am moving this PR to draft for now. I have not had the time yet, but from our discussion in our SIG meeting a few weeks ago, my remaining TODO on this PR was to try adding an integration test for .NET Framework. Even if we ultimately do decide to hold off removing the dependency awhile longer it'll still be nice to have this branch ready for whenever we are. |
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
I have a question about this PR, as October 2024 is soon... Anyone considers reopening this topic? |
Tagging @alanwest |
I think that it was again started by @vishweshbankwar in #5731. I doubt that it will be finished by him. Do you know what is the status of this PR? |
I think there is no plan to reopen/resurrect this PR. The plan is to remove gRPC library dependency entirely as mentioned by @Kielek #4860 (comment). |
I cannot find any updates regarding the end of support extension to October 2024. If we find that it will be extended even further then I do not think there's any rush to remove the dependency to the old gRPC library. Though, if someone can validate that it truly is ending October 2024, then resurrecting this PR could make sense. It would in no way affect the longer term desire to eliminate dependencies on all gRPC libraries. |
Fixes #3421
Fixes #4613
We have discussed recently dropping our dependency on the old gRPC library. Let's use this PR to make a final decision.
The gRPC C# library may technically still be in "maintenance mode", though from the following comment this may change "any day now" grpc/grpc#32719 (comment):
There is a possibility that dropping this dependency will be a breaking change for some .NET Framework users. The newer
Grpc.Net.Client
library does support .NET Framework under certain scenarios (i.e., Windows 11 / Windows Server 2022 using the WinHttpHandler).Until very recently, the newer library
Grpc.Net.Client
did not have anet462
target. To support our .NET Framework users, this would have required us to take a dependency onWinHttpHandler
and to construct the gRPC channel to use it. However,Grpc.Net.Client
as of version 2.56.0 introduced anet462
target which appears to make it "just work" on .NET Framework (still with the Win 11/Server 2022 requirement). See: grpc/grpc-dotnet#2220 (comment)I'm just putting this PR out there to start the conversation. I have not yet had a chance to try it out on .NET Framework to validate that using
Grpc.Net.Client
version 2.56.0 without any special configuration works.