Skip to content

[connector/servicegraphconnector] Add native histograms support #39951

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[connector/servicegraphconnector] Add native histograms support #39951

wants to merge 1 commit into from
+406 −33

Conversation

Frapschen
Copy link
Contributor

Description

issue: #35460

@Frapschen Frapschen requested review from djaglowski and a team as code owners May 8, 2025 16:07
@Frapschen Frapschen requested a review from mapno May 8, 2025 16:08
@github-actions github-actions bot requested a review from JaredTan95 May 8, 2025 16:08
@Frapschen
Copy link
Contributor Author

After This PR merged, the spanmetrics connector also can switch to pdatautil.ExpoHistToExponentialDataPoint

JaredTan95
JaredTan95 approved these changes May 9, 2025
View reviewed changes
Copy link
Member

@JaredTan95 JaredTan95 left a comment

Choose a reason for hiding this comment

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

LGTM

@mapno any suggestions?

@djaglowski djaglowski removed their request for review May 19, 2025 15:55
@mapno
Copy link
Contributor

mapno commented May 22, 2025

I'm curious, why is it using github.com/lightstep/go-expohisto instead of OTel's native implentation of exponential histograms?

@Frapschen
Copy link
Contributor Author

#19144 introduce the github.com/lightstep/go-expohisto

A friendly ping to @kovrus, could you please explain why to use this package?

@kovrus
Copy link
Member

kovrus commented Jun 4, 2025 •
edited
Loading

#19144 introduce the github.com/lightstep/go-expohisto

A friendly ping to @kovrus, could you please explain why to use this package?

I honestly do not remember since I haven't touched Otel stuff for the last couple of years. I think back then there was no Go implementation of the exp histogram or it was beta? You can validate that.

@mar4uk mar4uk mentioned this pull request Jun 9, 2025
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 1, 2025

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 1, 2025
@github-actions GitHub Actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jul 15, 2025
@Frapschen Frapschen reopened this Jul 28, 2025
@Frapschen
Copy link
Contributor Author

@mapno Can you give some docs about OTel's native implentation of exponential histograms

@Frapschen
Copy link
Contributor Author

@mapno, I suggest we implement this feature with github.com/lightstep/go-expohisto first(spanmetrics has used it for a long time), then create a separate issue to evaluate using the native exponential histogram implementation.

@github-actions github-actions bot removed the Stale label Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mar4uk mar4uk mar4uk approved these changes

@JaredTan95 JaredTan95 JaredTan95 approved these changes

@mapno mapno Awaiting requested review from mapno

@TylerHelmuth TylerHelmuth Awaiting requested review from TylerHelmuth TylerHelmuth is a code owner

@evan-bradley evan-bradley Awaiting requested review from evan-bradley evan-bradley is a code owner

@edmocosta edmocosta Awaiting requested review from edmocosta edmocosta is a code owner

@mx-psi mx-psi Awaiting requested review from mx-psi mx-psi is a code owner

@songy23 songy23 Awaiting requested review from songy23 songy23 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@mx-psi mx-psi

Labels
connector/exceptions connector/servicegraph exporter/datadog Datadog components internal/pdatautil processor/transform Transform processor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Frapschen @mapno @kovrus @mar4uk @JaredTan95 @mx-psi