Default unit is blank instead of 1 (open-telemetry#4431)

jack-berg · web-flow · commit 04617c35501a · 2022-05-05T10:34:26.000-05:00
diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java
@@ -19,7 +19,7 @@
 /** Helper to make implementing builders easier. */
 abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuilder<?>> {
 
-  static final String DEFAULT_UNIT = "1";
+  static final String DEFAULT_UNIT = "";
 
   private final MeterProviderSharedState meterProviderSharedState;
   private String description;
@@ -45,7 +45,8 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
 
   public BuilderT setUnit(String unit) {
     if (!ValidationUtil.checkValidInstrumentUnit(
-        unit, " Using " + DEFAULT_UNIT + " for instrument " + this.instrumentName + " instead.")) {
+        unit,
+        " Using \"" + DEFAULT_UNIT + "\" for instrument " + this.instrumentName + " instead.")) {
       this.unit = DEFAULT_UNIT;
     } else {
       this.unit = unit;
diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/IdentityTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/IdentityTest.java
@@ -290,15 +290,14 @@ void sameMeterConflictingInstrumentUnitNoViews() {
                     .hasLongSum()
                     .points()
                     .satisfiesExactly(point -> assertThat(point).hasValue(10)),
-            metricData -> {
-              assertThat(metricData)
-                  .hasInstrumentationScope(forMeter("meter1"))
-                  .hasName("counter1")
-                  .hasLongSum()
-                  .points()
-                  .satisfiesExactly(point -> assertThat(point).hasValue(10));
-              assertThat(metricData.getUnit()).isEqualTo("1");
-            });
+            metricData ->
+                assertThat(metricData)
+                    .hasInstrumentationScope(forMeter("meter1"))
+                    .hasName("counter1")
+                    .hasUnit("")
+                    .hasLongSum()
+                    .points()
+                    .satisfiesExactly(point -> assertThat(point).hasValue(10)));
 
     assertThat(metricStorageRegistryLogs.getEvents())
         .allSatisfy(
diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleCounterTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleCounterTest.java
@@ -131,7 +131,7 @@ void collectMetrics_WithMultipleCollects() {
                       .hasInstrumentationScope(INSTRUMENTATION_SCOPE_INFO)
                       .hasName("testCounter")
                       .hasDescription("")
-                      .hasUnit("1")
+                      .hasUnit("")
                       .hasDoubleSum()
                       .isMonotonic()
                       .isCumulative()
diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterProviderTest.java
@@ -115,7 +115,7 @@ void collectAllSyncInstruments() {
                     .hasResource(RESOURCE)
                     .hasInstrumentationScope(INSTRUMENTATION_SCOPE_INFO)
                     .hasDescription("")
-                    .hasUnit("1"))
+                    .hasUnit(""))
         .satisfiesExactlyInAnyOrder(
             metric ->
                 assertThat(metric)
@@ -284,7 +284,7 @@ void collectAllSyncInstruments_DeltaHistogram() {
                     .hasResource(RESOURCE)
                     .hasInstrumentationScope(INSTRUMENTATION_SCOPE_INFO)
                     .hasDescription("")
-                    .hasUnit("1")
+                    .hasUnit("")
                     .hasDoubleHistogram()
                     .isDelta()
                     .points()
@@ -313,7 +313,7 @@ void collectAllSyncInstruments_DeltaHistogram() {
                     .hasResource(RESOURCE)
                     .hasInstrumentationScope(INSTRUMENTATION_SCOPE_INFO)
                     .hasDescription("")
-                    .hasUnit("1")
+                    .hasUnit("")
                     .hasDoubleHistogram()
                     .isDelta()
                     .points()
@@ -365,7 +365,7 @@ void collectAllAsyncInstruments() {
                     .hasResource(RESOURCE)
                     .hasInstrumentationScope(INSTRUMENTATION_SCOPE_INFO)
                     .hasDescription("")
-                    .hasUnit("1"))
+                    .hasUnit(""))
         .satisfiesExactlyInAnyOrder(
             metric ->
                 assertThat(metric)
diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterTest.java
@@ -135,7 +135,7 @@ void builder_InvalidUnit() {
         .allMatch(
             log ->
                 log.equals(
-                    "Unit \"日\" is invalid. Instrument unit must be 63 or less ASCII characters. Using 1 for instrument my-instrument instead."));
+                    "Unit \"日\" is invalid. Instrument unit must be 63 or less ASCII characters. Using \"\" for instrument my-instrument instead."));
   }
 
   @Test

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ void builder_InvalidUnit() {`
`135`	`135`	`.allMatch(`
`136`	`136`	`log ->`
`137`	`137`	`log.equals(`
`138`		`- "Unit \"日\" is invalid. Instrument unit must be 63 or less ASCII characters. Using 1 for instrument my-instrument instead."));`
	`138`	`+ "Unit \"日\" is invalid. Instrument unit must be 63 or less ASCII characters. Using \"\" for instrument my-instrument instead."));`
`139`	`139`	`}`
`140`	`140`
`141`	`141`	`@Test`