Conversation
Sushisource
left a comment
There was a problem hiding this comment.
Overall this makes sense to me. I think we probably want to expand the integration test a bit to include more kinds of spans. I think there are some good examples in the TS SDK.
| static WORKFLOW_OTEL_TRACER: OnceLock<SdkTracer> = OnceLock::new(); | ||
|
|
There was a problem hiding this comment.
This will end up shared across the two tests which seems wrong
| /// Use this generator in the tracer provider for [`OpenTelemetryPlugin`]. This generator gives | ||
| /// the same IDs to spans during execution and replay. Outside a Workflow context that requires | ||
| /// replay safety, it uses the standard OpenTelemetry random ID generator. | ||
| #[derive(Clone, Debug, Default)] |
There was a problem hiding this comment.
Is it possible to, when they call with_tracer, force the generator to be this?
| loop { | ||
| let id = TraceId::from(random.random::<u128>()); | ||
| if id != TraceId::INVALID { | ||
| return id; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a bit odd. Why would it be invalid? Can we just clamp the random output or something if the issue is extreme values?
There was a problem hiding this comment.
IIRC the spec only classifies 0 as an invalid trace id
| the Temporal client. Workers that use the client automatically get the worker interceptors. | ||
|
|
||
| ```toml | ||
| temporalio-sdk = { version = "1.0", features = ["opentelemetry"] } |
There was a problem hiding this comment.
Codex loves adding these to our README, but I don't think they're really helpful. Version string will be annoying to to keep up to date and I don't think it is necessary to show how to enable a feature.
| loop { | ||
| let id = TraceId::from(random.random::<u128>()); | ||
| if id != TraceId::INVALID { | ||
| return id; | ||
| } | ||
| } |
There was a problem hiding this comment.
IIRC the spec only classifies 0 as an invalid trace id
What was changed
opentelemetryfeature to the main Rust SDK crate._tracer-dataheader.This integration does not trace inbound Nexus handlers because the Rust SDK does not provide an interceptor for those handlers.
Why?
The Rust SDK needs the same general OpenTelemetry behavior as other Temporal SDKs. Applications must be able to trace Temporal operations and propagate trace context across SDK boundaries.
Checklist
Closes: N/A
How was this tested:
cargo lintcargo test-lintcargo test -p temporalio-sdk --features opentelemetry --libcargo integ-test opentelemetry_plugin_parents_activity_to_application_workflow_spancargo doc -p temporalio-sdk --features opentelemetry --no-depstbd