+span implement withSpan() convenience #13
Conversation
| @@ -156,9 +156,9 @@ final class OSSignpostSpan: Span { | |||
| #if DEBUG | |||
There was a problem hiding this comment.
this entire file will be removed anyway in remove use cases, we should have them in a future extras repo and the examples repos #14
| InstrumentationSystem.bootstrap(tracer) | ||
| InstrumentationSystem.bootstrapInternal(tracer) | ||
| defer { | ||
| InstrumentationSystem.bootstrapInternal(NoOpTracer()) |
There was a problem hiding this comment.
Didn't we make the argument optional for bootstrapInternal to reset it in tearDown? Seems like you made the NoOpTracer publicly initializable for that purpose which seems a bit unnecessary to me.
There was a problem hiding this comment.
We need to provide a NoOpTracer publicly anyway "just in case". The same has come up with logging where people wanted to manually create stuff and default to a NoOp. So pre-empting the same question coming up here;
For reference:
- Add NoOpLogHandler for when you don't want logging swift-log#142
- Make SwiftLogNoOpLogHandler.init() public swift-log#146 Adam's "i want to cry" 😆
I also believe it is required by otel to provide an no-op, so we might as well play nice there right away.
There was a problem hiding this comment.
Didn't we make the argument optional for bootstrapInternal to reset it in tearDown?
Yeah valid point... but I still want to make the init public for the above reasons
Resolves #4
A missing convenience that definitely is very useful.
This also sets precedence for other libraries to offer their
future.withSpan()etc which we do want to encourage.