Change event.name attribute into top-level event name field#4320
Change event.name attribute into top-level event name field#4320reyang merged 11 commits intoopen-telemetry:mainfrom
event.name attribute into top-level event name field#4320Conversation
trask
force-pushed
the
add-event-name-field
branch
from
3f68017 to
e994c1b
Compare
reyang
left a comment
reyang
left a comment
There was a problem hiding this comment.
LGTM, I left a nonblocking comment.
cijothomas
left a comment
cijothomas
left a comment
There was a problem hiding this comment.
💯 Looks good!
(coming from .NET, C++, Rust clients where EventName was a top-level field all the time.)
lalitb
left a comment
lalitb
left a comment
There was a problem hiding this comment.
+1 for this. Rust tokio tracing library already has "event name" as top-level field. Would be easy to model this into otel-rust with this change.
| - [Severity Text](./data-model.md#field-severitytext) (optional) | ||
| - [Body](./data-model.md#field-body) (optional) | ||
| - [Attributes](./data-model.md#field-attributes) (optional) | ||
| - **Status**: [Development](../document-status.md) - [Event Name](./data-model.md#event-name) (optional) |
There was a problem hiding this comment.
Looks like I'm late to the game, but here's some thoughts...
-
In .NET ILogger API we have CategoryName always and optional EventId structure (which contains Name & Id). So what goes here? EventId.Name may not be present and if it is, presumably, it is only unique the context of a CategoryName. So should this be prefixed? EventName = $"{CategoryName}.{EventName}" or do we also need something like EventNamespace defined?
-
Seems odd to introduce "Event" into the Log API given there is an "Event" API defined right below. Seems to invite confusion 😄 Why not just "Name" here?
There was a problem hiding this comment.
Seems odd to introduce "Event" into the Log API given there is an "Event" API defined right below
check out discussions at https://cloud-native.slack.com/archives/C062HUREGUV/p1733358994263159
since it's low level API it should support all logs features (including events), so it should take a name
The whole reason why the Go SIG was asking that the "Emit an Event" should be optional in the OTEP (see: open-telemetry/oteps#265 (comment)) is that after spending about a year designing the Logs API and SDK we felt that adding a separate method is a wrong decision.
There was a problem hiding this comment.
Why not just "Name" here?
it's going to map to new proto field event_name (open-telemetry/opentelemetry-proto#600)
chalin
left a comment
chalin
left a comment
There was a problem hiding this comment.
This PR has breaking changes, w.r.t. link checking, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/12313997571/job/34369378422?pr=5782.
See other inline comments. /cc @svrnm
| # Logs Data Model | ||
|
|
||
| **Status**: [Stable](../document-status.md) | ||
| **Status**: [Stable](../document-status.md), except where otherwise specified |
There was a problem hiding this comment.
I think that this should be reported as:
Status: Mixed
There was a problem hiding this comment.
"Stable, except where otherwise specified" seems more consistent with the rest of the spec? https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-specification%20except%20where%20otherwise%20specified&type=code
| - [Severity Text](./data-model.md#field-severitytext) (optional) | ||
| - [Body](./data-model.md#field-body) (optional) | ||
| - [Attributes](./data-model.md#field-attributes) (optional) | ||
| - **Status**: [Development](../document-status.md) - [Event Name](./data-model.md#event-name) (optional) |
There was a problem hiding this comment.
Note that #event-name is an invalid hash, see the website build failures in https://github.com/open-telemetry/opentelemetry.io/actions/runs/12313997571/job/34369378422?pr=5782
The hash should be #field-eventname.
There was a problem hiding this comment.
Also, might it be better to present this as:
- Event Name (optional, Status: Development)
10 participants
Fixes #4260
Changes
Converts event name from an attribute (
event.name) into a top-level field.