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

process.executable.build_id: rename attribute #1520

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

florianl
Copy link
Contributor

Changes

With open-telemetry/opentelemetry-specification#4197 it was decided to rename the attribute from process.executable.build_id.profiling to process.executable.build_id.htlhash (Head-Tail-Length Hash).

FYI @open-telemetry/profiling-maintainers

Merge requirement checklist

@florianl florianl requested review from a team as code owners October 28, 2024 07:45
@florianl florianl force-pushed the profiling-htlhash branch 3 times, most recently from 7d85188 to 40e4902 Compare October 28, 2024 07:52
Copy link
Member

@felixge felixge left a comment

Choose a reason for hiding this comment

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

LGTM.

Is this level of change tracking needed while we're still experimental?

@@ -75,6 +75,7 @@ Deprecated process attributes.
| Attribute | Type | Description | Examples | Stability |
|---|---|---|---|---|
| <a id="process-cpu-state" href="#process-cpu-state">`process.cpu.state`</a> | string | Deprecated, use `cpu.mode` instead. | `system`; `user`; `wait` | ![Deprecated](https://img.shields.io/badge/-deprecated-red)<br>Replaced by `cpu.mode` |
| <a id="process-executable-build-id-profiling" href="#process-executable-build-id-profiling">`process.executable.build_id.profiling`</a> | string | "Deprecated, use `process.executable.build_id.htlhash` instead." | | ![Deprecated](https://img.shields.io/badge/-deprecated-red)<br>Replaced by `process.executable.build_id.htlhash` |
Copy link
Member

@christos68k christos68k Oct 28, 2024

Choose a reason for hiding this comment

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

Since AFAIK we haven't released a profiling protocol version that includes build_id.profiling which means that we shouldn't be breaking anyone other than ourselves, maybe we shouldn't list it as deprecated to avoid carrying unnecessary baggage forward, if that's OK with other semconv maintainers and approvers.

Copy link
Contributor Author

@florianl florianl Oct 28, 2024

Choose a reason for hiding this comment

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

Other attributes, that got renamed and are marked as experimental, also require this approach - e.g. #1422

maybe @open-telemetry/specs-semconv-maintainers can provide more feedback and guidance

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

Successfully merging this pull request may close these issues.

3 participants