Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Add support for Cumulative API #480

Merged
merged 3 commits into from
Apr 11, 2019

Conversation

mayurkale22
Copy link
Member

Updates #450

Adding Derived Cumulative API and registering with Metric-Registry will be in a follow-up PR.

@codecov-io
Copy link

codecov-io commented Apr 9, 2019

Codecov Report

Merging #480 into master will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #480     +/-   ##
=========================================
+ Coverage   94.74%   94.84%   +0.1%     
=========================================
  Files         148      150      +2     
  Lines        9946    10106    +160     
  Branches      747      752      +5     
=========================================
+ Hits         9423     9585    +162     
+ Misses        523      521      -2
Impacted Files Coverage Δ
src/common-utils.ts 95.83% <0%> (-4.17%) ⬇️
src/index.ts 100% <0%> (ø) ⬆️
src/http-stats.ts 100% <0%> (ø) ⬆️
src/metrics/gauges/gauge.ts 100% <0%> (ø) ⬆️
src/grpc-stats/server-stats.ts 100% <0%> (ø) ⬆️
src/grpc-stats/client-stats.ts 100% <0%> (ø) ⬆️
src/grpc-stats/common-distributions.ts 100% <0%> (ø) ⬆️
src/grpc-stats/stats-common.ts 100% <0%> (ø) ⬆️
test/test-cumulative.ts 100% <0%> (ø)
src/metrics/cumulative/cumulative.ts 100% <0%> (ø)
... and 1 more

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 abd41db...6444d01. Read the comment docs.

Copy link
Contributor

@draffensperger draffensperger left a comment

Choose a reason for hiding this comment

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

LGTM pending a few comments

* @param {number} val The new value.
*/
inc(val?: number): void {
if (val && !Number.isFinite(val)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this catch NaN (I'm guessing not), and assuming not, could you make it do that?

packages/opencensus-core/src/metrics/types.ts Show resolved Hide resolved
packages/opencensus-core/src/metrics/types.ts Show resolved Hide resolved
@mayurkale22 mayurkale22 requested a review from songy23 April 10, 2019 02:34
Copy link
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM

@mayurkale22 mayurkale22 merged commit 1853dc9 into census-instrumentation:master Apr 11, 2019
@mayurkale22 mayurkale22 deleted the cumulative_api branch April 11, 2019 02:59
@mayurkale22 mayurkale22 added this to the Release 0.0.12 milestone May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants