profiles: drop Sample.stacktrace_id_index#575
profiles: drop Sample.stacktrace_id_index#575tigrannajaryan merged 2 commits intoopen-telemetry:mainfrom
Conversation
The value for Sample.stacktrace_id_index is hardly defined except for its length. It is also not defined how this value should be generated, interpreted and could be used. Some kind of stacktrace_id can be most efficient in stateful protocols, where transmitting duplicate information should be avoided. As the OTel Profiling protocol is a stateless protocol, this element can be droped.
simonswine
left a comment
simonswine
left a comment
There was a problem hiding this comment.
LGTM
I guess we should also update the model in the OTEP: https://github.com/open-telemetry/oteps/blob/87b36a69cf8c9eb6fbb8375d7bd7a1794ed976fe/text/profiles/0239-profiles-data-model.md?plain=1#L528-L529
|
LGTM |
|
Breaking change check fails since a filed is deleted: https://github.com/open-telemetry/opentelemetry-proto/actions/runs/10503389833/job/29096669193?pr=575 This is fine for Experimental status. We need to modify the breaking change detector to avoid profiling protos for now (or have a different/weaker checksfor profiling protos if that makes more sense). I suggest to do it in a separate PR. If you propose a PR that fixes this I can merge it first, and that will unblock this PR. |
I'm actually not sure. I thought OTEPs are meant to capture consensus around technical decisions at a point in time, but are not intended to become evergreen documentation? 🤔 |
|
@tigrannajaryan with #576 I have opened a PR to exclude the Profiles protocol from the breaking-changes check. |
Correct. Any substantial changes are typically done via a new OTEP (that can amend an old OTEP). If changes are not big enough to warrant a new OTEP, they are done directly on the impacted repository. |
|
#576 is merged. Please rebase, it should fix the build. |
|
friendly ping @yurishkuro |
|
The PR has enough approvals, merging. (@florianl when the PR is ready to be merged ping me I can merge it). |
9 participants
The value for Sample.stacktrace_id_index is hardly defined except for its length. It is also not defined how this value should be generated, interpreted and could be used.
Some kind of stacktrace_id can be most efficient in stateful protocols, where transmitting duplicate information should be avoided. As the OTel Profiling protocol is a stateless protocol, this element can be droped.
The proposed change is a breaking change. As the OTel Profiling signal is still marked as experimental this should be fine? Please let me know.
FYI: @open-telemetry/profiling-maintainers