Change event.name attribute into top-level event name field
#4320
Conversation
trask
force-pushed
the
add-event-name-field
branch
from
3f68017 to
e994c1b
Compare
| @@ -137,6 +137,7 @@ The API MUST accept the following parameters: | |||
| - [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
and https://cloud-native.slack.com/archives/C062HUREGUV/p1733398117155609
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)
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
| **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
| @@ -137,6 +137,7 @@ The API MUST accept the following parameters: | |||
| - [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)
Fixes #4260
Changes
Converts event name from an attribute (
event.name) into a top-level field.