Add netstandard2.0 target to Grpc.Net.Client instrumentation #3105
Conversation
|
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. |
| var httpClient = ClientTestHelpers.CreateTestClient(async request => | ||
| { | ||
| var streamContent = await ClientTestHelpers.CreateResponseContent(new HelloReply()); | ||
| var response = ResponseUtils.CreateResponse(HttpStatusCode.OK, streamContent, grpcStatusCode: global::Grpc.Core.StatusCode.OK); | ||
| response.TrailingHeaders().Add("grpc-message", "value"); | ||
| return response; | ||
| }); |
There was a problem hiding this comment.
This test uses the pattern used to mock an http client in the grpc-dotnet project for testing the Grpc.Net.Client. It demonstrates that the gRPC client instrumentation is invoked correctly for .NET Framework and modern .NET.
| @@ -104,6 +117,7 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool sho | |||
| Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); | |||
| } | |||
|
|
|||
| #if !NETFRAMEWORK | |||
There was a problem hiding this comment.
Many of the tests in this suite require an actual end-to-end call using a non-mocked HttpClient. To test the same functionality for .NET Framework, we would need to devise a more complicated integration test. Most likely involving spinning up Windows containers.
I'm proposing we acknowledge the short-comings of this test suite for .NET Framework and we ship a netstandard2.0 version of the gRPC client instrumentation anyways.
alanwest
changed the title
Codecov Report
@@ Coverage Diff @@
## main #3105 +/- ##
==========================================
- Coverage 85.58% 85.54% -0.05%
==========================================
Files 261 261
Lines 9395 9395
==========================================
- Hits 8041 8037 -4
- Misses 1354 1358 +4
|
test/OpenTelemetry.Instrumentation.Grpc.Tests/OpenTelemetry.Instrumentation.Grpc.Tests.csproj
Outdated
Show resolved
Hide resolved
This is important for users consuming Grpc.Net.Client in .NET Framework applications.
There have been two previous attempts to introduce a
netstandard2.0build of the Grpc.Net.Client instrumentationWe did not merge this previous PRs due to trickiness with testing the instrumentation's functionality for .NET Framework. Much of the current test suite spins up an in-process ASP.NET Core gRPC server. This is not possible for the
net462target.I've modified one of the existing tests to use a mock HttpClient, so that starting an in-process gRPC server is unnecessary.
The code that mocks up the HttpClient was fork-lifted from here: https://github.com/grpc/grpc-dotnet/tree/master/test/Shared.
This PR does not port the entire Grpc.Net.Client test suite to run on .NET Framework. Many of the tests test both the instrumentation of the gRPC call and the underlying HttpClient call. To my knowledge, testing the underlying HttpClient call requires making a real call since the mock HttpClient will not invoke the HttpClient instrumentation.