Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename event payload to body #4035

Merged
merged 4 commits into from
May 16, 2024

Conversation

martinkuba
Copy link
Contributor

@martinkuba martinkuba commented May 6, 2024

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
Copy link
Member

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
Copy link
Member

trask commented May 7, 2024

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

@martinkuba
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor

tedsuo commented May 10, 2024

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 #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
Copy link

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.

@carlosalberto carlosalberto changed the title Rename event payload to data Rename event payload to body May 13, 2024
@reyang reyang merged commit f85fe4d into open-telemetry:main May 16, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet