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

Add WCF ServiceBehavior implementation. Resolves #142. #152

Merged
merged 7 commits into from
Sep 7, 2021

Conversation

Jaans
Copy link
Contributor

@Jaans Jaans commented Sep 4, 2021

Adds IServiceOperationBehavior implementation to enable based configuration of WCF services at the service level.
Refer Microsoft Docs

@Jaans Jaans requested a review from CodeBlanch as a code owner September 4, 2021 14:08
@Jaans Jaans requested a review from a team September 4, 2021 14:08
@CodeBlanch
Copy link
Member

@Jaans Thanks would you mind updating README with instructions on how to apply to config files?

@codecov
Copy link

codecov bot commented Sep 4, 2021

Codecov Report

Merging #152 (c4ec80a) into main (739c6b7) will not change coverage.
The diff coverage is 50.00%.

❗ Current head c4ec80a differs from pull request most recent head 147db06. Consider uploading reports for the commit 147db06 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #152   +/-   ##
=======================================
  Coverage   79.37%   79.37%           
=======================================
  Files          67       67           
  Lines        1789     1789           
=======================================
  Hits         1420     1420           
  Misses        369      369           
Impacted Files Coverage Δ
...b.Instrumentation.Wcf/TelemetryEndpointBehavior.cs 91.66% <50.00%> (ø)

…Behavior.cs

Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
@Jaans
Copy link
Contributor Author

Jaans commented Sep 5, 2021

@Jaans Thanks would you mind updating README with instructions on how to apply to config files?

@CodeBlanch I looked at doing this quick.

TelemetryEndpointBehavior uses the TelemetryBehaviorExtensionElement to apply it in the config files against endpoints. Similarly we'll need another BehaviorExtensionElement (e.g. TelemetryServiceBehaviorExtensionElement) in order to apply the new TelemetryServiceBehavior against services in the config files.

My gut instinct is to rename the existing TelemetryBehaviorExtensionElement to TelemetryEndpointBehaviorExtensionElement, but that would be a breaking change.

Happy to make those changes, just wanted to validate that's the direction you want it to go down.

Let me know. I'll continue making the other changes and refactoring so long.

…hat adds dispatch behavior to endpoint from both TelemetryServiceBehavior and TelemetryEndpointBehaviorUpdate

src/OpenTelemetry.Contrib.Instrumentation.Wcf/TelemetryServiceBehavior.cs
Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
@CodeBlanch
Copy link
Member

@Jaans I'm OK if you want to rename TelemetryBehaviourExtensionElement to TelemetryEndpointBehaviorExtensionElement. Wcf instrumentation library is still pre-release so it is a now or never kind of thing 😄

PS: Just to be clear, please drop the "Behaviour" spelling when you do that and go with "Behavior" so it matches the base class. I'm not sure how that "u" snuck in there!

Rename endpoint behavior element class, update examples and README.md usage documentation.
Merge branch 'main' of https://github.com/Jaans/opentelemetry-dotnet-contrib into main
@Jaans
Copy link
Contributor Author

Jaans commented Sep 6, 2021

@CodeBlanch OK, that's done too :-)
Also, example project app.config files are also updated due to the rename.
Thanks.

@CodeBlanch CodeBlanch merged commit b907e05 into open-telemetry:main Sep 7, 2021
@CodeBlanch
Copy link
Member

Now available in OpenTelemetry.Contrib.Instrumentation.Wcf 1.0.0-rc3

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