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

Remove monotonic and absolute metrics intrument options #410

Merged

Conversation

mauriciovasquezbernal
Copy link
Member

Fixes #404.

Following the spec open-telemetry/opentelemetry-specification#430.

Just some cleanup before implementing other things in the metrics.

@mauriciovasquezbernal mauriciovasquezbernal requested a review from a team February 11, 2020 21:15
@codecov-io
Copy link

codecov-io commented Feb 11, 2020

Codecov Report

Merging #410 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #410      +/-   ##
==========================================
- Coverage   85.85%   85.76%   -0.09%     
==========================================
  Files          41       41              
  Lines        2078     2065      -13     
  Branches      242      239       -3     
==========================================
- Hits         1784     1771      -13     
  Misses        223      223              
  Partials       71       71
Impacted Files Coverage Δ
...elemetry-api/src/opentelemetry/metrics/__init__.py 95.58% <ø> (ø) ⬆️
...etry-sdk/src/opentelemetry/sdk/metrics/__init__.py 98.29% <ø> (-0.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe99dbc...e07cefa. Read the comment docs.

Copy link
Member

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

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

LGTM, Monotonicity support will be discussed and added at later time, currently in Future Work: Option Support in the spec

Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

No reason to hold this up, but note that we'll need input validation when we have default aggregators for both counter and measure instruments.

@c24t c24t merged commit 96943b2 into open-telemetry:master Feb 14, 2020
toumorokoshi pushed a commit to toumorokoshi/opentelemetry-python that referenced this pull request Feb 17, 2020
@mauriciovasquezbernal mauriciovasquezbernal deleted the mauricio/remove-monotonic-absolute branch April 14, 2020 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove absolute, monotonic metric instrument options
4 participants