Skip to content
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

Improve test coverage for HttpClient instrumentation #1734

Open
reyang opened this issue Oct 3, 2023 · 5 comments
Open

Improve test coverage for HttpClient instrumentation #1734

reyang opened this issue Oct 3, 2023 · 5 comments
Assignees
Labels
comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http enhancement New feature or request

Comments

@reyang
Copy link
Member

reyang commented Oct 3, 2023

@CodeBlanch suggested that we should have test coverage for the NuGet version of HttpClient open-telemetry/opentelemetry-dotnet#4912 (comment)

It seems 4.1.0, 4.3.0 and 4.3.4 are still being actively used https://www.nuget.org/stats/packages/System.Net.Http?groupby=Version.

image

@reyang
Copy link
Member Author

reyang commented Oct 3, 2023

@vishweshbankwar could you look into this?

@vishweshbankwar
Copy link
Member

vishweshbankwar commented Oct 3, 2023

dotnet/runtime#86667 Based on discussions on this, I think we should not pursue this. A note in our Readme should be good enough that this scenario is not supported.

@reyang
Copy link
Member Author

reyang commented Oct 7, 2023

dotnet/runtime#86667 Based on discussions on this, I think we should not pursue this. A note in our Readme should be good enough that this scenario is not supported.

@CodeBlanch see if you agree - if yes, close the issue.

@CodeBlanch
Copy link
Member

@ViktorHofer's comment:

...we can't mark the package as deprecated as it is part of the supported netstandard1.x dependency graph.

Seems to be a supported scenario?

@ViktorHofer
Copy link

ViktorHofer commented Oct 9, 2023

The package contains some API that isn't available in earlier .NET Framework versions than 4.7.2 hence we can't deprecate it yet. That said, the majority of .NET Framework customers are already on 4.7.2 or newer. Other still in support TFMs don't require this package and we recommend to remove it from the dependency graph.

Note that the download numbers above are misleading as this package gets automatically downloaded if ANY of the packages in the dependency graph still targets netstandard1.x without providing a newer framework. Usually, that package shouldn't get unless for those earlier .NET Framework versions as the inbox assembly is preferred.

I agree with @vishweshbankwar to indicate that using the package isn't a supported scenario.

@reyang reyang transferred this issue from open-telemetry/opentelemetry-dotnet May 13, 2024
@reyang reyang added the comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants