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 storing the references to the activities created by framework and instrumentation #1878

Conversation

utpilla
Copy link
Contributor

@utpilla utpilla commented Mar 8, 2021

Fixes #1740

Changes

  • Updated OnStartActivity method to make both the framework and the instrumentation created activities to store each others' references
  • Since the gRPC ASP.NET Core framework adds the tags grpc.method and grpc.status_code only to the activity created by the framework (Link to the framework code) we don't update the Activity.Current in OnStartActivity but we enrich the instrumentation created activity in the OnStopActivity method

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #1878 (f48b8fa) into main (cb066cb) will decrease coverage by 0.54%.
The diff coverage is 77.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1878      +/-   ##
==========================================
- Coverage   83.77%   83.22%   -0.55%     
==========================================
  Files         187      188       +1     
  Lines        5967     6153     +186     
==========================================
+ Hits         4999     5121     +122     
- Misses        968     1032      +64     
Impacted Files Coverage Δ
src/OpenTelemetry/Sdk.cs 100.00% <ø> (ø)
src/OpenTelemetry/Logs/OpenTelemetryLogger.cs 26.82% <20.54%> (-44.60%) ⬇️
...tation.AspNetCore/Implementation/HttpInListener.cs 81.52% <73.17%> (-5.04%) ⬇️
src/OpenTelemetry/Logs/LogRecord.cs 82.50% <77.77%> (-5.00%) ⬇️
.../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
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-LinkToFrameworkActivity 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
1 participant