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

Update HttpInListener to add gRPC tags - By retrieving gRPC tag values from payload #1877

Conversation

utpilla
Copy link
Contributor

@utpilla utpilla commented Mar 7, 2021

Fixes #1740

Changes

  • Add a gRPC request check in the the listener OnStartActivity method to get the grpc.method value from payload and add the tag to the activity
  • Possibly add grpc.status_code tag to the activity in the OnStopActivity method of the listener. Need some advice on the best way to obtain the status code from payload.

@codecov
Copy link

codecov bot commented Mar 7, 2021

Codecov Report

Merging #1877 (9b568af) into main (cb066cb) will decrease coverage by 0.43%.
The diff coverage is 78.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1877      +/-   ##
==========================================
- Coverage   83.77%   83.33%   -0.44%     
==========================================
  Files         187      188       +1     
  Lines        5967     6134     +167     
==========================================
+ Hits         4999     5112     +113     
- Misses        968     1022      +54     
Impacted Files Coverage Δ
src/OpenTelemetry/Sdk.cs 100.00% <ø> (ø)
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 26.82% <20.54%> (-44.60%) ⬇️
src/OpenTelemetry/Logs/LogRecord.cs 82.50% <77.77%> (-5.00%) ⬇️
...tation.AspNetCore/Implementation/HttpInListener.cs 86.23% <85.71%> (-0.34%) ⬇️
.../OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs 77.55% <88.23%> (+7.09%) ⬆️
src/OpenTelemetry/Trace/ExceptionProcessor.cs 90.90% <90.90%> (ø)
...rc/OpenTelemetry/Trace/TracerProviderBuilderSdk.cs 89.83% <91.66%> (-0.92%) ⬇️
src/OpenTelemetry/Trace/TracerProviderSdk.cs 95.16% <98.64%> (+4.38%) ⬆️
...metryProtocol/Implementation/ActivityExtensions.cs 86.89% <100.00%> (+0.05%) ⬆️
...ry.Instrumentation.AspNet/AspNetInstrumentation.cs 100.00% <100.00%> (ø)
... and 23 more

@utpilla utpilla changed the title Update HttpInListener.cs to add gRPC tags - By retrieving gRPC tag values from payload Update HttpInListener to add gRPC tags - By retrieving gRPC tag values from payload Mar 8, 2021
// Set IsAllDataRequested to false for the activity created by the framework to only export the sibling activity and not the framework activity
activity.IsAllDataRequested = false;

// Make the new one Activity.Current
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is misleading. Activity.Current is modified when newOne.Start() is called itself, not at this line.

@utpilla
Copy link
Contributor Author

utpilla commented Mar 8, 2021

Closing this PR as we are going forward with the approach of creating the new activity with the OperationName used by the framework #1879

@utpilla utpilla closed this Mar 8, 2021
@utpilla utpilla deleted the utpilla/Fix-gRPC-Instrumentation-For-Sibling-Activity-gRPCSpecificCheck branch July 30, 2021 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instrumentation.AspNetCore for gRPC services omits ALL rpc.* attributes under certain conditions
2 participants