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.

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

Verify prev point is returned for cumulative readers instead of None #2755

Closed
srikanthccv opened this issue Jun 10, 2022 · 1 comment · Fixed by #2773
Closed

Verify prev point is returned for cumulative readers instead of None #2755

srikanthccv opened this issue Jun 10, 2022 · 1 comment · Fixed by #2773
Assignees
Labels
metrics sdk Affects the SDK package.

Comments

@srikanthccv
Copy link
Member

This LGTM. For cumulative readers though, we need to make sure they return the previous point value instead of None. For example, with a prometheus exporter:

counter.add(10, {"foo": "bar"})
# gets scraped, reports 10 for {foo="bar"}
# ... no measurements

# gets scraped again, should continue to report 10 for {foo="bar"}
# even though there were no measurements.

Do you know if that is the case? If not we should open another issue for rc2.

Originally posted by @aabmass in #2745 (review)

@srikanthccv srikanthccv added sdk Affects the SDK package. metrics 1.12.0rc2 labels Jun 10, 2022
@ocelotl ocelotl removed the 1.12.0rc2 label Jun 13, 2022
@ocelotl ocelotl moved this to In Progress in Python Metrics RC2 Jun 15, 2022
@ocelotl ocelotl self-assigned this Jun 15, 2022
@ocelotl
Copy link
Contributor

ocelotl commented Jun 16, 2022

I am a bit confused, what is a "cumulative reader"? Is it an arbitrary designation or can we find out if a MetricReader is cumulative from looking into a certain characteristic of the MetricReader?

ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jun 21, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jun 21, 2022
@ocelotl ocelotl moved this from In Progress to In Review in Python Metrics RC2 Jun 22, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jun 27, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jun 30, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jun 30, 2022
ocelotl added a commit that referenced this issue Jun 30, 2022
* Verify previous point is returned for cumulative instruments

Fixes #2755

* Reject non-cumulative temporalities for Prometheus metric reader

* Add test case for InMemoryMetricReader

* Fix prometheus import paths

* Remove parameters for Prometheus init

* Add sleep for windows tests

* Add temporality overriding dictionary
Repository owner moved this from In Review to Done in Python Metrics RC2 Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics sdk Affects the SDK package.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants