Skip to content

Add event.data attribute to provide mapping from span events to events#954

Closed
lmolkova wants to merge 6 commits intoopen-telemetry:mainfrom
lmolkova:event-body-mapping
Closed

Add event.data attribute to provide mapping from span events to events#954
lmolkova wants to merge 6 commits intoopen-telemetry:mainfrom
lmolkova:event-body-mapping

Conversation

@lmolkova
Copy link
Copy Markdown
Member

@lmolkova lmolkova commented Apr 24, 2024

Related to #829, #695, open-telemetry/opentelemetry-specification#3406

Changes

In GenAI WG, we'd like to be able to document semantic conventions for events, prototype and ship instrumentation libraries that emit them.

Event API is not available in the majority of languages yet, log are also still in development in many of them and the instrumentation libraries should not use logging bridge and produce log records anyway.

As explained in #829, we'd like to record events payload as if they were reported with Event API, but record them as span events until Event API is widely available.

This PR introduces event.body attribute that can be used to report event payload on a span event (available and stable everywhere).

The attribute records the payload as a JSON string.
Note that a "plain string" is a valid JSON - see RFC 7159

o Changed the definition of "JSON text" so that it can be any JSON
value, removing the constraint that it be an object or array.

TODO

  • Send spec PR to introduce mapping to transform span events to events.

Merge requirement checklist

@lmolkova lmolkova requested review from a team April 24, 2024 03:29
@lmolkova lmolkova changed the title Adds event.body attribute to provide mapping from span events to proper events Add event.body attribute to provide mapping from span events to events Apr 24, 2024
@tedsuo
Copy link
Copy Markdown

tedsuo commented Apr 26, 2024

This makes sense as a way to define recording events as span events. Even once events are stable, there will still be cases where end users want the SDK/Collector to record events that have a span context attached as span events, so this is useful in the long term.

Comment thread docs/attributes-registry/event.md Outdated
@lmolkova lmolkova force-pushed the event-body-mapping branch from 4043778 to 5e76492 Compare April 29, 2024 00:02
@lmolkova lmolkova changed the title Add event.body attribute to provide mapping from span events to events Add event.data attribute to provide mapping from span events to events Apr 29, 2024
| Attribute | Type | Description | Examples | Stability |
|---|---|---|---|---|
| `event.name` | string | Identifies the class / type of event. [1] | `browser.mouse.click`; `device.app.lifecycle` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `event.data` | string | The event payload serialized into JSON string. [1] | `{"role":"user","content":"how to use OTel Event API?"}`; `"plain string"` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is aligned with the Body semantics of a LogRecord, why is it a string and not Any? Even in #880 there is no strict requirement that it needs to be JSON.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trask
Copy link
Copy Markdown
Member

trask commented May 1, 2024

Even once events are stable, there will still be cases where end users want the SDK/Collector to record events that have a span context attached as span events

I was curious about this, do you have scenarios in mind where this would be helpful/needed? Thanks

@austinlparker
Copy link
Copy Markdown
Member

Even once events are stable, there will still be cases where end users want the SDK/Collector to record events that have a span context attached as span events

I was curious about this, do you have scenarios in mind where this would be helpful/needed? Thanks

Well, we have a few places where this pattern is used already - gRPC instrumentation jumps to mind. I've personally demonstrated span events in modeling a streaming operation by creating events as part of a callback to consume data from a stream --

func readStream
   stream.on{
       data:
           span.addEvent('got data')
       complete:
           span.addEvent('done')
       error:
           span.addEvent('error', err)
    }

@trask
Copy link
Copy Markdown
Member

trask commented May 7, 2024

Even once events are stable, there will still be cases where end users want the SDK/Collector to record events that have a span context attached as span events

I was curious about this, do you have scenarios in mind where this would be helpful/needed? Thanks

Well, we have a few places where this pattern is used already - gRPC instrumentation jumps to mind

I'm hoping we can migrate gRPC semantic conventions to use (log-based) events. We discussed in semconv meeting that there should be a way for users to opt into emitting log-based events as span events in the short term until backends (e.g. Jaeger) support log-based events.

@lmolkova
Copy link
Copy Markdown
Member Author

lmolkova commented May 7, 2024

The existing span events (all but exceptions) can be mapped to log-based events without introducing event.body - they don't define any.

Based on the discussions in the Spec and Semconv WG, we'd try to leverage Event API (or log API in the short term) to emit events with body instead of adding mapping to span events.
The would help us to retire span events sooner and invest into Event API where it's lacking.

The attribute like event.data that would be essential for Event API -> log-based events mapping (if it's needed).

So I'm going to close this PR.

@lmolkova lmolkova closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

8 participants