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

Plug-in Derived Cumulative into the Metric-Registry #513

Conversation

mayurkale22
Copy link
Member

@mayurkale22 mayurkale22 commented May 9, 2019

Fixes #450

Completed earlier PRs.

  • Add support for Cumulative API
  • Plug-in Cumulative APIs into the Metric-Registry
  • Add support for Derived Cumulative API
  • Plug-in Derived Cumulative APIs into the Metric-Registry (current)

*/
addDerivedInt64Cumulative(name: string, options?: MetricOptions):
DerivedCumulative {
const description =
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like some of this code is duplicated with other metric adding functions. That isn't necessarily bad but makes me wonder if there would be a more templatized approach to the different metric types. (On the other hand, Duplication is better than the wrong abstraction so I think this is fine)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, thanks.

@mayurkale22 mayurkale22 added this to the Release 0.0.12 milestone May 10, 2019
@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
+ Coverage   94.78%   94.96%   +0.17%     
==========================================
  Files         147      147              
  Lines       10486    10780     +294     
  Branches      907      913       +6     
==========================================
+ Hits         9939    10237     +298     
+ Misses        547      543       -4
Impacted Files Coverage Δ
src/trace/propagation/noop-propagation.ts 40% <0%> (-10%) ⬇️
src/common/noop-logger.ts 11.11% <0%> (-5.56%) ⬇️
src/stackdriver-monitoring.ts 80.89% <0%> (-1.13%) ⬇️
src/internal/string-utils.ts 100% <0%> (ø) ⬆️
test/test-tag-map.ts 100% <0%> (ø) ⬆️
src/resource-labels.ts 100% <0%> (ø) ⬆️
test/test-metric-producer-manager.ts 100% <0%> (ø) ⬆️
test/test-exporter-buffer.ts 100% <0%> (ø) ⬆️
test/test-metric-component.ts 100% <0%> (ø) ⬆️
test/test-sampler.ts 100% <0%> (ø) ⬆️
... and 30 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 8cc92a4...d43b8a1. Read the comment docs.

@mayurkale22 mayurkale22 merged commit 88a4ab1 into census-instrumentation:master May 10, 2019
@mayurkale22 mayurkale22 deleted the hook_derived_cumulative_api_to_registry branch May 10, 2019 21:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metrics: Add Cumulative APIs
4 participants