sdk-trace: add SdkSpanBackend
#395
Conversation
iRevive
commented
Dec 6, 2023
iRevive
commented
| Reference | Link |
|---|---|
| Java implementation | SdkSpan.java |
iRevive
added
tracing
iRevive
force-pushed
the
sdk-trace/sdk-span-backend
branch
from
af7a3fa to
6d8dd32
Compare
iRevive
force-pushed
the
sdk-trace/sdk-span-backend
branch
from
6d8dd32 to
810556e
Compare
| private[trace] final class SdkSpanBackend[F[_]: Monad: Clock: Console] private ( | ||
| spanProcessor: SpanProcessor[F], | ||
| immutableState: SdkSpanBackend.ImmutableState, | ||
| mutableState: AtomicCell[F, SdkSpanBackend.MutableState] |
There was a problem hiding this comment.
Can this be a Ref instead of an AtomicCell?
| Console[F] | ||
| .println( | ||
| s"SdkSpanBackend: calling [$method] on the ended span $context" | ||
| ) |
There was a problem hiding this comment.
Since Console is not a real logger and a lot of users might just pass the default, is this annoying? Or was it just for debugging?
There was a problem hiding this comment.
It shouldn't be logged often. If a user uses a library in a general way, these messages will not be logged at all.
The log message may occur if you try to update the span that has already ended.
For example:
for {
escapedSpan <- Tracer[F].span("my-span").use { span =>
IO.pure(span)
}
_ <- escapedSpan.updateName("new name") // <- 'SdkSpanBackend: calling [updateName] on the ended span ...' will be logged
} yield ()There was a problem hiding this comment.
Hmm, right. But it doesn't serious enough to be an errorln I suppose.
There was a problem hiding this comment.
We can hide these warnings behind a config (e.g. traceProvider.enableConsoleWarnings).
There was a problem hiding this comment.
Hmm, right. But it doesn't serious enough to be an
errorlnI suppose.
yeah. If we had a proper logging system, I would use warn.
| } | ||
|
|
||
| private def addTimedEvent(event: EventData): F[Unit] = | ||
| updateState("addEvent")(s => s.copy(events = s.events :+ event)).void |
There was a problem hiding this comment.
events is a List, isn't this pretty inefficient?
There was a problem hiding this comment.
Valid point. Let's use Vector under the hood.
