`cloud-monitoring-exporter` tries to create time series metric descriptors on every application bootstrap
leonardopliski opened this issue · 2 comments
What version of OpenTelemetry are you using?
1.9.0.
What version of Node are you using?
v16.15.1.
What did you do?
Created multiple metric descriptors on the application bootstrap. When using cloud-monitoring-exporter, I noticed that it tries to create the metric descriptors always, if you're running multiple instances at scale, GCP might rate limit you due to the metric descriptors creation quotas limit (see the screenshot):
What did you expect to see?
I expected that the package would create the metric descriptors just once, if it doesn't exist, and after that only send the time series.
What did you see instead?
It tries to create the metric descriptors on every application bootstrap, causing tons of requests to GCP;
I'm happy to help to provide a bugfix to this one, with a GET request to check if the metric descriptor exists once it's registered, within packages/opentelemetry-cloud-monitoring-exporter/src/monitoring.ts, the best solution would probably be to query the descriptors in batches, but that would require a bigger refactor and the google APIs probably doesn't support get requests with multiple descriptor ids, as far as I know.
We did that in the go SDK exporter: https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/blob/main/exporter/metric/metric.go#L201.
We would be willing to accept a PR which adds the same behavior to the JS exporter.
As a note, there is a default quota of 6k requests / minute / project for both GetMetricDescriptor and CreateMetricDescriptor. That would mean that if you implemented the fix to do a Get first, you would still run into quota issues. An alternative to consider would be to allow disabling CreateMetricDescriptor entirely. The metric descriptor is only needed to populate the unit and description, so you could temporarily enable CMD, and then keep it disabled as long as you aren't changing those.
Great @dashpole, thanks for the comment, I created the #533 PR to add the same behavior to the package. If possible, please take a look and re-test to check everything is working as expected.
I like your strategy for disabling CreateMetricDescriptor entirely, maybe we could add an option to disable it in the exporter constructor? I'd be happy to create another PR to add it. What do you think?