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] Streamline the exposed public interface (breaking change) #1376

Merged

Conversation

repl-chris
Copy link
Contributor

Changes

This moves the TelemetryClientMessageInspector and TelemetryDispatchMessageInspector to be internal-only classes. The only documented/supported ways to use this library is via the TelemetryEndpointBehavior, TelemetryServiceBehavior, and/or TelemetryContractBehaviorAttribute so there is no need for the message inspectors to be public. Technically this is a breaking change, however consumers are likely never referencing these classes directly today as there is no need or reason for them to - they provide no additional capabilities over the published/documented way to consume this library.

For significant contributions please make sure you have completed the following items:

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

@repl-chris repl-chris requested a review from a team September 27, 2023 16:00
@github-actions github-actions bot requested a review from CodeBlanch September 27, 2023 16:01
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #1376 (046f9ba) into main (71655ce) will increase coverage by 5.28%.
Report is 15 commits behind head on main.
The diff coverage is 81.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1376      +/-   ##
==========================================
+ Coverage   73.91%   79.20%   +5.28%     
==========================================
  Files         267       30     -237     
  Lines        9615      678    -8937     
==========================================
- Hits         7107      537    -6570     
+ Misses       2508      141    -2367     
Flag Coverage Δ
unittests-Instrumentation.Wcf 79.20% <81.35%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../Implementation/TelemetryClientMessageInspector.cs 100.00% <ø> (ø)
...mplementation/TelemetryDispatchMessageInspector.cs 86.15% <ø> (ø)
...entation.Wcf/TelemetryContractBehaviorAttribute.cs 0.00% <ø> (ø)
...ry.Instrumentation.Wcf/TelemetryServiceBehavior.cs 0.00% <ø> (ø)
...rumentation.Wcf/TracerProviderBuilderExtensions.cs 88.88% <100.00%> (+1.38%) ⬆️
...cf/Implementation/WcfInstrumentationEventSource.cs 21.73% <0.00%> (-6.04%) ⬇️
...on.Wcf/Implementation/AspNetParentSpanCorrector.cs 88.67% <88.67%> (ø)

... and 237 files with indirect coverage changes

@repl-chris repl-chris force-pushed the StreamlinePublicInterface branch from d06db6d to ed84e3f Compare September 27, 2023 16:12
@Kielek Kielek added the comp:instrumentation.wcf Things related to OpenTelemetry.Instrumentation.Wcf label Sep 27, 2023
@github-actions github-actions bot requested a review from CodeBlanch October 2, 2023 17:03
Copy link
Member

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

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

LGTM

@CodeBlanch CodeBlanch merged commit 4c5d646 into open-telemetry:main Oct 2, 2023
20 checks passed
@repl-chris repl-chris deleted the StreamlinePublicInterface branch October 3, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.wcf Things related to OpenTelemetry.Instrumentation.Wcf
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants