Skip to content

profiles: Move lookup tables to ProfilesData#644

Merged
tigrannajaryan merged 2 commits intoopen-telemetry:mainfrom
florianl:issue-628
Apr 24, 2025
Merged

profiles: Move lookup tables to ProfilesData#644
tigrannajaryan merged 2 commits intoopen-telemetry:mainfrom
florianl:issue-628

Conversation

@florianl
Copy link
Copy Markdown
Member

Implements #628 (comment).

Implements open-telemetry#628 (comment).

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Co-authored-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Co-authored-by: Nayef Ghattas <11560964+Gandem@users.noreply.github.com>
@florianl florianl requested review from a team April 16, 2025 11:54
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto
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.

Thanks, this is a great step forward 🙇 ! LGTM overall.

Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
@petethepig
Copy link
Copy Markdown
Member

I like this approach — it gets us everything we want, with the only downside being more complexity in merging Profiles. Since it's a one-time, internal change in the collector, I think it's worth it.

@petethepig petethepig self-requested a review April 17, 2025 15:45
Copy link
Copy Markdown
Member

@christos68k christos68k left a comment

Choose a reason for hiding this comment

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

Added some suggestions for clarity and consistency

Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Comment thread opentelemetry/proto/profiles/v1development/profiles.proto Outdated
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@tigrannajaryan
Copy link
Copy Markdown
Member

I will merge this tomorrow. We normally merge 2 days after the last change.

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.