chore: add batch unit test and update gauge double handling

Author: jeremyprime

src/metrics/spanner-metrics-exporter.ts

@@ -21,7 +21,7 @@ import {status} from '@grpc/grpc-js';
 
 // Stackdriver Monitoring v3 only accepts up to 200 TimeSeries per
 // CreateTimeSeries call.
-const MAX_BATCH_EXPORT_SIZE = 200;
+export const MAX_BATCH_EXPORT_SIZE = 200;
 
 /**
  * Format and sends metrics information to Google Cloud Monitoring.

src/metrics/transform.ts

@@ -71,8 +71,10 @@ function _transformValueType(metric: MetricData): ValueType {
  dataPointType === DataPointType.EXPONENTIAL_HISTOGRAM
  ) {
  return ValueType.DISTRIBUTION;
- } else if (name.includes('count')) {
+ } else if (dataPointType === DataPointType.SUM) {
  return ValueType.INT64;
+ } else if (dataPointType === DataPointType.GAUGE) {
+ return ValueType.DOUBLE;
  }
  console.warn('Encountered unexpected metric %s', name);
  return ValueType.VALUE_TYPE_UNSPECIFIED;
@@ -211,9 +213,12 @@ function _normalizeLabelKey(key: string): string {
 /** Transforms a OpenTelemetry Point's value to a GCM Point value. */
 function _transformNumberValue(valueType: ValueType, value: number) {
  if (valueType === ValueType.INT64) {
- return {int64Value: value.toString()};
+ return {int64Value: Math.round(value).toString()};
  } else if (valueType === ValueType.DOUBLE) {
- return {doubleValue: value};
+ const doubleString = Number.isInteger(value)
+ ? `${value}.0`
+ : value.toString();
+ return {doubleValue: doubleString};
  }
  throw Error(`unsupported value type: ${valueType}`);
 }
@@ -275,6 +280,7 @@ function _transformExponentialHistogramValue(value: ExponentialHistogram) {
  };
 }
 
+/** Transforms an OpenTelemetry time value to a GCM time value. */
 function _formatHrTimeToGcmTime(hrTime) {
  return {
  seconds: hrTime[0],

test/metrics/spanner-metrics-exporter.ts

@@ -16,7 +16,10 @@ import * as assert from 'assert';
 import * as sinon from 'sinon';
 import {MeterProvider, MetricReader} from '@opentelemetry/sdk-metrics';
 import {GoogleAuth} from 'google-auth-library';
-import {CloudMonitoringMetricsExporter} from '../../src/metrics/spanner-metrics-exporter';
+import {
+ CloudMonitoringMetricsExporter,
+ MAX_BATCH_EXPORT_SIZE,
+} from '../../src/metrics/spanner-metrics-exporter';
 import {
  SPANNER_METER_NAME,
  METRIC_NAME_ATTEMPT_COUNT,
@@ -86,7 +89,7 @@ describe('Export', () => {
  let gfe_connectivity_error_count: Counter;
  let attempt_latency: Histogram;
  let operation_latency: Histogram;
- let gfe_latencies: Histogram;
+ let gfe_latency: Histogram;
  let metricAttributes: {[key: string]: string};
  let exporter: CloudMonitoringMetricsExporter;
 
@@ -139,7 +142,7 @@ describe('Export', () => {
  unit: 'ms',
  });
 
- gfe_latencies = meter.createHistogram(METRIC_NAME_GFE_LATENCIES, {
+ gfe_latency = meter.createHistogram(METRIC_NAME_GFE_LATENCIES, {
  description: 'Test GFE latencies in ms',
  unit: 'ms',
  });
@@ -151,7 +154,7 @@ describe('Export', () => {
  gfe_connectivity_error_count.add(12, metricAttributes);
  attempt_latency.record(30, metricAttributes);
  operation_latency.record(45, metricAttributes);
- gfe_latencies.record(22, metricAttributes);
+ gfe_latency.record(22, metricAttributes);
 
  const {errors, resourceMetrics} = await reader.collect();
  if (errors.length !== 0) {
@@ -224,4 +227,44 @@ describe('Export', () => {
 
  assert(sendTimeSeriesStub.calledOnce);
  });
+
+ it('should batch exports into multiple calls', async () => {
+ // Create metircs larger than the batch size
+ const numberOfDistinctMetrics = MAX_BATCH_EXPORT_SIZE * 2 + 1;
+ for (let i = 0; i < numberOfDistinctMetrics; i++) {
+ attempt_counter.add(1, {...metricAttributes, testId: `batch-test-${i}`});
+ }
+
+ const {resourceMetrics} = await reader.collect();
+
+ const sendTimeSeriesStub = sinon
+ .stub(exporter as any, '_sendTimeSeries')
+ .resolves();
+ const resultCallbackSpy = sinon.spy();
+
+ exporter.export(resourceMetrics, resultCallbackSpy);
+
+ await new Promise(resolve => setImmediate(resolve));
+
+ // Confirm number of metrics for each batch
+ const expectedNumberOfCalls = Math.ceil(
+ numberOfDistinctMetrics / MAX_BATCH_EXPORT_SIZE,
+ );
+ assert.strictEqual(sendTimeSeriesStub.callCount, expectedNumberOfCalls);
+ assert.strictEqual(
+ sendTimeSeriesStub.getCall(0).args[0].length,
+ MAX_BATCH_EXPORT_SIZE,
+ );
+ assert.strictEqual(
+ sendTimeSeriesStub.getCall(1).args[0].length,
+ MAX_BATCH_EXPORT_SIZE,
+ );
+ assert.strictEqual(
+ sendTimeSeriesStub.getCall(2).args[0].length,
+ numberOfDistinctMetrics % MAX_BATCH_EXPORT_SIZE,
+ );
+
+ const callbackResult = resultCallbackSpy.getCall(0).args[0];
+ assert.strictEqual(callbackResult.code, ExportResultCode.SUCCESS);
+ });
 });

test/metrics/transform.ts

@@ -61,6 +61,7 @@ describe('transform', () => {
  let metricExponentialHistogram: ExponentialHistogramMetricData;
  let metricUnknown;
  let sumDataPoint: DataPoint<number>;
+ let gaugeDataPoint: DataPoint<number>;
  let histogramDataPoint: DataPoint<Histogram>;
  let exponentialHistogramDataPoint: DataPoint<ExponentialHistogram>;
 
@@ -93,14 +94,14 @@ describe('transform', () => {
  aggregationTemporality: AggregationTemporality.DELTA,
  isMonotonic: true,
  dataPointType: DataPointType.SUM,
- descriptor: {valueType: OTValueType.DOUBLE, name: 'some_count'} as any,
+ descriptor: {valueType: OTValueType.INT, name: 'some_count'} as any,
  };
 
  metricGauge = {
  dataPoints: [],
  aggregationTemporality: '' as any,
  dataPointType: DataPointType.GAUGE,
- descriptor: {valueType: OTValueType.INT, name: 'a_count'} as any,
+ descriptor: {valueType: OTValueType.DOUBLE, name: 'a_count'} as any,
  };
 
  metricHistogram = {
@@ -131,6 +132,13 @@ describe('transform', () => {
  endTime: process.hrtime(),
  };
 
+ gaugeDataPoint = {
+ attributes,
+ value: 0.0,
+ startTime: process.hrtime(),
+ endTime: process.hrtime(),
+ };
+
  histogramDataPoint = {
  attributes,
  startTime: process.hrtime(),
@@ -244,7 +252,7 @@ describe('transform', () => {
  it('should transform otel value types to GCM value types', () => {
  assert.strictEqual(_transformValueType(metricSum), ValueType.INT64);
 
- assert.strictEqual(_transformValueType(metricGauge), ValueType.INT64);
+ assert.strictEqual(_transformValueType(metricGauge), ValueType.DOUBLE);
 
  assert.strictEqual(
  _transformValueType(metricHistogram),
@@ -279,15 +287,15 @@ describe('transform', () => {
 
  const gaugeExpectation = {
  value: {
- int64Value: '0',
+ doubleValue: '0.0',
  },
  interval: {
- endTime: _formatHrTimeToGcmTime(sumDataPoint.endTime),
+ endTime: _formatHrTimeToGcmTime(gaugeDataPoint.endTime),
  },
  };
 
  assert.deepStrictEqual(
- _transformPoint(metricGauge, sumDataPoint),
+ _transformPoint(metricGauge, gaugeDataPoint),
  gaugeExpectation,
  );