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

Fix: add batch limit for stackdriver exporter #644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mayurkale22
Copy link
Member

Fixes #634

These changes have all been tested locally against a real stackdriver exporter and been confirmed to be working.

@mayurkale22
Copy link
Member Author

/cc @mluggy Please review.

@mayurkale22
Copy link
Member Author

@draffensperger Please review when you have a time.

}

it('should return a partition lists', () => {
const list = partitionList(timeSeriesList, 200);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to test the exporter directly rather than the helper function? That would enable check that the helper function is used correctly and would implicitly check the helper (which could then even be made non-exported).

I'm open to leaving as-is for now if you prefer though.

@ofpiyush
Copy link

Can we merge this and get a release?

@philicious
Copy link
Contributor

I'd like to mention another, related problem with exporting Metrics to Stackdriver:

Rate at which data can be written to a single time series: one point each 10 seconds

(used to be even only 1 per minute in December 2019 when I run into this problem)

its still ridiculously low for per-request metrics and also causes millions of API errors, eating up memory and CPU in your service if you dont implement some throttling yourself

this caused us to not even start using Stackdriver Monitoring for custom metrics but ended up using Prometheus again.

Copy link

@JanSunavec JanSunavec left a comment

Choose a reason for hiding this comment

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

Tested in kubernetes production and it's working.

@6utt3rfly
Copy link

Is there a reason this isn't released yet? I'm running into the same issue as #634

@d-goldin
Copy link

Would be quite nice if this got some attention again. Kinda makes it barely usable with stackdriver.

@mamidon
Copy link

mamidon commented Mar 7, 2022

We're seeing this issue ourselves, is there any workaround available?

@philicious
Copy link
Contributor

I guess there won't be much love for opencensus anymore as it's deprecated in favor of opentelemetry

@mamidon
Copy link

mamidon commented Mar 7, 2022

Bah, that would probably explain it. Of course the GCP docs I read must've been out of date. Thanks for the FYI.

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.

Support specifying batch limit
9 participants