Skip to content

Conversation

aalexand
Copy link
Member

@aalexand aalexand commented Sep 4, 2025

…ields.

Protobuf
docs say:

Use pluralized names for repeated fields.

Other fields in profiles.proto and other OTLP types follow that recommended style AFAICT. The only fields that do not are line and sample, and I think those are remnants of the now-gone exact pprof format compatibility we explored early on.

As we approach the Alpha release of the profiles signal, let's be consistent in how we name repeated fields and use plural here.

@aalexand aalexand requested a review from a team September 4, 2025 20:29
@aalexand aalexand changed the title Rename line -> lines and sample -> samples since these are repeated f… Rename line -> lines and sample -> samples since these are repeated fields Sep 4, 2025
@aalexand
Copy link
Member Author

aalexand commented Sep 4, 2025

@open-telemetry/profiling-approvers

@reyang
Copy link
Member

reyang commented Sep 5, 2025

@aalexand would you update the changelog? Thanks!

…ields.

Protobuf
[docs](https://protobuf.dev/programming-guides/style/#field-names) say:

> Use pluralized names for repeated fields.

Other fields in profiles.proto and other OTLP types follow that
recommended style AFAICT. The only fields that do not are `line` and
`sample`, and I think those are remnants of the now-gone exact pprof
format compatibility we explored early on.

As we approach the Alpha release of the profiles signal, let's be
consistent in how we name repeated fields and use plural here.
@aalexand
Copy link
Member Author

aalexand commented Sep 5, 2025

@aalexand would you update the changelog? Thanks!

Done.

@reyang reyang merged commit 99fc853 into open-telemetry:main Sep 5, 2025
17 checks passed
## Unreleased

The full list of changes can be found in the compare view for the respective release at <https://github.com/open-telemetry/opentelemetry-proto/releases>.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants