fix(metrics): updates unit option behavior to be spec-compliant (#2983)

Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
open-telemetry · May 27, 2022 · 7196b7f · 7196b7f
1 parent 7149f6c
commit 7196b7f
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 16 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -27,6 +27,7 @@ All notable changes to experimental packages in this project will be documented
 * fix(otlp-exporter-base): include esm and esnext in package files #2952 @dyladan
 * fix(otlp-http-exporter): update endpoint to match spec #2895 @svetlanabrennan
 * fix(otlp-transformer): include esm and esnext in package files and update README #2992 @pichlermarc
+* fix(metrics): specification compliant default metric unit #2983 @andyfleming
 
 ### :books: (Refine Doc)
 

diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/metricsHelper.ts
@@ -131,7 +131,7 @@ export function ensureExportedCounterIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'int-counter',
     description: 'sample counter description',
-    unit: '1',
+    unit: '',
     data: 'sum',
     sum: {
       dataPoints: [
@@ -159,7 +159,7 @@ export function ensureExportedObservableGaugeIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'double-observable-gauge',
     description: 'sample observable gauge description',
-    unit: '1',
+    unit: '',
     data: 'gauge',
     gauge: {
       dataPoints: [
@@ -187,7 +187,7 @@ export function ensureExportedHistogramIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'int-histogram',
     description: 'sample histogram description',
-    unit: '1',
+    unit: '',
     data: 'histogram',
     histogram: {
       dataPoints: [

diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/metricsHelper.ts
@@ -237,7 +237,7 @@ export function ensureCounterIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'int-counter',
     description: 'sample counter description',
-    unit: '1',
+    unit: '',
     sum: {
       dataPoints: [
         {
@@ -261,7 +261,7 @@ export function ensureDoubleCounterIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'double-counter',
     description: 'sample counter description',
-    unit: '1',
+    unit: '',
     doubleSum: {
       dataPoints: [
         {
@@ -287,7 +287,7 @@ export function ensureObservableGaugeIsCorrect(
   assert.deepStrictEqual(metric, {
     name,
     description: 'sample observable gauge description',
-    unit: '1',
+    unit: '',
     gauge: {
       dataPoints: [
         {
@@ -311,7 +311,7 @@ export function ensureObservableCounterIsCorrect(
   assert.deepStrictEqual(metric, {
     name,
     description: 'sample observable counter description',
-    unit: '1',
+    unit: '',
     doubleSum: {
       isMonotonic: true,
       dataPoints: [
@@ -337,7 +337,7 @@ export function ensureObservableUpDownCounterIsCorrect(
   assert.deepStrictEqual(metric, {
     name,
     description: 'sample observable up down counter description',
-    unit: '1',
+    unit: '',
     doubleSum: {
       isMonotonic: false,
       dataPoints: [
@@ -363,7 +363,7 @@ export function ensureHistogramIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'int-histogram',
     description: 'sample histogram description',
-    unit: '1',
+    unit: '',
     histogram: {
       dataPoints: [
         {

diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/test/metricsHelper.ts
@@ -135,7 +135,7 @@ export function ensureExportedCounterIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'int-counter',
     description: 'sample counter description',
-    unit: '1',
+    unit: '',
     sum: {
       dataPoints: [
         {
@@ -158,7 +158,7 @@ export function ensureExportedObservableGaugeIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'double-observable-gauge',
     description: 'sample observable gauge description',
-    unit: '1',
+    unit: '',
     gauge: {
       dataPoints: [
         {
@@ -181,7 +181,7 @@ export function ensureExportedHistogramIsCorrect(
   assert.deepStrictEqual(metric, {
     name: 'int-histogram',
     description: 'sample histogram description',
-    unit: '1',
+    unit: '',
     histogram: {
       dataPoints: [
         {

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/InstrumentDescriptor.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/InstrumentDescriptor.ts
@@ -42,7 +42,7 @@ export function createInstrumentDescriptor(name: string, type: InstrumentType, o
     name,
     type,
     description: options?.description ?? '',
-    unit: options?.unit ?? '1',
+    unit: options?.unit ?? '',
     valueType: options?.valueType ?? ValueType.DOUBLE,
   };
 }

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/InstrumentDescriptor.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/InstrumentDescriptor.test.ts
@@ -0,0 +1,31 @@
+/*
+ * Copyright The 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 {createInstrumentDescriptor, InstrumentType} from '../src/InstrumentDescriptor';
+
+describe('InstrumentDescriptor', () => {
+  describe('createInstrumentDescriptor', () => {
+    for (const val of [null, undefined]) {
+      it(`should interpret an empty unit value as a blank string (${val})`, () => {
+        const result = createInstrumentDescriptor('example', InstrumentType.COUNTER, {
+          unit: val as any,
+        });
+        assert.strictEqual(result.unit, '');
+      });
+    }
+  });
+});
diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/Instruments.test.ts
@@ -64,7 +64,7 @@ describe('Instruments', () => {
         descriptor: {
           name: 'test',
           description: '',
-          unit: '1',
+          unit: '',
           type: InstrumentType.COUNTER,
           valueType: ValueType.INT,
         },
@@ -180,7 +180,7 @@ describe('Instruments', () => {
         descriptor: {
           name: 'test',
           description: '',
-          unit: '1',
+          unit: '',
           type: InstrumentType.UP_DOWN_COUNTER,
           valueType: ValueType.INT,
         },
@@ -265,7 +265,7 @@ describe('Instruments', () => {
         descriptor: {
           name: 'test',
           description: '',
-          unit: '1',
+          unit: '',
           type: InstrumentType.HISTOGRAM,
           valueType: ValueType.INT,
         },