fix: Refactor to eliminate usage of .GetAwaiter().GetResult() in Framework builds. (#2534)

[tippmar-nr]

Refactors HttpCollectorWire.Send() to remove usage of .GetAwaiter().GetResult(), which required additional refactoring in IHttpClient and IHttpContentWrapper.

With this change, there are now no usages of .GetAwaiter().GetResult() in code built for the .NET Framework target.

Resolves #2534

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.50%. Comparing base (227003a) to head (26e7d24).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2535      +/-   ##
==========================================
+ Coverage   80.47%   80.50%   +0.02%     
==========================================
  Files         459      459              
  Lines       28775    28873      +98     
  Branches     3153     3167      +14     
==========================================
+ Hits        23158    23244      +86     
- Misses       4837     4843       +6     
- Partials      780      786       +6     

Flag	Coverage Δ
Agent	81.49% <80.00%> (+0.02%)	⬆️
Profiler	71.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
.../Agent/Core/DataTransport/Client/HttpClientBase.cs	84.09% <ø> (ø)
...ic/Agent/Core/DataTransport/Client/HttpResponse.cs	79.41% <100.00%> (-10.59%)	⬇️
...ic/Agent/Core/DataTransport/Client/NRHttpClient.cs	78.43% <100.00%> (-14.91%)	⬇️
...nt/Core/DataTransport/Client/NRWebRequestClient.cs	89.79% <100.00%> (-10.21%)	⬇️
...elic/Agent/Core/DataTransport/HttpCollectorWire.cs	88.13% <100.00%> (ø)
...nt/Core/DataTransport/Client/HttpContentWrapper.cs	0.00% <0.00%> (ø)
...e/DataTransport/Client/WebRequestClientResponse.cs	91.17% <50.00%> (ø)

... and 2 files with indirect coverage changes

[oleksandrk-adorama]

Hi @tippmar-nr, @nrcventura

Can I ask you how these changes prevent potential deadlocks since I still see GetAwaiter().GetResult() in many places?

Also I'm curious now when you replaced asynchronous calls by sync analogues how this affected performance (e,g. scalability) ?

I'm not familiar with the code base but maybe it was possible to use async natively all the way around?

Thanks

P.S.
I guess the PR description is a bit misleading
"With this change, there are now no usages of .GetAwaiter().GetResult() in code built for the .NET Framework target."

[tippmar-nr]

@oleksandrk-adorama Thanks for your comment.

The code snippet you referenced above is only included in the .NET Standard 2.0 build of the agent (see line 4).

So while we do still have several usages of .GetAwaiter().GetResult(), none of them occur in the .NET Framework build.

As for performance, there shouldn't be any impact, as the previous calls were synchronously waiting for the async method to finish and have been replaced with direct sync methods.

There are a few architectural concerns with the present implementation of the .NET agent that prevent us from using async everywhere; we're hoping to resolve those concerns in a future version of the agent but have no firm timeline for it at present.