fix: do not clear other labelsets when updating metrics (#941)

* fix: do not clear other labelsets when updating metrics

* chore: add test for batcher

* chore: test for one counter with multiple labels

* chore: lint

* chore: fix year in copyright

packages/opentelemetry-exporter-prometheus/src/prometheus.ts

@@ -123,29 +123,26 @@ export class PrometheusExporter implements MetricExporter {
     const metric = this._registerMetric(record);
     if (!metric) return;
 
-    const labelKeys = record.descriptor.labelKeys;
+    const labelValues = this._getLabelValues(
+      record.descriptor.labelKeys,
+      record.labels
+    );
     const point = record.aggregator.toPoint();
 
     if (metric instanceof Counter) {
       // Prometheus counter saves internal state and increments by given value.
       // MetricRecord value is the current state, not the delta to be incremented by.
       // Currently, _registerMetric creates a new counter every time the value changes,
       // so the increment here behaves as a set value (increment from 0)
-      metric.inc(
-        this._getLabelValues(labelKeys, record.labels),
-        point.value as Sum
-      );
+      metric.inc(labelValues, point.value as Sum);
     }
 
     if (metric instanceof Gauge) {
       if (record.aggregator instanceof CounterSumAggregator) {
-        metric.set(
-          this._getLabelValues(labelKeys, record.labels),
-          point.value as Sum
-        );
+        metric.set(labelValues, point.value as Sum);
       } else if (record.aggregator instanceof ObserverAggregator) {
         metric.set(
-          this._getLabelValues(labelKeys, record.labels),
+          labelValues,
           point.value as LastValue,
           hrTimeToMilliseconds(point.timestamp)
         );
@@ -179,8 +176,12 @@ export class PrometheusExporter implements MetricExporter {
      * https://prometheus.io/docs/instrumenting/exposition_formats/
      */
     if (metric instanceof Counter) {
-      this._registry.removeSingleMetric(metricName);
-    } else if (metric) return metric;
+      metric.remove(
+        ...record.descriptor.labelKeys.map(k => record.labels[k].toString())
+      );
+    }
+
+    if (metric) return metric;
 
     return this._newMetric(record, metricName);
   }

packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts

@@ -266,13 +266,14 @@ describe('PrometheusExporter', () => {
       });
     });
 
-    it('should export multiple aggregations', done => {
+    it('should export multiple labelsets', done => {
       const counter = meter.createCounter('counter', {
         description: 'a test description',
         labelKeys: ['counterKey1'],
       }) as CounterMetric;
 
       counter.bind({ counterKey1: 'labelValue1' }).add(10);
+      counter.bind({ counterKey1: 'labelValue2' }).add(20);
       meter.collect();
       exporter.export(meter.getBatcher().checkPointSet(), () => {
         http
@@ -285,6 +286,7 @@ describe('PrometheusExporter', () => {
                 '# HELP counter a test description',
                 '# TYPE counter counter',
                 'counter{counterKey1="labelValue1"} 10',
+                'counter{counterKey1="labelValue2"} 20',
                 '',
               ]);
 

packages/opentelemetry-metrics/src/export/Batcher.ts

@@ -59,9 +59,9 @@ export class UngroupedBatcher extends Batcher {
   }
 
   process(record: MetricRecord): void {
-    this._batchMap.set(
-      record.descriptor.name + record.labels.identifier,
-      record
-    );
+    const labels = record.descriptor.labelKeys
+      .map(k => `${k}=${record.labels[k]}`)
+      .join(',');
+    this._batchMap.set(record.descriptor.name + labels, record);
   }
 }

packages/opentelemetry-metrics/test/Batcher.test.ts

@@ -0,0 +1,61 @@
+/*!
+ * Copyright 2020, OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as assert from 'assert';
+import * as types from '@opentelemetry/api';
+import { NoopLogger } from '@opentelemetry/core';
+import { Meter, MeterProvider } from '../src';
+
+describe('Batcher', () => {
+  describe('Ungrouped', () => {
+    let meter: Meter;
+    let fooCounter: types.BoundCounter;
+    let barCounter: types.BoundCounter;
+    let counter: types.Metric<types.BoundCounter>;
+    beforeEach(() => {
+      meter = new MeterProvider({
+        logger: new NoopLogger(),
+        interval: 10000,
+      }).getMeter('test-meter');
+      counter = meter.createCounter('ungrouped-batcher-test', {
+        labelKeys: ['key'],
+      });
+      fooCounter = counter.bind({ key: 'foo' });
+      barCounter = counter.bind({ key: 'bar' });
+    });
+
+    it('should process a batch', () => {
+      fooCounter.add(1);
+      barCounter.add(1);
+      barCounter.add(2);
+      meter.collect();
+      const checkPointSet = meter.getBatcher().checkPointSet();
+      assert.strictEqual(checkPointSet.length, 2);
+      for (const record of checkPointSet) {
+        switch (record.labels.key) {
+          case 'foo':
+            assert.strictEqual(record.aggregator.toPoint().value, 1);
+            break;
+          case 'bar':
+            assert.strictEqual(record.aggregator.toPoint().value, 3);
+            break;
+          default:
+            throw new Error('Unknown labelset');
+        }
+      }
+    });
+  });
+});