feat: LogSpanner — pluggable span dispatch bridging ZIO.logSpan with OTEL (#1022)#1138
feat: LogSpanner — pluggable span dispatch bridging ZIO.logSpan with OTEL (#1022)#1138li-nkSN wants to merge 1 commit intozio:v4.0.0-rcfrom
Conversation
|
Hello! Great stuff! I would like to suggest streamlining the resulting design to align with the library's established architecture. It basically just moves the constructors and layers around to make it more coherent with the current design. I haven't touched the tests in my PR, so they need to be adjusted accordingly if you are happy with the proposed changes. Feel free to copy or re-create the suggested changes in your PR so I can remove my draft PR later. |
…OTEL (zio#1022) Implements the LogSpanner design proposed in zio#1022: a FiberRef-based dispatch mechanism that lets library code mark spans via `effect @@ LogSpanner.span("name")` without depending on Tracer. Three backends: - Default: delegates to ZIO.logSpan (zero OTEL overhead) - OTEL: creates real OpenTelemetry spans via Tracer.span - Hybrid: creates both OTEL and ZIO logSpans API surface follows grouzen's suggested restructuring: - LogSpanner trait with curried logSpan(name)(zio)(implicit trace) - installOtel/installHybrid on LogSpanner companion - logSpan + aspects.logSpan on core.OpenTelemetry trait - installOtelLogSpanner/installHybridLogSpanner layers on OpenTelemetry object OtelLogSpanner folded into LogSpanner companion. Design docs removed in favor of scaladoc. 14 test cases covering default/OTEL/hybrid backends, scoped installation, layer factory, and fiber correctness. Design based on the prototype by @thiloplanz in zio#1022. Thanks to @grouzen for design guidance and restructuring review.
li-nkSN
force-pushed
the
feature/logspanner-1022
branch
from
61ff58a to
cf6fb47
Compare
| /** | ||
| * A `ZIOAspect` that wraps an effect with a named span using the currently installed `LogSpanner`. | ||
| * | ||
| * This is the primary call-site API for library code that should not depend on `Tracer` or `OpenTelemetry`. | ||
| * | ||
| * Usage: | ||
| * {{{ | ||
| * myEffect @@ LogSpanner.span("operationName") | ||
| * }}} | ||
| * | ||
| * @param spanName | ||
| * the span name | ||
| * @return | ||
| * a ZIOAspect that applies the span | ||
| */ | ||
| def span(spanName: String): ZIOAspect[Nothing, Any, Nothing, Any, Nothing, Any] = | ||
| new ZIOAspect[Nothing, Any, Nothing, Any, Nothing, Any] { | ||
| override def apply[R, E, A](zio: ZIO[R, E, A])(implicit trace: Trace): ZIO[R, E, A] = | ||
| currentLogSpanner.getWith(_.logSpan(spanName)(zio)) | ||
| } |
There was a problem hiding this comment.
I probably missed mentioning this one in my PR, but it can be removed since we basically moved it to OpenTelemetry.logSpan. See the next comment below
| test("delegates to ZIO.logSpan — no OTEL span created") { | ||
| for { | ||
| tracerTestkit <- ZIO.service[TracerTestkit] | ||
| _ <- ZIO.unit @@ LogSpanner.span("mySpan") |
There was a problem hiding this comment.
Sorry, I had to be explicit about this. The idea is to replace this call with OpenTelemetry.logSpan.
This way, we kinda mimic the ZIO.logSpan, using the same naming convention.
|
@grouzen Thanks for the restructuring feedback — we've incorporated all of it (curried signatures, implicit trace, installOtel/installHybrid on LogSpanner, OpenTelemetry.logSpan + aspects.logSpan on the trait, layer factories on the OpenTelemetry object). Regarding removing LogSpanner.span — we'd like to keep it alongside OpenTelemetry.aspects.logSpan. The reason is the viral dependency problem that #1022 was designed to solve. Consider a database module that instruments repository operations: // database module — depends only on zio-opentelemetry-core def findById(id: Long): Task[Option[Entity]] = No OpenTelemetry instance needed. No environment requirement. The module depends only on zio-opentelemetry-core. If LogSpanner.span is removed and the call site must use openTelemetry.aspects.logSpan: // database module — now needs OpenTelemetry instance Where does openTelemetry come from? Either a constructor parameter or ZIO.serviceWithZIO[OpenTelemetry]. Both propagate — every module that calls findById must provide OpenTelemetry, and every module that calls those modules must provide it too. This is the viral Tracer dependency that LogSpanner.span and OpenTelemetry.aspects.logSpan are functionally identical — same FiberRef[LogSpanner], same dispatch. They can coexist: OpenTelemetry.aspects.logSpan for application code that already has an instance, LogSpanner.span for library/infrastructure code that shouldn't need |
|
Hi! Sorry for being silent for so long. I've thought about it a lot, and I feel like this kind of stuff shouldn't be added to the library in this form. I think the proper approach would be to add a similar mechanism to the one used for switching between logging backends in ZIO: https://zio.dev/guides/tutorials/create-custom-logger-for-a-zio-application. I believe, this is the conclusion we came to with @thiloplanz in the original issue. The main idea is we want to be able to swap the backends of logSpan and logAnnotate using the non-existent ZIO API. The main argument against the suggested API is that it feels like we are adding another clunky version of two already existing APIs:
The better strategy would be to keep this additional layer/API in a separate library. |
2 participants
Implements the LogSpanner design proposed in #1022: a FiberRef-based
dispatch mechanism that lets library code mark spans via
effect @@ LogSpanner.span("name")without depending on Tracer.Three backends:
Includes 15 test cases covering default/OTEL/hybrid backends,
scoped installation, and fiber correctness (fork, timeout,
child fiber inheritance).
Design based on the prototype by @thiloplanz in #1022.
Thanks to @grouzen for design guidance and confirming the
ZIO.logAnnotate attribute propagation approach.