From 9a923758a4fff651a8ec0fbdf694821ae914fc7a Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 20 Oct 2022 17:53:01 -0700 Subject: [PATCH 1/4] Fix metric units --- docs/contributing/using-instrumenter-api.md | 2 +- .../api/instrumenter/http/HttpServerMetrics.java | 2 +- .../api/metrics/db/DbConnectionPoolMetrics.java | 10 +++++----- .../micrometer/v1_5/OpenTelemetryLongTaskTimer.java | 2 +- .../instrumentation/oshi/SystemMetrics.java | 6 +++--- .../instrumentation/runtimemetrics/BufferPools.java | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/contributing/using-instrumenter-api.md b/docs/contributing/using-instrumenter-api.md index bd2bd4ce9037..9b3005f9df60 100644 --- a/docs/contributing/using-instrumenter-api.md +++ b/docs/contributing/using-instrumenter-api.md @@ -349,7 +349,7 @@ class MyOperationMetrics implements OperationListener { MyOperationMetrics(Meter meter) { activeRequests = meter .upDownCounterBuilder("mylib.active_requests") - .setUnit("requests") + .setUnit("{requests}") .build(); } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java index 8eb17691cf26..b27026c5d4a8 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java @@ -57,7 +57,7 @@ private HttpServerMetrics(Meter meter) { activeRequests = meter .upDownCounterBuilder("http.server.active_requests") - .setUnit("requests") + .setUnit("{requests}") .setDescription("The number of concurrent HTTP requests that are currently in-flight") .build(); diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/metrics/db/DbConnectionPoolMetrics.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/metrics/db/DbConnectionPoolMetrics.java index 76b07961e1d6..e398250e6e1b 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/metrics/db/DbConnectionPoolMetrics.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/metrics/db/DbConnectionPoolMetrics.java @@ -58,7 +58,7 @@ public static DbConnectionPoolMetrics create( public ObservableLongMeasurement connections() { return meter .upDownCounterBuilder("db.client.connections.usage") - .setUnit("connections") + .setUnit("{connections}") .setDescription( "The number of connections that are currently in state described by the state attribute.") .buildObserver(); @@ -67,7 +67,7 @@ public ObservableLongMeasurement connections() { public ObservableLongMeasurement minIdleConnections() { return meter .upDownCounterBuilder("db.client.connections.idle.min") - .setUnit("connections") + .setUnit("{connections}") .setDescription("The minimum number of idle open connections allowed.") .buildObserver(); } @@ -75,7 +75,7 @@ public ObservableLongMeasurement minIdleConnections() { public ObservableLongMeasurement maxIdleConnections() { return meter .upDownCounterBuilder("db.client.connections.idle.max") - .setUnit("connections") + .setUnit("{connections}") .setDescription("The maximum number of idle open connections allowed.") .buildObserver(); } @@ -83,7 +83,7 @@ public ObservableLongMeasurement maxIdleConnections() { public ObservableLongMeasurement maxConnections() { return meter .upDownCounterBuilder("db.client.connections.max") - .setUnit("connections") + .setUnit("{connections}") .setDescription("The maximum number of open connections allowed.") .buildObserver(); } @@ -91,7 +91,7 @@ public ObservableLongMeasurement maxConnections() { public ObservableLongMeasurement pendingRequestsForConnection() { return meter .upDownCounterBuilder("db.client.connections.pending_requests") - .setUnit("requests") + .setUnit("{requests}") .setDescription( "The number of pending requests for an open connection, cumulative for the entire pool.") .buildObserver(); diff --git a/instrumentation/micrometer/micrometer-1.5/library/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryLongTaskTimer.java b/instrumentation/micrometer/micrometer-1.5/library/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryLongTaskTimer.java index c2aae70c0ee2..ed4d1e1dec16 100644 --- a/instrumentation/micrometer/micrometer-1.5/library/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryLongTaskTimer.java +++ b/instrumentation/micrometer/micrometer-1.5/library/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryLongTaskTimer.java @@ -44,7 +44,7 @@ final class OpenTelemetryLongTaskTimer extends DefaultLongTaskTimer implements R otelMeter .upDownCounterBuilder(name + ".active") .setDescription(Bridging.description(id)) - .setUnit("tasks") + .setUnit("{tasks}") .buildWithCallback( new LongMeasurementRecorder<>(this, DefaultLongTaskTimer::activeTasks, attributes)); this.observableDuration = diff --git a/instrumentation/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/SystemMetrics.java b/instrumentation/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/SystemMetrics.java index 1e3db20dff63..ccdcf17c62a2 100644 --- a/instrumentation/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/SystemMetrics.java +++ b/instrumentation/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/SystemMetrics.java @@ -76,7 +76,7 @@ public static void registerObservers(OpenTelemetry openTelemetry) { meter .counterBuilder("system.network.packets") .setDescription("System network packets") - .setUnit("packets") + .setUnit("{packets}") .buildWithCallback( r -> { for (NetworkIF networkIf : hal.getNetworkIFs()) { @@ -92,7 +92,7 @@ public static void registerObservers(OpenTelemetry openTelemetry) { meter .counterBuilder("system.network.errors") .setDescription("System network errors") - .setUnit("errors") + .setUnit("{errors}") .buildWithCallback( r -> { for (NetworkIF networkIf : hal.getNetworkIFs()) { @@ -123,7 +123,7 @@ public static void registerObservers(OpenTelemetry openTelemetry) { meter .counterBuilder("system.disk.operations") .setDescription("System disk operations") - .setUnit("operations") + .setUnit("{operations}") .buildWithCallback( r -> { for (HWDiskStore diskStore : hal.getDiskStores()) { diff --git a/instrumentation/runtime-metrics/library/src/main/java/io/opentelemetry/instrumentation/runtimemetrics/BufferPools.java b/instrumentation/runtime-metrics/library/src/main/java/io/opentelemetry/instrumentation/runtimemetrics/BufferPools.java index 998053cb32f3..0a7c4c806e6e 100644 --- a/instrumentation/runtime-metrics/library/src/main/java/io/opentelemetry/instrumentation/runtimemetrics/BufferPools.java +++ b/instrumentation/runtime-metrics/library/src/main/java/io/opentelemetry/instrumentation/runtimemetrics/BufferPools.java @@ -59,7 +59,7 @@ public static void registerObservers(OpenTelemetry openTelemetry) { meter .upDownCounterBuilder("process.runtime.jvm.buffer.count") .setDescription("The number of buffers in the pool") - .setUnit("buffers") + .setUnit("{buffers}") .buildWithCallback(callback(bufferBeans, BufferPoolMXBean::getCount)); } From 0187172b1f0d7995dd807ed516c8c64906c0255b Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 20 Oct 2022 18:15:46 -0700 Subject: [PATCH 2/4] Tests --- .../oshi/AbstractSystemMetricsTest.java | 8 +++++--- .../junit/db/DbConnectionPoolMetricsAssertions.java | 12 ++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/instrumentation/oshi/testing/src/main/java/io/opentelemetry/instrumentation/oshi/AbstractSystemMetricsTest.java b/instrumentation/oshi/testing/src/main/java/io/opentelemetry/instrumentation/oshi/AbstractSystemMetricsTest.java index 2fd86a83eec1..d62ee8e6cfe0 100644 --- a/instrumentation/oshi/testing/src/main/java/io/opentelemetry/instrumentation/oshi/AbstractSystemMetricsTest.java +++ b/instrumentation/oshi/testing/src/main/java/io/opentelemetry/instrumentation/oshi/AbstractSystemMetricsTest.java @@ -66,7 +66,7 @@ public void test() { metrics -> metrics.anySatisfy( metric -> - assertThat(metric).hasUnit("packets").hasLongSumSatisfying(sum -> {}))); + assertThat(metric).hasUnit("{packets}").hasLongSumSatisfying(sum -> {}))); testing() .waitAndAssertMetrics( "io.opentelemetry.oshi", @@ -74,7 +74,7 @@ public void test() { metrics -> metrics.anySatisfy( metric -> - assertThat(metric).hasUnit("errors").hasLongSumSatisfying(sum -> {}))); + assertThat(metric).hasUnit("{errors}").hasLongSumSatisfying(sum -> {}))); testing() .waitAndAssertMetrics( "io.opentelemetry.oshi", @@ -89,6 +89,8 @@ public void test() { metrics -> metrics.anySatisfy( metric -> - assertThat(metric).hasUnit("operations").hasLongSumSatisfying(sum -> {}))); + assertThat(metric) + .hasUnit("{operations}") + .hasLongSumSatisfying(sum -> {}))); } } diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java index aeb33083cb89..1d1dfc9d504f 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java @@ -132,7 +132,7 @@ private void verifyConnectionUsage() { private void verifyUsageMetric(MetricData metric) { assertThat(metric) - .hasUnit("connections") + .hasUnit("{connections}") .hasDescription( "The number of connections that are currently in state described by the state attribute.") .hasLongSumSatisfying( @@ -156,7 +156,7 @@ private void verifyMaxConnections() { private void verifyMaxConnectionsMetric(MetricData metric) { assertThat(metric) - .hasUnit("connections") + .hasUnit("{connections}") .hasDescription("The maximum number of open connections allowed.") .hasLongSumSatisfying(this::verifyPoolName); } @@ -170,7 +170,7 @@ private void verifyMinIdleConnections() { private void verifyMinIdleConnectionsMetric(MetricData metric) { assertThat(metric) - .hasUnit("connections") + .hasUnit("{connections}") .hasDescription("The minimum number of idle open connections allowed.") .hasLongSumSatisfying(this::verifyPoolName); } @@ -184,7 +184,7 @@ private void verifyMaxIdleConnections() { private void verifyMaxIdleConnectionsMetric(MetricData metric) { assertThat(metric) - .hasUnit("connections") + .hasUnit("{connections}") .hasDescription("The maximum number of idle open connections allowed.") .hasLongSumSatisfying(this::verifyPoolName); } @@ -203,7 +203,7 @@ private void verifyPendingRequests() { private void verifyPendingRequestsMetric(MetricData metric) { assertThat(metric) - .hasUnit("requests") + .hasUnit("{requests}") .hasDescription( "The number of pending requests for an open connection, cumulative for the entire pool.") .hasLongSumSatisfying(this::verifyPoolName); @@ -218,7 +218,7 @@ private void verifyTimeouts() { private void verifyTimeoutsMetric(MetricData metric) { assertThat(metric) - .hasUnit("timeouts") + .hasUnit("{timeouts}") .hasDescription( "The number of connection timeouts that have occurred trying to obtain a connection from the pool.") .hasLongSumSatisfying( From bb6b435e74f06b15e581e74443818323e4be8ee4 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 21 Oct 2022 12:01:14 -0700 Subject: [PATCH 3/4] Fix tests --- .../micrometer/v1_5/AbstractLongTaskTimerHistogramTest.java | 4 ++-- .../micrometer/v1_5/AbstractLongTaskTimerSecondsTest.java | 2 +- .../micrometer/v1_5/AbstractLongTaskTimerTest.java | 2 +- .../micrometer/v1_5/AbstractPrometheusModeTest.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerHistogramTest.java b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerHistogramTest.java index 3c9458a25552..4d3d1288654a 100644 --- a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerHistogramTest.java +++ b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerHistogramTest.java @@ -52,7 +52,7 @@ void testMicrometerHistogram() { metric -> assertThat(metric) .hasDescription("This is a test timer") - .hasUnit("tasks") + .hasUnit("{tasks}") .hasLongSumSatisfying( sum -> sum.isNotMonotonic() @@ -118,7 +118,7 @@ void testMicrometerHistogram() { metric -> assertThat(metric) .hasDescription("This is a test timer") - .hasUnit("tasks") + .hasUnit("{tasks}") .hasLongSumSatisfying( sum -> sum.isNotMonotonic() diff --git a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerSecondsTest.java b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerSecondsTest.java index 352807aaa0e3..9225c791d34f 100644 --- a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerSecondsTest.java +++ b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerSecondsTest.java @@ -42,7 +42,7 @@ void testLongTaskTimerWithBaseUnitSeconds() throws InterruptedException { metric -> assertThat(metric) .hasDescription("This is a test long task timer") - .hasUnit("tasks") + .hasUnit("{tasks}") .hasLongSumSatisfying( sum -> sum.isNotMonotonic() diff --git a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerTest.java b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerTest.java index d3ecd3ff5093..87b1f3e36d7c 100644 --- a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerTest.java +++ b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractLongTaskTimerTest.java @@ -42,7 +42,7 @@ void testLongTaskTimer() throws InterruptedException { metric -> assertThat(metric) .hasDescription("This is a test long task timer") - .hasUnit("tasks") + .hasUnit("{tasks}") .hasLongSumSatisfying( sum -> sum.isNotMonotonic() diff --git a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractPrometheusModeTest.java b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractPrometheusModeTest.java index a844da53e2c3..7bb782e10a76 100644 --- a/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractPrometheusModeTest.java +++ b/instrumentation/micrometer/micrometer-1.5/testing/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/AbstractPrometheusModeTest.java @@ -220,7 +220,7 @@ void testLongTaskTimer() throws InterruptedException { metric -> assertThat(metric) .hasDescription("This is a test long task timer") - .hasUnit("tasks") + .hasUnit("{tasks}") .hasLongSumSatisfying( sum -> sum.isNotMonotonic() From a0058535ffa8906bbc464b06dcbd3626c00d803f Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 21 Oct 2022 13:39:07 -0700 Subject: [PATCH 4/4] Fix test --- .../api/instrumenter/http/HttpServerMetricsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java index b8c02830310d..b17263bfeb46 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java @@ -79,7 +79,7 @@ void collectsMetrics() { .hasName("http.server.active_requests") .hasDescription( "The number of concurrent HTTP requests that are currently in-flight") - .hasUnit("requests") + .hasUnit("{requests}") .hasLongSumSatisfying( sum -> sum.hasPointsSatisfying(