feat: Add OpenTelemetrySpanExt methods for direct span event creation #201

alessandrobologna · 2025-04-26T17:55:10Z

This PR adds methods to OpenTelemetrySpanExt for directly adding OpenTelemetry span events, addressing the limitations of using tracing::event! for events with dynamic attributes.

Fixes #200

Motivation

As discussed in issue #200, the current tracing::event! macro requires attribute keys to be known at compile time. This makes it difficult or impossible to record OpenTelemetry span events with dynamic attributes (e.g., from processing arbitrary JSON payloads) without resorting to lossy serialization, which hinders observability.

This change provides a way to directly add opentelemetry::trace::Event objects with dynamic Vec<KeyValue> attributes, similar to how set_attribute allows setting dynamic span attributes. This improves flexibility, makes porting code easier, and aligns better with the capabilities of the OpenTelemetry Span API.

Solution

This PR extends the OpenTelemetrySpanExt trait with two new methods:

add_otel_span_event<T>(&self, name: T, attributes: Vec<KeyValue>) where T: Into<Cow<'static, str>>: Adds an OpenTelemetry span event with the current timestamp.
add_otel_span_event_with_timestamp<T>(&self, name: T, timestamp: SystemTime, attributes: Vec<KeyValue>) where T: Into<Cow<'static, str>>: Adds an OpenTelemetry span event with a specified timestamp.

The implementation uses the existing with_subscriber -> downcast_ref::<WithContext> -> with_context pattern found in other OpenTelemetrySpanExt methods (set_attribute, set_status) to access the internal OtelData builder and push the event.

The name add_otel_span_event was chosen over add_event to avoid potential confusion with the tracing::event! macro within the context of this crate.

This PR includes:

The trait method definitions and implementations in src/span_ext.rs.
Corresponding test cases in tests/span_ext.rs.
Updates to README.md documentation.
A new example file examples/opentelemetry-span-event.rs.
Updated CHANGELOG.md.

alessandrobologna · 2025-04-26T18:39:49Z

Hi @djc (and other maintainers),
Just wanted to add a couple of notes about this PR:
Clippy Fix Commit: You'll notice the last commit (c54ba96) addresses a Clippy lint (double_ended_iterator_last in tests/metrics_publishing.rs). This warning was surfaced by the CI checks (-D warnings) and appeared unrelated to the core feature addition, so I kept it as a separate commit for clarity.
Versioning & Changelog: I haven't bumped the crate version in Cargo.toml, and the changelog entries for the new feature and example are currently under the ## Unreleased section in CHANGELOG.md.

I'm happy to handle the version bump and finalize the changelog entry if after your review this PR is ready to merge. I wasn't sure about the appropriate version target for this addition, since it's adding new API surface (non-breaking) to a 0.x crate, would you typically target a minor version bump (e.g., 0.31.0) or a patch release (e.g., 0.30.1)?
Happy to follow whatever the project's policy is.
Thanks for taking a look!

djc

This is mostly looking good, thanks!

Please keep the clippy commits separate and squash any additional changes into your original commit (might be easier to reorder them so that the clippy commits come first).

djc · 2025-05-01T10:02:00Z

src/span_ext.rs

+    ///
+    /// app_root.add_otel_span_event("job_completed", vec![KeyValue::new("status", "success")]);
+    /// ```
+    fn add_otel_span_event<T>(&self, name: T, attributes: Vec<KeyValue>)


Are there other kinds of events in OTel? The name is a little verbose, so would be nice if we can get away with something a bit shorter like add_otel_event().

Yes, there's a new WIP spec for OTel Events, as a new signal type. More background about it in here:

https://github.com/open-telemetry/opentelemetry-specification/blob/main/oteps/0202-events-and-logs-api.md

https://github.com/open-telemetry/opentelemetry-specification/blob/main/oteps/0265-event-vision.md

https://github.com/open-telemetry/opentelemetry-specification/blob/main/oteps/4430-span-event-api-deprecation-plan.md

I think though that it's still far away from being widely implemented and adopted, so I am happy with the span events API for now (and I don't necessarily agree that it was a good idea to introduce a new signal type, but that's digressing).

I agree that the name is verbose, but add_otel_event would then be a little ambiguous when those specs are more widely adopted and known. Alternatively, we could use add_event (as in the OTel Span trait, in the same way that you have add_link) and since this is a method of a trait that is meant to "wrap" an OTel span, maybe it would be clear that is not a Tracing event. Let me know your preference and I will update the PR.

Yeah, let's do add_event() for now -- it seems there will still be sufficient opportunities to make breaking changes should the need arise.

djc · 2025-05-01T10:02:41Z

src/span_ext.rs

+    ///
+    /// app_root.add_otel_span_event("job_completed", vec![KeyValue::new("status", "success")]);
+    /// ```
+    fn add_otel_span_event<T>(&self, name: T, attributes: Vec<KeyValue>)


Nit: please use impl trait in arguments consistently where possible, so this should have name: impl Into<Cow<'static, str>> (everywhere).

djc · 2025-05-01T10:02:57Z

examples/opentelemetry-span-event.rs

@@ -0,0 +1,65 @@
+use opentelemetry::trace::TracerProvider as _;


This seems a little niche so I'd prefer to avoid maintaining an example for it.

Ok I can remove it

djc · 2025-05-01T10:04:25Z

src/span_ext.rs

+            if let Some(get_context) = subscriber.downcast_ref::<WithContext>() {
+                get_context.with_context(subscriber, id, move |data, _tracer| {
+                    if let Some(event) = event.take() {
+                        data.builder.events.get_or_insert_with(Vec::new).push(event);
+                    }
+                });
+            }


Nit: I'm aware this is mostly copy/pasting, but please use let-else to reduce rightward drift where possible. (If you want to fix up the original function too, that would be nice, but please keep it in a separate commit.)

djc · 2025-05-01T10:06:08Z

CHANGELOG.md

+- Add `OpenTelemetrySpanExt::add_otel_span_event` function to allow adding OpenTelemetry
+  events directly to a `tracing::Span`, enabling the use of dynamic attribute keys.
+- Add `OpenTelemetrySpanExt::add_otel_span_event_with_timestamp` function, similar to
+  `add_otel_span_event`, but allowing a specific `SystemTime` to be provided for the event.


Nit: I think these changes belong in a single bullet point.

djc · 2025-05-01T10:06:20Z

CHANGELOG.md

+  events directly to a `tracing::Span`, enabling the use of dynamic attribute keys.
+- Add `OpenTelemetrySpanExt::add_otel_span_event_with_timestamp` function, similar to
+  `add_otel_span_event`, but allowing a specific `SystemTime` to be provided for the event.
+- Add `examples/opentelemetry-span-event.rs` demonstrating the usage of the new span event methods.


Don't forget to remove this when you remove the example.

Use `.next_back()` instead of `.last()` on a `DoubleEndedIterator` in `tests/metrics_publishing.rs` to satisfy the `clippy::double_ended_iterator_last` lint, which was likely surfaced by CI flags (`-D warnings`) or a toolchain update.

alessandrobologna · 2025-05-01T14:54:13Z

@djc I think I have addressed all the changes requested. Let me know if you spot something else.

djc · 2025-05-02T08:23:11Z

As mentioned:

squash any additional changes into your original commit

Please squash the last two commits ("rename span event methods" and "use impl trait consistently"), which AFAICT only touch new code you have added, into your original commit ("add OpenTelemetrySpanExt methods").

Adds `add_event` and `add_event_with_timestamp` methods to the `OpenTelemetrySpanExt` trait. These methods allow users to add OpenTelemetry span events directly to a `tracing::Span`, including events with dynamic attribute keys, bypassing the limitations of the `tracing::event!` macro for such use cases. The signatures use `impl Into<Cow<'static, str>>` for the event name. Includes: - Trait method definitions and implementations. - Corresponding test cases. - Updates to README.md. - Changelog entries. Also incorporates refactoring based on review feedback: - Use let-else patterns to reduce rightward drift. - Rename methods for API simplification (`add_event`). - Use `impl trait` consistently in method parameters. - Remove example file (`examples/opentelemetry-span-event.rs`).

alessandrobologna · 2025-05-02T12:48:56Z

Hi @djc, thanks! I think I've addressed the feedback:

Kept the clippy fixes separate and moved them first.
Squashed the feature addition and subsequent refactorings into the final commit (78e8247).

The commit history should now match your suggestion. Let me know if there's something else!

djc · 2025-05-02T21:37:20Z

You actually didn't address my feedback exactly, because you also squashed the refactoring that changed other pre-existing code that you didn't add, so that's a little yucky from a reviewing perspective. However, I don't think it's worth doing another round trip on this, so I'll just merge this.

alessandrobologna · 2025-05-02T21:52:27Z

Apologies, I misunderstood, I thought you just wanted the clippy fixes separated. I see your point now.

djc reviewed May 1, 2025

View reviewed changes

alessandrobologna added 2 commits May 1, 2025 10:09

Fix: Address clippy doc indentation warning

cccb0b8

alessandrobologna force-pushed the feature/add-otel-span-event branch from c54ba96 to f7cc03e Compare May 1, 2025 14:30

alessandrobologna force-pushed the feature/add-otel-span-event branch from 55880e7 to 78e8247 Compare May 2, 2025 12:35

djc approved these changes May 2, 2025

View reviewed changes

djc merged commit 85e28ac into tokio-rs:v0.1.x May 2, 2025
14 checks passed

		@@ -0,0 +1,65 @@
		use opentelemetry::trace::TracerProvider as _;

feat: Add OpenTelemetrySpanExt methods for direct span event creation #201

feat: Add OpenTelemetrySpanExt methods for direct span event creation #201

Uh oh!

Conversation

alessandrobologna commented Apr 26, 2025

Motivation

Solution

Uh oh!

alessandrobologna commented Apr 26, 2025

Uh oh!

djc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alessandrobologna commented May 1, 2025

Uh oh!

djc commented May 2, 2025

Uh oh!

alessandrobologna commented May 2, 2025

Uh oh!

djc commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

alessandrobologna commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

djc commented May 2, 2025 •

edited

Loading

alessandrobologna commented May 2, 2025 •

edited

Loading