feat: add stackdriver stats exporter #94
Conversation
eduardoemery
force-pushed
the
stats_stackdriver
branch
3 times, most recently
from
95a80f8
to
2625906
Compare
eduardoemery
force-pushed
the
stats_stackdriver
branch
from
2625906
to
d1e2414
Compare
|
|
||
| return new Promise((resolve, reject) => { | ||
| monitoring.projects.metricDescriptors.create(request, (err: Error) => { | ||
| this.logger.debug(request.resource); |
There was a problem hiding this comment.
Can you prefix debug calls with info about the data being printed, or remove them entirely?
There was a problem hiding this comment.
Sure, please let me know if the message doesn't make ir clearer.
| } | ||
|
|
||
| /** | ||
| * Formats an OpenCensus' Distribution to Stackdriver's format. |
| * Gets the bucket boundaries in an monotonicaly increasing order. | ||
| * @param buckets The bucket list to get the boundaries from | ||
| */ | ||
| private getBucketBoundaries(buckets: Bucket[]): number[] { |
There was a problem hiding this comment.
Is it possible for this array not to be in order in the first place?
There was a problem hiding this comment.
Yes, since buckets is public, anyone can mutate it (like I was doing myself), not guaranteeing that Stackdriver will receive the bucket boundaries in order.
|
|
||
| private getBucketCounts(buckets: Bucket[]): number[] { | ||
| return buckets | ||
| .sort((bucket1, bucket2) => { |
There was a problem hiding this comment.
(1) Same comment as above, wouldn't it be better to make it a pre-condition that buckets is sorted? If this can't be uphold, you should at least clone buckets first since .sort mutates it.
There was a problem hiding this comment.
I'm not sure that making buckets sorted as precondition is a great idea. Since buckets is public, anyone (just like I was doing) can mutate it, making it harder to guarantee it will always be sorted. We should trust the user won't change the order.
Thanks for remembering about the cloning!
| } | ||
| return MetricKind.GAUGE; | ||
| } | ||
| } |
eduardoemery
force-pushed
the
stats_stackdriver
branch
from
f8ffaaa
to
8e0152e
Compare
eduardoemery
force-pushed
the
stats_stackdriver
branch
from
8e0152e
to
7d50c95
Compare
* feat: add stackdriver stats exporter * refactor(fix): changes to address review comments
* feat: add stackdriver stats exporter * refactor(fix): changes to address review comments
* feat: add stackdriver stats exporter * refactor(fix): changes to address review comments
No description provided.