search

open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.71k stars 887 forks source link

Rename event payload to body #4035

Closed martinkuba closed 4 months ago

martinkuba commented 4 months ago

We should be consistent in what we call the event payload/data/body field. This was discussed in the Event SIG meeting on 5/10 and agreed that it should be called body.

Related:

yurishkuro commented 4 months ago

discussed in the Event SIG meeting on 5/3 and agreed

As a point of order, this not an adequate explanation for the change. Decisions need to be documented.

trask commented 4 months ago

what are your thoughts around calling it "event body" (to align with the Logs SDK / proto field name)?

martinkuba commented 4 months ago

what are your thoughts around calling it "event body" (to align with the Logs SDK / proto field name)?

IMO, this does not need to align because it's matching the field in the API/data model to the specific OTLP protocol. I think event data is more general (less log specific), and seems more natural for describing the data that the event carries. With that said, there was a consensus in the Event SIG on event data but not necessarily a strong opinion.

For reference, we are already using data in these places:

JamieDanielson commented 4 months ago

what are your thoughts around calling it "event body" (to align with the Logs SDK / proto field name)?

If our general intention is to be more consistent, body is probably more consistent. We chose to use the same pipeline for EventLogger and Logger. If these are feeding into the same pipeline they should have the same named fields.

tedsuo commented 4 months ago

Ok, so we had further discussion in the Event SIG today, and dove deep into the definition and motivation for having events in OpenTelemetry. We realized that making these kinds of design decisions without first defining the purpose of events was becoming difficult and somewhat arbitrary.

You can find the proposed definition here https://github.com/open-telemetry/opentelemetry-specification/issues/4045.

Based on those definitions, we've changed our minds! Naming this field event or payload was originally proposed under the assumption that we wanted to differentiate events from logs. But in OpenTelemetry, we do in fact want events to be tightly associated with logs. It is important for end users to understand this. For this reason, the field should be called body, and we should change all of the references to data in the spec to body. Otherwise, users have to map these concepts in their heads, and that only creates confusion.

If there are objections we can of course discuss this further, but in that case I suggest we resolve the larger issue first. If there are no objections, I think it would be fine to update this PR to change everything to body and to go ahead and merge it.

alexvanboxel commented 4 months ago

I've looked at the recording of the Event SIG and am happy with the direction of using body. The first comment should probably be edited, as this is the first thing people will see when referencing the PR.