-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: Use HttpWebRequest instead of HttpClient on .NET Framework #1853
feat: Use HttpWebRequest instead of HttpClient on .NET Framework #1853
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1853 +/- ##
==========================================
- Coverage 83.57% 83.40% -0.17%
==========================================
Files 389 402 +13
Lines 24504 24654 +150
==========================================
+ Hits 20478 20563 +85
- Misses 4026 4091 +65
|
…rd builds, conditional compilation, comments
src/Agent/NewRelic/Agent/Core/DataTransport/Client/HttpClientBase.cs
Outdated
Show resolved
Hide resolved
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.
Looks good! Very clean
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.
Great work!
src/Agent/NewRelic/Agent/Core/DataTransport/Client/NRWebRequestClient.cs
Outdated
Show resolved
Hide resolved
tests/Agent/UnitTests/Core.UnitTest/DataTransport/HttpCollectorWireWebRequestTests.cs
Outdated
Show resolved
Hide resolved
It looks like the tests caught a potential problem with the new code. https://github.com/newrelic/newrelic-dotnet-agent/actions/runs/5896225851/job/15993852246?pr=1853
HttpCollectorWire
to introduce an abstraction layer that allows usage of eitherSystem.Net.HttpClient
(on .NET 6+) orSystem.Net.HttpWebRequest
(on .NET Framework 4.6.2+) for sending data to the NR1 collector.Resolves #1844