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

WCF: Misc cleanup #163

Merged
merged 3 commits into from
Oct 22, 2021
Merged

WCF: Misc cleanup #163

merged 3 commits into from
Oct 22, 2021

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Sep 29, 2021

Changes

A bit of misc cleanup:

  • Sealed a couple of internal classes.
  • Removed the propagator from WcfInstrumentationOptions.
  • Instrumentation now resets Activity.Current if it was changed and clears baggage at the end of a request.

TODOs

  • CHANGELOG update

@CodeBlanch CodeBlanch requested a review from a team September 29, 2021 06:05
@codecov
Copy link

codecov bot commented Sep 29, 2021

Codecov Report

Merging #163 (95b5ee0) into main (91a1269) will decrease coverage by 0.08%.
The diff coverage is 83.33%.

❗ Current head 95b5ee0 differs from pull request most recent head 9f440c4. Consider uploading reports for the commit 9f440c4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
- Coverage   78.78%   78.69%   -0.09%     
==========================================
  Files          85       85              
  Lines        2102     2098       -4     
==========================================
- Hits         1656     1651       -5     
- Misses        446      447       +1     
Impacted Files Coverage Δ
...strumentation.Wcf/Implementation/ActionMetadata.cs 100.00% <ø> (ø)
...entation.Wcf/TelemetryContractBehaviorAttribute.cs 0.00% <ø> (ø)
...b.Instrumentation.Wcf/WcfInstrumentationOptions.cs 80.00% <ø> (-10.00%) ⬇️
...rumentation.Wcf/TelemetryClientMessageInspector.cs 81.81% <80.00%> (-1.08%) ⬇️
...cf/Implementation/WcfInstrumentationEventSource.cs 26.31% <100.00%> (ø)

Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

Would be nice to add code comment about the need to restore Activity.Current

@CodeBlanch CodeBlanch merged commit 136ec1e into open-telemetry:main Oct 22, 2021
@CodeBlanch CodeBlanch deleted the wcf-cleanup branch October 22, 2021 17:03
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.

2 participants