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

Add gauge example #460

Merged

Conversation

mayurkale22
Copy link
Member

Fixes #459

@codecov-io
Copy link

codecov-io commented Apr 2, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #460      +/-   ##
==========================================
+ Coverage    94.7%   94.73%   +0.03%     
==========================================
  Files         150      150              
  Lines        9785     9785              
  Branches      738      735       -3     
==========================================
+ Hits         9267     9270       +3     
+ Misses        518      515       -3
Impacted Files Coverage Δ
src/stackdriver-monitoring.ts 83.33% <0%> (+3.12%) ⬆️

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 27de0b1...e179ff9. 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

// [END setup_exporter ==============================================]

setTimeout(() => {
console.log('Timeout executed.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you document what this timeout is for? I remember in another example the timeout kept the Node process alive while the metric was being collected.

};
const gauge = metricRegistry.addDerivedInt64Gauge('active_handles_total', metricOptions);

gauge.createTimeSeries(labelValues, process._getActiveHandles());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you pass process._getActiveHandles() vs. passing the function itself process._getActiveHandles? I would have expected it to take a function that it calls to calculate the gauge value rather than the output of a function for a single value.

Copy link
Member Author

Choose a reason for hiding this comment

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

The createTimeSeries method accepts the object with AccessorInterface type.

AccessorInterface = LengthAttributeInterface | LengthMethodInterface | 
                    SizeAttributeInterface | SizeMethodInterface | ToValueInterface;

I have changed the example to show derived gauge example with queue depth. Do you think we should add support for function based extractor?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, makes sense. Yes, I think supporting a function as well would be nice so that people can just pass in an anonymous arrow function easily.

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 sure, I will open an issue to include function based extractor in derived gauge API. Thanks

examples/gauge/gauge-quickstart.js Outdated Show resolved Hide resolved
@mayurkale22 mayurkale22 merged commit 227fb39 into census-instrumentation:master Apr 2, 2019
@mayurkale22 mayurkale22 deleted the gauge_example branch April 2, 2019 23:58
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.

4 participants