Skip to content

model/profile: add symbolization information#2522

Merged
lmolkova merged 13 commits intoopen-telemetry:mainfrom
florianl:profiles-symb-level
Sep 11, 2025
Merged

model/profile: add symbolization information#2522
lmolkova merged 13 commits intoopen-telemetry:mainfrom
florianl:profiles-symb-level

Conversation

@florianl
Copy link
Copy Markdown
Member

Changes

Add Profiling specific attributes that describe the symbolization level of a Mapping.

FYI: @open-telemetry/profiling-approvers

Merge requirement checklist

  • CONTRIBUTING.md guidelines followed.
  • Change log entry added, according to the guidelines in When to add a changelog entry.
    • If your PR does not need a change log, start the PR title with [chore]
  • Links to the prototypes or existing instrumentations (when adding or changing conventions)

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@florianl florianl force-pushed the profiles-symb-level branch from 158b6a4 to b92f85c Compare July 15, 2025 12:20
Comment thread docs/general/profiles.md Outdated
Comment thread docs/general/profiles.md Outdated
@aalexand
Copy link
Copy Markdown
Member

I think we agreed here to add these as pprof.* boolean attributes equivalent to the current pprof's proto attributes since use cases other than pprof's were unclear to us and for pprof keeping the current semantics is the easiest for the migration.

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@florianl florianl force-pushed the profiles-symb-level branch from 55c2de6 to b139af0 Compare July 16, 2025 12:19
@florianl
Copy link
Copy Markdown
Member Author

@aalexand thanks for reminding me about the pprof group. I did forget about this 🙈 updated the PR accordingly

But I don't recall from the recoding, that we agreed on a 1:1 mapping from the google/pprof has_* fields. To me, the suggestions you made in open-telemetry/opentelemetry-proto#595, made sense as they add information about the quality of the symbol information. Using the 1:1 mapping from the google/pprof has_* fields does not allow the backend to make a sound decision on the quality of the received information.

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Comment thread model/profile/common.yaml Outdated
Comment thread model/profile/registry.yaml Outdated
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
florianl added a commit to florianl/opentelemetry-proto that referenced this pull request Jul 30, 2025
Similar to the boolean attributes `Mapping.has_*` (open-telemetry#595 and open-telemetry/semantic-conventions#2522) also drop `Location.is_folded`.


The complementary PR for the OTel SemConv, that builds on top of open-telemetry/semantic-conventions#2522 is florianl/semantic-conventions#1
@lmolkova lmolkova moved this from Untriaged to Awaiting codeowners approval in Semantic Conventions Triage Jul 31, 2025
@aalexand
Copy link
Copy Markdown
Member

aalexand commented Aug 7, 2025

Given open-telemetry/opentelemetry-proto#690 and the discussion in the Slack, what do you think about making adding the is_folded attribute a part of this PR as well? I think it might be easier and more convenient to review adding all of these at once. This will help ensure consistency in naming and typing etc.

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@florianl
Copy link
Copy Markdown
Member Author

florianl commented Aug 7, 2025

Cherry-picked and so added the is_folded change here as well.

@thompson-tomo
Copy link
Copy Markdown
Contributor

thompson-tomo commented Aug 7, 2025

Hmm, I am not sure about this proposal here. Is it the idea that the collector would transform a profile message to become a span hence wanting attribute to represent the low level message properties?

Keep in mind the idea of adding attributes to the registry is so that they can be the key & sometimes value used in the attributes property of the message ie https://github.com/open-telemetry/opentelemetry-proto/blob/8654ab7a5a43ca25fe8046e59dcd6935c3f76de0/opentelemetry/proto/profiles/v1development/profiles.proto#L114

If you are wanting to define a profile message hence using attributes to indicate if that message should have functions for instance I would recommend you raise an issue weaver listing what are the properties of a profile.

It might help to define how these attributes would be used,

tigrannajaryan pushed a commit to open-telemetry/opentelemetry-proto that referenced this pull request Aug 7, 2025
The has_* debug info fields were derived from the pprof format. We decided to move them to attributes, see open-telemetry/semantic-conventions#2522.
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
tigrannajaryan added a commit to open-telemetry/opentelemetry-proto that referenced this pull request Aug 11, 2025
Similar to the boolean attributes `Mapping.has_*` (#595 and open-telemetry/semantic-conventions#2522) also drop `Location.is_folded`.


The complementary PR for the OTel SemConv, that builds on top of open-telemetry/semantic-conventions#2522 is florianl/semantic-conventions#1

Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
Comment thread model/profile/common.yaml Outdated
@aalexand
Copy link
Copy Markdown
Member

@jhalliday @christos68k to take a look also

Comment thread model/profile/common.yaml Outdated
Comment thread docs/registry/attributes/profile.md Outdated
Comment thread docs/registry/attributes/profile.md Outdated
Comment thread model/profile/registry.yaml Outdated
Comment thread model/profile/common.yaml Outdated
Comment thread model/profile/registry.yaml Outdated
Co-authored-by: Christos Kalkanis <christos.kalkanis@elastic.co>
Comment thread model/profile/registry.yaml Outdated
Comment thread model/profile/registry.yaml Outdated
Comment thread docs/registry/attributes/profile.md Outdated
Comment thread docs/registry/attributes/profile.md Outdated
Copy link
Copy Markdown
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, thanks @florianl 🙇

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@lmolkova lmolkova moved this from Awaiting codeowners approval to Needs More Approval in Semantic Conventions Triage Sep 8, 2025
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@lmolkova lmolkova added this pull request to the merge queue Sep 11, 2025
Merged via the queue into open-telemetry:main with commit d5d54cb Sep 11, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:profile enhancement New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

9 participants