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

Missing metrics creator fns #425

Merged
merged 3 commits into from
Jan 15, 2021

Conversation

markdingram
Copy link
Contributor

Looks a bit messy, first commit is a consistent reordering of the existing creator fns.

Second commit adds a few that were missing.

@markdingram markdingram requested a review from a team January 13, 2021 23:09
@markdingram markdingram changed the title Missing creator fns Missing metrics creator fns Jan 13, 2021
@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #425 (5a68470) into master (a56337c) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   51.55%   51.57%   +0.01%     
==========================================
  Files          65       65              
  Lines        5117     5117              
==========================================
+ Hits         2638     2639       +1     
+ Misses       2479     2478       -1     
Impacted Files Coverage Δ
opentelemetry/src/metrics/meter.rs 50.00% <ø> (ø)
...ntelemetry/src/sdk/metrics/aggregators/ddsketch.rs 76.95% <0.00%> (+0.19%) ⬆️

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 a56337c...5a68470. Read the comment docs.

@jtescher
Copy link
Member

Code changes look good, can either address docs first or in follow up PR

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
- same order as api spec (sync then async)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@jtescher jtescher merged commit 777fdb9 into open-telemetry:master Jan 15, 2021
jtescher added a commit that referenced this pull request Jan 18, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This reverts commit 777fdb9.
jtescher added a commit that referenced this pull request Jan 18, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This reverts commit 777fdb9.
jtescher added a commit that referenced this pull request Jan 18, 2021
This reverts some of the extra instruments introduced in #425.
Specifically `Meter::u64_up_down_counter` and
`Meter::u64_up_down_sum_observer` which are not necessary because of
their type (e.g. a monotonically increasing  `up_down_counter` is simply
a `counter`).
jtescher added a commit that referenced this pull request Jan 18, 2021
This reverts some of the extra instruments introduced in #425.
Specifically `Meter::u64_up_down_counter` and
`Meter::u64_up_down_sum_observer` which are not necessary because of
their type (e.g. a monotonically increasing  `up_down_counter` is simply
a `counter`).
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.

None yet

4 participants