-
Notifications
You must be signed in to change notification settings - Fork 7
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 optional Tracer to appcommon.Config #14
Conversation
db29947
to
036773c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we usually squash commits during merge unless there are very large, disparate changes that make sense to be separated
thank you for this! I made a few minor style notes, but it looks fine and straightforward |
Co-authored-by: Owen Williams <owen-github@ywwg.com>
Thanks for the swift response @ywwg ! I've accepted your changes + renamed "tearDown" -> "resetTracingGlobals" (I think that explains the purpose of the function and makes it a little bit self-documenting). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the quick fixes -- now that I understand the call behavior better, I want to tweak the invocation slightly.
d292380
to
dafe707
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thanks!
Add the ability to pass an opentracing.Tracer via appcommon.Config.Tracer
We are using harness.go as the basis for the Faro Endpoint, and we would like to configure the OpenTracing->OTel bridge tracer for use in our code ( https://github.com/grafana/app-o11y-kwl-endpoint/blob/main/cmd/kwl/main.go )
@rlankfo authored the 1st commit. I've attempted to add a test to show the behavior is correct.
I will squash the commits before merging if that is preferred?