feat(sdk-metrics)!: drop deprecated type field on MetricDescriptor (

#5291)
open-telemetry · Jan 8, 2025 · 3b56060 · 3b56060
1 parent 04e74d7
commit 3b56060
Show file tree

Hide file tree

Showing 21 changed files with 61 additions and 82 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,7 +11,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
 
 ### :boom: Breaking Change
 
-* feat(sdk-metrics)!: drop deprecated `InstrumentDescriptor` type; use `MetricDescriptor` instead [#5277](https://github.com/open-telemetry/opentelemetry-js/pull/5266)
+* feat(sdk-metrics)!: drop deprecated `type` field on `MetricDescriptor` [#5291](https://github.com/open-telemetry/opentelemetry-js/pull/5291) @chancancode
+* feat(sdk-metrics)!: drop deprecated `InstrumentDescriptor` type; use `MetricDescriptor` instead [#5277](https://github.com/open-telemetry/opentelemetry-js/pull/5266) @chancancode
 * feat(sdk-metrics)!: bump minimum version of `@opentelemetry/api` peer dependency to 1.9.0 [#5254](https://github.com/open-telemetry/opentelemetry-js/pull/5254) @chancancode
 * chore(shim-opentracing): replace deprecated SpanAttributes [#4430](https://github.com/open-telemetry/opentelemetry-js/pull/4430) @JamieDanielson
 * chore(otel-core): replace deprecated SpanAttributes [#4408](https://github.com/open-telemetry/opentelemetry-js/pull/4408) @JamieDanielson

diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -7,6 +7,10 @@ All notable changes to experimental packages in this project will be documented
 
 ### :boom: Breaking Change
 
+* feat(exporter-prometheus)!: stop the using `type` field to enforce naming conventions [#5291](https://github.com/open-telemetry/opentelemetry-js/pull/5291) @chancancode
+  * Any non-monotonic sums will now be treated as counters and will therefore include the `_total` suffix.
+* feat(shim-opencenus)!: stop mapping removed Instrument `type` to OpenTelemetry metrics [#5291](https://github.com/open-telemetry/opentelemetry-js/pull/5291) @chancancode
+  * The internal OpenTelemetry data model dropped the concept of instrument type on exported metrics, therefore mapping it is not necessary anymore.  
 * feat(instrumentation-fetch)!: passthrough original response to `applyCustomAttributes` hook [#5281](https://github.com/open-telemetry/opentelemetry-js/pull/5281) @chancancode
   * Previously, the fetch instrumentation code unconditionally clones every `fetch()` response in order to preserve the ability for the `applyCustomAttributes` hook to consume the response body. This is fundamentally unsound, as it forces the browser to buffer and retain the response body until it is fully received and read, which crates unnecessary memory pressure on large or long-running response streams. In extreme cases, this is effectively a memory leak and can cause the browser tab to crash. If your use case for `applyCustomAttributes` requires access to the response body, please chime in on [#5293](https://github.com/open-telemetry/opentelemetry-js/issues/5293).
 

diff --git a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
@@ -17,7 +17,6 @@
 import { diag, Attributes, AttributeValue } from '@opentelemetry/api';
 import {
   ResourceMetrics,
-  InstrumentType,
   DataPointType,
   ScopeMetrics,
   MetricData,
@@ -90,10 +89,14 @@ function sanitizePrometheusMetricName(name: string): string {
  */
 function enforcePrometheusNamingConvention(
   name: string,
-  type: InstrumentType
+  data: MetricData
 ): string {
   // Prometheus requires that metrics of the Counter kind have "_total" suffix
-  if (!name.endsWith('_total') && type === InstrumentType.COUNTER) {
+  if (
+    !name.endsWith('_total') &&
+    data.dataPointType === DataPointType.SUM &&
+    data.isMonotonic
+  ) {
     name = name + '_total';
   }
 
@@ -210,7 +213,7 @@ export class PrometheusSerializer {
     }
     const dataPointType = metricData.dataPointType;
 
-    name = enforcePrometheusNamingConvention(name, metricData.descriptor.type);
+    name = enforcePrometheusNamingConvention(name, metricData);
 
     const help = `# HELP ${name} ${escapeString(
       metricData.descriptor.description || 'description missing'
@@ -225,25 +228,13 @@ export class PrometheusSerializer {
       case DataPointType.SUM:
       case DataPointType.GAUGE: {
         results = metricData.dataPoints
-          .map(it =>
-            this._serializeSingularDataPoint(
-              name,
-              metricData.descriptor.type,
-              it
-            )
-          )
+          .map(it => this._serializeSingularDataPoint(name, metricData, it))
           .join('');
         break;
       }
       case DataPointType.HISTOGRAM: {
         results = metricData.dataPoints
-          .map(it =>
-            this._serializeHistogramDataPoint(
-              name,
-              metricData.descriptor.type,
-              it
-            )
-          )
+          .map(it => this._serializeHistogramDataPoint(name, metricData, it))
           .join('');
         break;
       }
@@ -259,12 +250,12 @@ export class PrometheusSerializer {
 
   private _serializeSingularDataPoint(
     name: string,
-    type: InstrumentType,
+    data: MetricData,
     dataPoint: DataPoint<number>
   ): string {
     let results = '';
 
-    name = enforcePrometheusNamingConvention(name, type);
+    name = enforcePrometheusNamingConvention(name, data);
     const { value, attributes } = dataPoint;
     const timestamp = hrTimeToMilliseconds(dataPoint.endTime);
     results += stringify(
@@ -279,12 +270,12 @@ export class PrometheusSerializer {
 
   private _serializeHistogramDataPoint(
     name: string,
-    type: InstrumentType,
+    data: MetricData,
     dataPoint: DataPoint<Histogram>
   ): string {
     let results = '';
 
-    name = enforcePrometheusNamingConvention(name, type);
+    name = enforcePrometheusNamingConvention(name, data);
     const attributes = dataPoint.attributes;
     const histogram = dataPoint.value;
     const timestamp = hrTimeToMilliseconds(dataPoint.endTime);

diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts
@@ -389,9 +389,9 @@ describe('PrometheusExporter', () => {
 
       assert.deepStrictEqual(lines, [
         ...serializedDefaultResourceLines,
-        '# HELP metric_observable_counter a test description',
-        '# TYPE metric_observable_counter counter',
-        'metric_observable_counter{key1="attributeValue1"} 20',
+        '# HELP metric_observable_counter_total a test description',
+        '# TYPE metric_observable_counter_total counter',
+        'metric_observable_counter_total{key1="attributeValue1"} 20',
         '',
       ]);
     });

diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts
@@ -107,7 +107,7 @@ describe('PrometheusSerializer', () => {
 
         const result = serializer['_serializeSingularDataPoint'](
           metric.descriptor.name,
-          metric.descriptor.type,
+          metric,
           pointData[0]
         );
         return result;
@@ -162,7 +162,7 @@ describe('PrometheusSerializer', () => {
 
         const result = serializer['_serializeHistogramDataPoint'](
           metric.descriptor.name,
-          metric.descriptor.type,
+          metric,
           pointData[0]
         );
         return result;
@@ -511,7 +511,7 @@ describe('PrometheusSerializer', () => {
       } else {
         const result = serializer['_serializeSingularDataPoint'](
           metric.descriptor.name,
-          metric.descriptor.type,
+          metric,
           pointData[0]
         );
         return result;
@@ -598,7 +598,7 @@ describe('PrometheusSerializer', () => {
 
       const result = serializer['_serializeSingularDataPoint'](
         metric.descriptor.name,
-        metric.descriptor.type,
+        metric,
         pointData[0]
       );
       return result;

diff --git a/experimental/packages/otlp-transformer/test/metrics.test.ts b/experimental/packages/otlp-transformer/test/metrics.test.ts
@@ -18,7 +18,6 @@ import { Resource } from '@opentelemetry/resources';
 import {
   AggregationTemporality,
   DataPointType,
-  InstrumentType,
   MetricData,
   ResourceMetrics,
 } from '@opentelemetry/sdk-metrics';
@@ -110,7 +109,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.COUNTER,
         name: 'counter',
         unit: '1',
         valueType: ValueType.INT,
@@ -136,7 +134,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.UP_DOWN_COUNTER,
         name: 'up-down-counter',
         unit: '1',
         valueType: ValueType.INT,
@@ -162,7 +159,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.OBSERVABLE_COUNTER,
         name: 'observable-counter',
         unit: '1',
         valueType: ValueType.INT,
@@ -188,7 +184,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.OBSERVABLE_UP_DOWN_COUNTER,
         name: 'observable-up-down-counter',
         unit: '1',
         valueType: ValueType.INT,
@@ -211,7 +206,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.OBSERVABLE_GAUGE,
         name: 'gauge',
         unit: '1',
         valueType: ValueType.DOUBLE,
@@ -241,7 +235,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.HISTOGRAM,
         name: 'hist',
         unit: '1',
         valueType: ValueType.INT,
@@ -282,7 +275,6 @@ describe('Metrics', () => {
     return {
       descriptor: {
         description: 'this is a description',
-        type: InstrumentType.HISTOGRAM,
         name: 'xhist',
         unit: '1',
         valueType: ValueType.INT,

diff --git a/experimental/packages/shim-opencensus/src/metric-transform.ts b/experimental/packages/shim-opencensus/src/metric-transform.ts
@@ -22,14 +22,12 @@ import {
   DataPointType,
   GaugeMetricData,
   HistogramMetricData,
-  InstrumentType,
   MetricData,
   SumMetricData,
 } from '@opentelemetry/sdk-metrics';
 
 type BaseMetric = Omit<MetricData, 'dataPoints' | 'dataPointType'>;
 interface MappedType {
-  type: InstrumentType;
   valueType: ValueType;
   dataPointType:
     | DataPointType.GAUGE
@@ -51,7 +49,6 @@ export function mapOcMetric(metric: oc.Metric): MetricData | null {
       description,
       name,
       unit,
-      type: mappedType.type,
       valueType: mappedType.valueType,
     },
   };
@@ -72,33 +69,28 @@ function mapOcMetricDescriptorType(
   switch (type) {
     case oc.MetricDescriptorType.GAUGE_INT64:
       return {
-        type: InstrumentType.OBSERVABLE_GAUGE,
         valueType: ValueType.INT,
         dataPointType: DataPointType.GAUGE,
       };
     case oc.MetricDescriptorType.GAUGE_DOUBLE:
       return {
-        type: InstrumentType.OBSERVABLE_GAUGE,
         valueType: ValueType.DOUBLE,
         dataPointType: DataPointType.GAUGE,
       };
 
     case oc.MetricDescriptorType.CUMULATIVE_INT64:
       return {
-        type: InstrumentType.COUNTER,
         valueType: ValueType.INT,
         dataPointType: DataPointType.SUM,
       };
     case oc.MetricDescriptorType.CUMULATIVE_DOUBLE:
       return {
-        type: InstrumentType.COUNTER,
         valueType: ValueType.DOUBLE,
         dataPointType: DataPointType.SUM,
       };
 
     case oc.MetricDescriptorType.CUMULATIVE_DISTRIBUTION:
       return {
-        type: InstrumentType.HISTOGRAM,
         valueType: ValueType.DOUBLE,
         dataPointType: DataPointType.HISTOGRAM,
       };

diff --git a/experimental/packages/shim-opencensus/test/OpenCensusMetricProducer.test.ts b/experimental/packages/shim-opencensus/test/OpenCensusMetricProducer.test.ts
@@ -100,7 +100,6 @@ describe('OpenCensusMetricProducer', () => {
     assert.deepStrictEqual(ocMetric.descriptor, {
       description: 'Test OC description',
       name: 'measure',
-      type: 'COUNTER',
       unit: 'ms',
       valueType: ValueType.DOUBLE,
     });

diff --git a/experimental/packages/shim-opencensus/test/metric-transform.test.ts b/experimental/packages/shim-opencensus/test/metric-transform.test.ts
@@ -23,7 +23,6 @@ import {
   DataPointType,
   GaugeMetricData,
   HistogramMetricData,
-  InstrumentType,
   SumMetricData,
 } from '@opentelemetry/sdk-metrics';
 import * as assert from 'assert';
@@ -64,7 +63,6 @@ describe('metric-transform', () => {
       descriptor: {
         description: 'ocDescription',
         name: 'ocMetricName',
-        type: InstrumentType.COUNTER,
         unit: 'ocUnit',
         valueType: ValueType.INT,
       },
@@ -107,7 +105,6 @@ describe('metric-transform', () => {
       descriptor: {
         description: 'ocDescription',
         name: 'ocMetricName',
-        type: InstrumentType.COUNTER,
         unit: 'ocUnit',
         valueType: ValueType.DOUBLE,
       },
@@ -177,7 +174,6 @@ describe('metric-transform', () => {
       descriptor: {
         description: 'ocDescription',
         name: 'ocMetricName',
-        type: InstrumentType.HISTOGRAM,
         unit: 'ocUnit',
         valueType: ValueType.DOUBLE,
       },
@@ -219,7 +215,6 @@ describe('metric-transform', () => {
       descriptor: {
         description: 'ocDescription',
         name: 'ocMetricName',
-        type: InstrumentType.OBSERVABLE_GAUGE,
         unit: 'ocUnit',
         valueType: ValueType.INT,
       },
@@ -261,7 +256,6 @@ describe('metric-transform', () => {
       descriptor: {
         description: 'ocDescription',
         name: 'ocMetricName',
-        type: InstrumentType.OBSERVABLE_GAUGE,
         unit: 'ocUnit',
         valueType: ValueType.DOUBLE,
       },

diff --git a/packages/sdk-metrics/src/InstrumentDescriptor.ts b/packages/sdk-metrics/src/InstrumentDescriptor.ts
@@ -31,6 +31,12 @@ import { InstrumentType, MetricDescriptor } from './export/MetricData';
  * which may not contains internal fields like metric advice.
  */
 export interface InstrumentDescriptor extends MetricDescriptor {
+  /**
+   * For internal use; exporter should avoid depending on the type of the
+   * instrument as their resulting aggregator can be re-mapped with views.
+   */
+  readonly type: InstrumentType;
+
   /**
    * See {@link MetricAdvice}
    *

diff --git a/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts b/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts
@@ -25,11 +25,11 @@ import {
   DataPointType,
   ExponentialHistogramMetricData,
   InstrumentType,
-  MetricDescriptor,
 } from '../export/MetricData';
 import { diag, HrTime } from '@opentelemetry/api';
 import { Maybe } from '../utils';
 import { AggregationTemporality } from '../export/AggregationTemporality';
+import { InstrumentDescriptor } from '../InstrumentDescriptor';
 import { Buckets } from './exponential-histogram/Buckets';
 import { getMapping } from './exponential-histogram/mapping/getMapping';
 import { Mapping } from './exponential-histogram/mapping/types';
@@ -566,7 +566,7 @@ export class ExponentialHistogramAggregator
   }
 
   toMetricData(
-    descriptor: MetricDescriptor,
+    descriptor: InstrumentDescriptor,
     aggregationTemporality: AggregationTemporality,
     accumulationByAttributes: AccumulationRecord<ExponentialHistogramAccumulation>[],
     endTime: HrTime

diff --git a/packages/sdk-metrics/src/aggregator/Histogram.ts b/packages/sdk-metrics/src/aggregator/Histogram.ts
@@ -24,11 +24,11 @@ import {
   DataPointType,
   HistogramMetricData,
   InstrumentType,
-  MetricDescriptor,
 } from '../export/MetricData';
 import { HrTime } from '@opentelemetry/api';
 import { binarySearchUB, Maybe } from '../utils';
 import { AggregationTemporality } from '../export/AggregationTemporality';
+import { InstrumentDescriptor } from '../InstrumentDescriptor';
 
 /**
  * Internal value type for HistogramAggregation.
@@ -217,7 +217,7 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
   }
 
   toMetricData(
-    descriptor: MetricDescriptor,
+    descriptor: InstrumentDescriptor,
     aggregationTemporality: AggregationTemporality,
     accumulationByAttributes: AccumulationRecord<HistogramAccumulation>[],
     endTime: HrTime