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

Prometheus Collect() should check return value from CheckpointSet ForEach #563

Closed
evantorrie opened this issue Mar 17, 2020 · 0 comments · Fixed by #622
Closed

Prometheus Collect() should check return value from CheckpointSet ForEach #563

evantorrie opened this issue Mar 17, 2020 · 0 comments · Fixed by #622
Labels
area:metrics Part of OpenTelemetry Metrics
Milestone
Implement v0.5.0 ...

Comments

@evantorrie
Copy link
Contributor

#557 changed the CheckpointSet.ForEach() interface to

  • return an error from ForEach
  • allow the function passed to ForEach() to also return an error

The Prometheus Collect() makes use of CheckpointSet.ForEach() to iterate over the records. This code should be modified to reflect the change in error reporting behavior. For example, ErrNoData values could be returned from each of the export{Summary,Counter,LastValue} routines when appropriate, and any error returned from ForEach() should likely be reported via the error handler.
@evantorrie evantorrie changed the title Prometheus Collect() should check return value from CheckpointSet.ForEach Prometheus Collect() should check return value from CheckpointSet ForEach Mar 17, 2020
@evantorrie evantorrie mentioned this issue Mar 17, 2020
Merged
@jmacd jmacd added the area:metrics Part of OpenTelemetry Metrics label Mar 17, 2020
@rghetia rghetia added this to the Alpha v0.4 milestone Mar 26, 2020
oncilla added a commit to oncilla/opentelemetry-go that referenced this issue Apr 6, 2020
@oncilla
prometheus: check return value from CheckpointSet.ForEach
7aad502 
This PR modifies prometheus.Collect to reflect the change introduced
by open-telemetry#557.

The `export{Counter,Histogram,LastValue,Summary}` methods now all return
an error instead of calling the error callback directly.
The callback is now only called on the returned error from `ForEach`.

fixes open-telemetry#563
@oncilla oncilla mentioned this issue Apr 6, 2020
Merged
rghetia added a commit that referenced this issue Apr 7, 2020
@oncilla @rghetia
prometheus: check return value from CheckpointSet.ForEach (#622)
6005d01 
This PR modifies prometheus.Collect to reflect the change introduced
by #557.

The `export{Counter,Histogram,LastValue,Summary}` methods now all return
an error instead of calling the error callback directly.
The callback is now only called on the returned error from `ForEach`.

fixes #563

Co-authored-by: Rahul Patel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:metrics Part of OpenTelemetry Metrics
Projects
None yet
Milestone
Implement v0.5.0 Specification
Development

Successfully merging a pull request may close this issue.

prometheus: check return value from CheckpointSet.ForEach oncilla/opentelemetry-go
3 participants
@jmacd @evantorrie @rghetia