OxTelemetry - integration of ox, ox flows with OpenTelemetry

[pawelsadlo]

Provides some basic integration for open telemetry tracing via one OxTracing interface.

Implementation lives in:

1. OxTracingBase - general span API
   • byNameSpan - spans execution of any piece of code, makes opened span a child of current ongoing span
   • currentSpanUnsafe - access to current span, its not unsafe per se, however exposes otel span api to user which may be used unsafely breaking currently opened spans by other, safe oxTracing methods
   • spanUnsafe - opens span and returns AutoCloseableSpan, which contains created span and logic to finalize span (by closing it and bringing back previous otel context). its unsafe because user is expected to finalize span properly, even in case of errors and also should handle correct order of closing spans in case more spans are started by this method, makes opened span a child of current ongoing span
2. OxFlowTracing - flow span extensions
   • span - opens span when flow start emitting elements and closes it when it finishes/fails, makes opened span a child of current ongoing span

Provided native implementation relies on ox.otel.context.PropagatingVirtualThreadFactory as a open telemetry context propagation mechanism between threads.

note:
this wont work out of the box with ComputeIntensive

note2:
need to tidy up some tests yet

[adamw]

Thanks, some remarks:

• docs are missing - showing how to use the functionality
• I think that instead of import tracer.*; flow.span() it would be better to be more explicit: flow.span(tracer). The tracer needs to be imported anyway, and it's less "magical"
• do we need the context setting/unsetting? If we control the entire span start/stop?

[adamw]

Ah and finally - did you try reviewing your code with Claude/Codex/Cursor?

[pawelsadlo]

Thanks, some remarks:

• docs are missing - showing how to use the functionality

will do

• I think that instead of import tracer.*; flow.span() it would be better to be more explicit: flow.span(tracer). The tracer needs to be imported anyway, and it's less "magical"

• do we need the context setting/unsetting? If we control the entire span start/stop?

yes, for integration with third party libs relying on automatic otel instrumentation that may start spans within our span

[adamw]

yes, for integration with third party libs relying on automatic otel instrumentation that may start spans within our span

Bue doesn't span.start() automatically set the current span in the whatever is configured as context? That is, we could just rely on what the end-user sets as the context propagator.

[pawelsadlo]

nope, span must be set explicitly as current

ref: https://www.javadoc.io/doc/io.opentelemetry/opentelemetry-api/latest/io/opentelemetry/api/trace/SpanBuilder.html#startSpan()

[adamw]

This is an automated review (Claude Code).

Tests pass (18/18). The main idea (wrap runToEmit in a span, use PropagatingVirtualThreadFactory to pass context to forks) is fine. Issues below.

Correctness

1. A failure after the span marks the span as failed. span wraps flow.runToEmit(emit), and later stages (map, tap, ...) run inside that call. Checked: Flow.fromValues(1, 2).span(t)("down").map(x => if x == 2 then throw ... else x) ends the down span with status ERROR and an exception event, even though the spanned flow did not fail. The test "end span when a downstream stage fails after span" doesn't check the status.
2. Later stages run inside the span. Span.current() inside a .map placed after .span(...) is the flow's span. So the span's duration includes work of later stages, and spans opened there become its children. The test "keep the flow span as a child of the recovery span..." depends on this. Either fix it (only treat errors thrown by the spanned flow as its errors), or document it.
3. catch case e: Throwable treats every exception as an error. This includes InterruptedException (fork cancelled) and boundary.Break (either / .ok()). Cancelling a fork or leaving an either block early is recorded as an error.
4. The thread factory check has side effects. ContextHolder.CheckingThreadFactory reads the lazy ox.oxThreadFactory, which fixes it to the default. If a span is opened before setOxThreadFactory, that call then throws "already used". It also prints to System.err from a library, and misses factories that wrap PropagatingVirtualThreadFactory.
5. opentelemetry-sdk is a compile dependency. Main code only uses opentelemetry-api. The SDK should be test-only (sdk-testing already brings it in).

Not needed

• ContextHolder, native / custom. Both holders just call Context.current() / makeCurrent(); the only difference is the warning from (4). Remove them and say in the docs that PropagatingVirtualThreadFactory is required.
• OxTracing / OxFlowTracing. Without ContextHolder, OxTracing only holds a Tracer. The API could take the otel Tracer directly, e.g. flow.span(tracer)(...).
• Same API twice: as methods on OxTracing and as top-level functions / extensions in tracing.scala and flowTracing.scala. Keep one.
• currentSpanUnsafe is otel's Span.current().
• spanUnsafe / AutoCloseableSpan mostly repeat spanBuilder(...).startSpan() + makeCurrent(). Keep only if worth it.
• beforeClose: ((Span, Option[Throwable])) => Span: the tuple and the ignored return value lead to _._1.addEvent(...). Use (Span, Option[Throwable]) => Unit.
• FlowOps.scala: removing a stray // is unrelated to this PR.

Structure

• Existing module is otel-context / ox.otel.context. This PR uses ox-telemetry / ox.telemetry.tracing. Something like otel-tracing / ox.otel.tracing would match.
• private[tracing] trait OxFlowTracing is the parameter type of the public span extension, so a private type is part of the public API.

Docs

Still missing: a page in doc/integrations/, the module in documentation.dependsOn, and an entry in doc/other/stability.md.

Tests

• "should not open a span until the flow is run" never calls .span, so it can't fail.
• The three supervisedError tests are nearly the same, and none check status or events, though their names say "with event".
• No checks of span status for failures after the span or for cancellation (where 1 and 3 break).
• 8 compile warnings (unused imports such as Meter, deprecated JavaConverters, unused values).
• Test class is in the default package.