From 666a6d4797fec655b3c032718a03311a45325e3e Mon Sep 17 00:00:00 2001 From: Seth Webster Date: Fri, 22 Sep 2023 16:06:35 -0400 Subject: [PATCH] feat(monitoring-exporter): add disableCreateMetricDescriptors option to skip checking for metric descriptor existence (#623) * Issue #621 This change creates an option to skip checking if metric descriptors exist before sending metrics. Skipping the metric descriptor check can prevent the clients from getting rate limited by Google when a large number are all started at the same time. * Updated option name based on PR feedback. * Updated comment based on PR feedback. --- .../src/external-types.ts | 8 +++ .../src/monitoring.ts | 7 ++- .../test/monitoring.test.ts | 49 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-cloud-monitoring-exporter/src/external-types.ts b/packages/opentelemetry-cloud-monitoring-exporter/src/external-types.ts index 65b04b22..b7acfe02 100644 --- a/packages/opentelemetry-cloud-monitoring-exporter/src/external-types.ts +++ b/packages/opentelemetry-cloud-monitoring-exporter/src/external-types.ts @@ -47,6 +47,14 @@ export interface ExporterOptions { * monitoring.googleapis.com:443. */ apiEndpoint?: string; + /** + * Disable calling CreateMetricDescriptor before sending time series to Cloud Monitoring. + * Metric descriptors will be + * {@link https://cloud.google.com/monitoring/custom-metrics/creating-metrics#auto-creation | auto-created} + * if needed, but may be missing descriptions. This can prevent hitting a rate limit in + * Cloud Monitoring when a large number of clients are all started up at the same time. + */ + disableCreateMetricDescriptors?: boolean; } export interface Credentials { diff --git a/packages/opentelemetry-cloud-monitoring-exporter/src/monitoring.ts b/packages/opentelemetry-cloud-monitoring-exporter/src/monitoring.ts index e3f12ad7..995e6fc8 100644 --- a/packages/opentelemetry-cloud-monitoring-exporter/src/monitoring.ts +++ b/packages/opentelemetry-cloud-monitoring-exporter/src/monitoring.ts @@ -60,6 +60,7 @@ export class MetricExporter implements PushMetricExporter { private _projectId: string | void | Promise; private readonly _metricPrefix: string; private readonly _auth: GoogleAuth; + private readonly _disableCreateMetricDescriptors: boolean; static readonly DEFAULT_METRIC_PREFIX: string = 'workload.googleapis.com'; @@ -73,6 +74,8 @@ export class MetricExporter implements PushMetricExporter { constructor(options: ExporterOptions = {}) { this._metricPrefix = options.prefix ?? MetricExporter.DEFAULT_METRIC_PREFIX; + this._disableCreateMetricDescriptors = + !!options.disableCreateMetricDescriptors; this._auth = new GoogleAuth({ credentials: options.credentials, @@ -145,7 +148,9 @@ export class MetricExporter implements PushMetricExporter { const timeSeries: TimeSeries[] = []; for (const scopeMetric of resourceMetrics.scopeMetrics) { for (const metric of scopeMetric.metrics) { - const isRegistered = await this._registerMetricDescriptor(metric); + const isRegistered = + this._disableCreateMetricDescriptors || + (await this._registerMetricDescriptor(metric)); if (isRegistered) { timeSeries.push( ...createTimeSeries(metric, resource, this._metricPrefix) diff --git a/packages/opentelemetry-cloud-monitoring-exporter/test/monitoring.test.ts b/packages/opentelemetry-cloud-monitoring-exporter/test/monitoring.test.ts index 257b177a..4554b0b4 100644 --- a/packages/opentelemetry-cloud-monitoring-exporter/test/monitoring.test.ts +++ b/packages/opentelemetry-cloud-monitoring-exporter/test/monitoring.test.ts @@ -279,12 +279,14 @@ describe('MetricExporter', () => { }); }); + assert.strictEqual(metricDescriptorsGet.callCount, 1); assert.strictEqual(metricDescriptorCreate.callCount, 1); assert.strictEqual(timeSeries.callCount, 1); assert.deepStrictEqual(result.code, ExportResultCode.SUCCESS); // Second time around, MetricDescriptorCreate.create() should be skipped metricDescriptorCreate.resetHistory(); + metricDescriptorsGet.resetHistory(); timeSeries.resetHistory(); result = await new Promise(resolve => { exporter.export(resourceMetrics, result => { @@ -292,6 +294,53 @@ describe('MetricExporter', () => { }); }); + assert.strictEqual(metricDescriptorsGet.callCount, 0); + assert.strictEqual(metricDescriptorCreate.callCount, 0); + assert.strictEqual(timeSeries.callCount, 1); + assert.deepStrictEqual(result.code, ExportResultCode.SUCCESS); + }); + + it('should skip fetching the MetricDescriptors when disableCreateMetricDescriptors is set', async () => { + const exporterSkipDescriptorCreate = new MetricExporter({ + disableCreateMetricDescriptors: true, + }); + sinon.replace( + exporterSkipDescriptorCreate['_monitoring'].projects + .metricDescriptors, + 'create', + metricDescriptorCreate as sinon.SinonSpy + ); + sinon.replace( + exporterSkipDescriptorCreate['_monitoring'].projects + .metricDescriptors, + 'get', + metricDescriptorsGet as sinon.SinonSpy + ); + sinon.replace( + exporterSkipDescriptorCreate['_monitoring'].projects.timeSeries, + 'create', + timeSeries as any + ); + sinon.replace( + exporterSkipDescriptorCreate['_auth'], + 'getClient', + () => { + if (getClientShouldFail) { + throw new Error('fail'); + } + return {} as any; + } + ); + + resourceMetrics = await generateMetricsData(); + + const result = await new Promise(resolve => { + exporterSkipDescriptorCreate.export(resourceMetrics, result => { + resolve(result); + }); + }); + + assert.strictEqual(metricDescriptorsGet.callCount, 0); assert.strictEqual(metricDescriptorCreate.callCount, 0); assert.strictEqual(timeSeries.callCount, 1); assert.deepStrictEqual(result.code, ExportResultCode.SUCCESS);