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.

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

[exporter/prometheusexporter] accumulate delta temporality metrics #9006

Closed
wants to merge 4 commits into from
Closed

[exporter/prometheusexporter] accumulate delta temporality metrics #9006

wants to merge 4 commits into from

Conversation

nabam
Copy link
Contributor

@nabam nabam commented Apr 1, 2022

Description:
Accumulate delta temporality metrics in prometheus exporter to expose as monotonically increasing counters

Link to tracking Issue:
related to #4968

@nabam nabam requested a review from a team April 1, 2022 18:41
@nabam nabam requested a review from Aneurysm9 as a code owner April 1, 2022 18:41
@nabam nabam changed the title prometheusexporter: accumulate delta temporality metrics [exporter/prometheusexporter] accumulate delta temporality metrics Apr 1, 2022
@github-actions
Copy link
Contributor

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 Apr 19, 2022
@nabam
Copy link
Contributor Author

nabam commented Apr 27, 2022

any chance someone can review this PR? @bogdandrutu

@github-actions github-actions bot removed the Stale label Apr 28, 2022
@github-actions
Copy link
Contributor

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 May 13, 2022
@locmai locmai mentioned this pull request May 19, 2022
Closed
@nabam
Copy link
Contributor Author

nabam commented May 19, 2022

@Aneurysm9 @bogdandrutu just a friendly poke here. Let me know if that's and appropriate approach and I'll rebase.

@github-actions github-actions bot removed the Stale label May 20, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2022

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 Jun 4, 2022
@nabam
Copy link
Contributor Author

nabam commented Jun 8, 2022 •
edited
Loading

it is still relevant
@jmacd maybe you can take a look?

@github-actions github-actions bot removed the Stale label Jun 12, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2022

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 4, 2022
@nabam
Copy link
Contributor Author

nabam commented Jul 7, 2022

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

not stale

@jmacd
Copy link
Contributor

jmacd commented Jul 8, 2022

Could you explain how this relates to #9919? It looks like it continues refining that work. The PR description could use some details on what changes are happening here, since the linked issue is quite broad. Thank you.

(I'm still eager to see this form of translation happen in both the Prometheus and PRW exporters.)

@github-actions github-actions bot removed the Stale label Jul 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2022

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 Aug 7, 2022
@jmacd
Copy link
Contributor

jmacd commented Aug 17, 2022

Answering my own question above. This adds support for Delta->Cumulative for histograms, whereas #9919 only fixed the problem for Sums. @nabam agree? Will you update this PR?

@nabam
Copy link
Contributor Author

nabam commented Aug 17, 2022

@jmacd yeah, I will. Sorry I haven't replied, was on vacation

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 18, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: nabam / name: Lev Popov (3bf08f26b55d92944dbb1d9e05f59cee1f0ab543)

@nabam
Copy link
Contributor Author

nabam commented Aug 18, 2022

@jmacd ready for your review

@jaronoff97 jaronoff97 mentioned this pull request Aug 20, 2022
Open
@mike-jc
Copy link

mike-jc commented Aug 23, 2022

Hi. Is there any estimated/guestimated time when this will be ready?

@github-actions github-actions bot removed the Stale label Sep 29, 2022
@danielgblanco danielgblanco mentioned this pull request Oct 5, 2022
Open
@github-actions
Copy link
Contributor

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 Oct 14, 2022
@dmitryax dmitryax removed the Stale label Oct 15, 2022
nabam and others added 2 commits October 18, 2022 11:45
@nabam @Aneurysm9
Update exporter/prometheusexporter/accumulator.go
ac88fb9 
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
@nabam @Aneurysm9
Update exporter/prometheusexporter/accumulator.go
b96d0f5 
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
@MovieStoreGuy
Copy link
Contributor

Could I ask you to rebase this PR @nabam ?

@Aneurysm9 Aneurysm9 mentioned this pull request Mar 1, 2023
Closed
@mx-psi mx-psi linked an issue Mar 2, 2023 that may be closed by this pull request
[exporter/prometheus] collector do not support metric histogram with temporality = DELTA #19153
Closed
@locmai locmai mentioned this pull request Mar 31, 2023
Closed
@bitomaxsp
Copy link

Lev has left. I'll have to do this. I'll inform you once this will be done.
We are really depend on this moving forward

@nabam
Copy link
Contributor Author

nabam commented Apr 24, 2023

Hi there! Sorry, I had very little time lately to look into this since I got a new job recently. I can see that @locmai took this PR over and created #20530. Thanks a lot for getting it done! I'm closing this PR.

@nabam nabam closed this Apr 24, 2023
@hkfgo hkfgo mentioned this pull request Jun 27, 2023
Merged
@leocavalcante leocavalcante mentioned this pull request Aug 31, 2023
Closed
djaglowski pushed a commit that referenced this pull request Jan 8, 2024
@hkfgo @nabam
@Aneurysm9 @locmai
[exporter/prometheusexporter] accumulate delta temporality histograms (…
aaaaa0c 
…#23790)

**Description:** <Describe what has changed.>
This continues the work done in the now closed
[PR](#20530).
I have addressed issues raised in the original PR by
- Adding logic to handle timestamp misalignments 
- Adding fix + a out-of-bounds bug  

In addition, I have performed end-to-end testing in a local setup, and
confirmed that accumulated histogram time series are correct.

**Link to tracking Issue:** <Issue number if applicable>

#4968

#9006

#19153
**Testing:** <Describe what testing was performed and which tests were
added.>
Added tests for timestamp misalignment and an out-of-bounds bug
discovered in the previous PR.
End-to-end testing to ensure histogram bucket counts exported to
Prometheus are correct

---------

Signed-off-by: Loc Mai <locmai0201@gmail.com>
Signed-off-by: xchen <xchen@axon.com>
Signed-off-by: stephenchen <x.chen1016@gmail.com>
Co-authored-by: Lev Popov <nabam@nabam.net>
Co-authored-by: Lev Popov <leo@nabam.net>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Loc Mai <locmai0201@gmail.com>
Co-authored-by: Alex Boten <aboten@lightstep.com>
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jan 10, 2024
@hkfgo @nabam
@Aneurysm9 @locmai @cparkins
[exporter/prometheusexporter] accumulate delta temporality histograms (…
1f5677a 
…open-telemetry#23790)

**Description:** <Describe what has changed.>
This continues the work done in the now closed
[PR](open-telemetry#20530).
I have addressed issues raised in the original PR by
- Adding logic to handle timestamp misalignments 
- Adding fix + a out-of-bounds bug  

In addition, I have performed end-to-end testing in a local setup, and
confirmed that accumulated histogram time series are correct.

**Link to tracking Issue:** <Issue number if applicable>

open-telemetry#4968

open-telemetry#9006

open-telemetry#19153
**Testing:** <Describe what testing was performed and which tests were
added.>
Added tests for timestamp misalignment and an out-of-bounds bug
discovered in the previous PR.
End-to-end testing to ensure histogram bucket counts exported to
Prometheus are correct

---------

Signed-off-by: Loc Mai <locmai0201@gmail.com>
Signed-off-by: xchen <xchen@axon.com>
Signed-off-by: stephenchen <x.chen1016@gmail.com>
Co-authored-by: Lev Popov <nabam@nabam.net>
Co-authored-by: Lev Popov <leo@nabam.net>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Loc Mai <locmai0201@gmail.com>
Co-authored-by: Alex Boten <aboten@lightstep.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aneurysm9 Aneurysm9 Aneurysm9 left review comments

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

@dmitryax dmitryax Awaiting requested review from dmitryax

@jpkrohling jpkrohling Awaiting requested review from jpkrohling

@djaglowski djaglowski Awaiting requested review from djaglowski

@mx-psi mx-psi Awaiting requested review from mx-psi

@TylerHelmuth TylerHelmuth Awaiting requested review from TylerHelmuth

@bogdandrutu bogdandrutu Awaiting requested review from bogdandrutu

@jsuereth jsuereth Awaiting requested review from jsuereth

@punya punya Awaiting requested review from punya

@arminru arminru Awaiting requested review from arminru

@codeboten codeboten Awaiting requested review from codeboten

Assignees

@bogdandrutu bogdandrutu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[exporter/prometheus] collector do not support metric histogram with temporality = DELTA
8 participants
@nabam @jmacd @mike-jc @MovieStoreGuy @bitomaxsp @Aneurysm9 @bogdandrutu @dmitryax