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

Merge to main: New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API #2270

Merged
merged 12 commits into from
Aug 25, 2021

Conversation

CodeBlanch
Copy link
Member

Relates to #2222
Relates to #2249
Closes #1701

Changes

Merges the new design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API to main. Includes RecordException option in ASP.NET instrumentation.

TODOs

  • CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

CodeBlanch and others added 6 commits August 12, 2021 11:57
…ry.API part 2 (#2254)

* Use a single context.Items key for state management to make things more efficient.

* Added a comment for clarity.

* Code review.
…ry.API part 3 (#2256)

* Update ASP.NET instrumentation to use the new TelemetryHttpModule.

* Fixed TelemetryHttpModule not starting its Activity objects. Added an example of request suppression.

* Tweaks an logging improvements.

* Sealed AspNetInstrumentationEventSource.

* Code review.
…ry.API part 4 (#2258)

* Fixed up TelemetryHttpModule unit tests.

* Added tests for the new HasStarted helper and added checks for StartedButNotSampledObj when not sampled.

* Code review.
…ry.API part 5 (#2261)

* Updated ASP.NET instrumentation tests for new TelemetryHttpModule.

* Added a test for the new RecordException option.

* Code review.
…ry.API part 6 (#2264)

* CHANGELOG & README updates.

* Apply suggestions from code review

Co-authored-by: Reiley Yang <reyang@microsoft.com>

Co-authored-by: Reiley Yang <reyang@microsoft.com>
@CodeBlanch CodeBlanch requested a review from a team August 22, 2021 03:22
@codecov
Copy link

codecov bot commented Aug 22, 2021

Codecov Report

Merging #2270 (fd3752f) into main (f1e745d) will increase coverage by 0.34%.
The diff coverage is 72.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2270      +/-   ##
==========================================
+ Coverage   79.25%   79.59%   +0.34%     
==========================================
  Files         238      232       -6     
  Lines        7499     7284     -215     
==========================================
- Hits         5943     5798     -145     
+ Misses       1556     1486      -70     
Impacted Files Coverage Δ
....AspNet.TelemetryHttpModule/TelemetryHttpModule.cs 4.65% <12.50%> (+2.37%) ⬆️
....TelemetryHttpModule/AspNetTelemetryEventSource.cs 30.00% <23.52%> (+3.07%) ⬆️
...Implementation/AspNetInstrumentationEventSource.cs 76.92% <66.66%> (-3.08%) ⬇️
...ation.AspNet.TelemetryHttpModule/ActivityHelper.cs 82.69% <84.78%> (+2.69%) ⬆️
....TelemetryHttpModule/TelemetryHttpModuleOptions.cs 87.50% <87.50%> (ø)
...umentation.AspNet/Implementation/HttpInListener.cs 88.46% <87.87%> (-0.43%) ⬇️
...ry.Instrumentation.AspNet/AspNetInstrumentation.cs 100.00% <100.00%> (ø)
...rumentation.AspNet/AspNetInstrumentationOptions.cs 100.00% <100.00%> (ø)
...entation.AspNet/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs 76.47% <0.00%> (-5.89%) ⬇️
... and 1 more

Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM, consider merge the commits (do not squash).

@CodeBlanch
Copy link
Member Author

@reyang I'm going to squash merge just because the commits only say "New design for TelemetryHttpModule using ActivitySource + OpenTelemetry.API part X" where X is 1-6. It's not really useful info. Probably should have preserved the commits into the working branch and then we could have done it here. Sorry about that!

@CodeBlanch CodeBlanch merged commit d9ad092 into main Aug 25, 2021
@CodeBlanch CodeBlanch deleted the aspnet-telemetrycorrelation-otelintegration branch August 25, 2021 18:43
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.

AspNet Instrumentation OnException hook
3 participants