Fix ktor metric metadata (#15516)

Author: jaydeluca

docs/instrumentation-list.yaml

@@ -7601,6 +7601,87 @@ libraries:
       default: true
     telemetry:
     - when: default
+      metrics:
+      - name: http.client.request.duration
+        description: Duration of HTTP client requests.
+        type: HISTOGRAM
+        unit: s
+        attributes:
+        - name: http.request.method
+          type: STRING
+        - name: http.response.status_code
+          type: LONG
+        - name: network.protocol.version
+          type: STRING
+        - name: server.address
+          type: STRING
+        - name: server.port
+          type: LONG
+      - name: http.server.request.duration
+        description: Duration of HTTP server requests.
+        type: HISTOGRAM
+        unit: s
+        attributes:
+        - name: http.request.method
+          type: STRING
+        - name: http.response.status_code
+          type: LONG
+        - name: http.route
+          type: STRING
+        - name: network.protocol.version
+          type: STRING
+        - name: url.scheme
+          type: STRING
+      spans:
+      - span_kind: CLIENT
+        attributes:
+        - name: error.type
+          type: STRING
+        - name: http.request.method
+          type: STRING
+        - name: http.request.method_original
+          type: STRING
+        - name: http.request.resend_count
+          type: LONG
+        - name: http.response.status_code
+          type: LONG
+        - name: network.protocol.version
+          type: STRING
+        - name: server.address
+          type: STRING
+        - name: server.port
+          type: LONG
+        - name: url.full
+          type: STRING
+      - span_kind: SERVER
+        attributes:
+        - name: client.address
+          type: STRING
+        - name: error.type
+          type: STRING
+        - name: http.request.method
+          type: STRING
+        - name: http.request.method_original
+          type: STRING
+        - name: http.response.status_code
+          type: LONG
+        - name: http.route
+          type: STRING
+        - name: network.protocol.version
+          type: STRING
+        - name: server.address
+          type: STRING
+        - name: server.port
+          type: LONG
+        - name: url.path
+          type: STRING
+        - name: url.query
+          type: STRING
+        - name: url.scheme
+          type: STRING
+        - name: user_agent.original
+          type: STRING
+    - when: otel.instrumentation.http.server.emit-experimental-telemetry=true
       metrics:
       - name: http.client.request.duration
         description: Duration of HTTP client requests.

instrumentation-docs/instrumentations.sh

@@ -155,6 +155,7 @@ readonly INSTRUMENTATIONS=(
   "ktor:ktor-1.0:library:test"
   "ktor:ktor-2.0:library:test"
   "ktor:ktor-3.0:library:test"
+  "ktor:ktor-3.0:library:testExperimental"
   "kubernetes-client-7.0:javaagent:test"
   "kubernetes-client-7.0:javaagent:testExperimental"
   "lettuce:lettuce-4.0:javaagent:test"

instrumentation/ktor/ktor-3.0/javaagent/build.gradle.kts

@@ -54,3 +54,16 @@ kotlin {
     javaParameters = true
   }
 }
+
+tasks {
+  val testExperimental by registering(Test::class) {
+    testClassesDirs = sourceSets.test.get().output.classesDirs
+    classpath = sourceSets.test.get().runtimeClasspath
+
+    jvmArgs("-Dotel.instrumentation.http.server.emit-experimental-telemetry=true")
+  }
+
+  check {
+    dependsOn(testExperimental)
+  }
+}

instrumentation/ktor/ktor-3.0/javaagent/src/test/java/io/opentelemetry/instrumentation/ktor/v3_0/ServerMetricsTest.kt

@@ -0,0 +1,32 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.ktor.v3_0
+
+import io.ktor.server.application.Application
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension
+import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty
+import org.junit.jupiter.api.extension.RegisterExtension
+
+@EnabledIfSystemProperty(
+  named = "otel.instrumentation.http.server.emit-experimental-telemetry",
+  matches = "true"
+)
+class ServerMetricsTest : AbstractKtorServerMetricsTest() {
+  companion object {
+    @JvmStatic
+    @RegisterExtension
+    val testing: InstrumentationExtension = HttpServerInstrumentationExtension.forAgent()
+  }
+
+  override fun serverInstall(application: Application) {
+    // javaagent automatically instruments the application
+  }
+
+  // For javaagent, HTTP server metrics are emitted by the Netty instrumentation
+  // since Ktor runs on top of Netty as its HTTP engine
+  override fun instrumentationName(): String = "io.opentelemetry.netty-4.1"
+}

instrumentation/ktor/ktor-3.0/library/build.gradle.kts

@@ -33,6 +33,21 @@ kotlin {
   }
 }
 
-tasks.test {
-  systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
+tasks {
+  withType<Test>().configureEach {
+    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
+  }
+
+  val testExperimental by registering(Test::class) {
+    testClassesDirs = sourceSets.test.get().output.classesDirs
+    classpath = sourceSets.test.get().runtimeClasspath
+
+    // this is used for enabling/disabling tests, library instrumentation doesn't use this flag
+    jvmArgs("-Dotel.instrumentation.http.server.emit-experimental-telemetry=true")
+    systemProperty("metadataConfig", "otel.instrumentation.http.server.emit-experimental-telemetry=true")
+  }
+
+  check {
+    dependsOn(testExperimental)
+  }
 }

instrumentation/ktor/ktor-3.0/library/src/test/kotlin/io/opentelemetry/instrumentation/ktor/v3_0/ServerMetricsTest.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.ktor.v3_0
+
+import io.ktor.server.application.install
+import io.opentelemetry.instrumentation.ktor.v2_0.common.internal.Experimental
+import io.opentelemetry.instrumentation.ktor.v3_0.InstrumentationProperties.INSTRUMENTATION_NAME
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension
+import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty
+import org.junit.jupiter.api.extension.RegisterExtension
+
+@EnabledIfSystemProperty(
+  named = "otel.instrumentation.http.server.emit-experimental-telemetry",
+  matches = "true"
+)
+class ServerMetricsTest : AbstractKtorServerMetricsTest() {
+  companion object {
+    @JvmStatic
+    @RegisterExtension
+    val testing: InstrumentationExtension = HttpServerInstrumentationExtension.forLibrary()
+  }
+
+  override fun serverInstall(application: io.ktor.server.application.Application) {
+    application.install(KtorServerTelemetry) {
+      setOpenTelemetry(testing.openTelemetry)
+      Experimental.emitExperimentalTelemetry(this)
+    }
+  }
+
+  override fun instrumentationName(): String = INSTRUMENTATION_NAME
+}

instrumentation/ktor/ktor-3.0/library/src/test/kotlin/io/opentelemetry/instrumentation/ktor/v3_0/KtorServerMetricsTest.kt renamed to instrumentation/ktor/ktor-3.0/testing/src/main/kotlin/io/opentelemetry/instrumentation/ktor/v3_0/AbstractKtorServerMetricsTest.kt

@@ -5,45 +5,49 @@
 
 package io.opentelemetry.instrumentation.ktor.v3_0
 
-import io.ktor.http.*
-import io.ktor.server.application.*
-import io.ktor.server.engine.*
-import io.ktor.server.netty.*
-import io.ktor.server.response.*
-import io.ktor.server.routing.*
-import io.opentelemetry.instrumentation.ktor.v2_0.common.internal.Experimental
-import io.opentelemetry.instrumentation.ktor.v3_0.InstrumentationProperties.INSTRUMENTATION_NAME
-import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension
+import io.ktor.http.HttpStatusCode
+import io.ktor.server.application.Application
+import io.ktor.server.engine.EmbeddedServer
+import io.ktor.server.engine.embeddedServer
+import io.ktor.server.netty.Netty
+import io.ktor.server.response.respondBytesWriter
+import io.ktor.server.response.respondText
+import io.ktor.server.routing.get
+import io.ktor.server.routing.routing
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerUsingTest
-import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension
 import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint
 import io.opentelemetry.sdk.metrics.data.MetricData
-import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions
+import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat
+import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo
 import io.opentelemetry.semconv.HttpAttributes
 import io.opentelemetry.semconv.UrlAttributes
 import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest
 import io.opentelemetry.testing.internal.armeria.common.HttpMethod
 import org.assertj.core.api.ThrowingConsumer
 import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.BeforeAll
-import org.junit.jupiter.api.extension.RegisterExtension
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.Arguments.arguments
 import org.junit.jupiter.params.provider.MethodSource
 import java.util.concurrent.TimeUnit
 import java.util.stream.Stream
 
-class KtorServerMetricsTest : AbstractHttpServerUsingTest<EmbeddedServer<*, *>>() {
-  companion object {
-    @JvmStatic
-    @RegisterExtension
-    val testing: InstrumentationExtension = HttpServerInstrumentationExtension.forLibrary()
-  }
+/**
+ * Abstract test class for testing experimental HTTP server metrics (http.server.active_requests).
+ * This is a regression test for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/15303
+ *
+ * Subclasses should use @EnabledIfSystemProperty to ensure tests only run when
+ * experimental flag is enabled.
+ */
+abstract class AbstractKtorServerMetricsTest : AbstractHttpServerUsingTest<EmbeddedServer<*, *>>() {
 
   private val errorDuringSendEndpoint = ServerEndpoint("errorDuringSend", "error-during-send", 500, "")
   private val errorAfterSendEndpoint = ServerEndpoint("errorAfterSend", "error-after-send", 200, "")
 
+  abstract fun serverInstall(application: Application)
+  abstract fun instrumentationName(): String
+
   @BeforeAll
   fun setupOptions() {
     startServer()
@@ -57,10 +61,7 @@ class KtorServerMetricsTest : AbstractHttpServerUsingTest<EmbeddedServer<*, *>>(
   override fun getContextPath() = ""
 
   override fun setupServer(): EmbeddedServer<*, *> = embeddedServer(Netty, port = port) {
-    install(KtorServerTelemetry) {
-      setOpenTelemetry(testing.openTelemetry)
-      Experimental.emitExperimentalTelemetry(this)
-    }
+    serverInstall(this)
 
     routing {
       get(errorDuringSendEndpoint.path) {
@@ -69,7 +70,10 @@ class KtorServerMetricsTest : AbstractHttpServerUsingTest<EmbeddedServer<*, *>>(
         }
       }
       get(errorAfterSendEndpoint.path) {
-        call.respondText(errorAfterSendEndpoint.body, status = HttpStatusCode.fromValue(errorAfterSendEndpoint.status))
+        call.respondText(
+          errorAfterSendEndpoint.body,
+          status = HttpStatusCode.fromValue(errorAfterSendEndpoint.status)
+        )
         throw IllegalArgumentException("exception")
       }
     }
@@ -92,20 +96,20 @@ class KtorServerMetricsTest : AbstractHttpServerUsingTest<EmbeddedServer<*, *>>(
       // we expect server error
     }
 
-    testing.waitAndAssertMetrics(
-      INSTRUMENTATION_NAME,
+    testing().waitAndAssertMetrics(
+      instrumentationName(),
       "http.server.active_requests"
     ) { metrics ->
       metrics!!.anySatisfy(ThrowingConsumer { metric: MetricData? ->
-        OpenTelemetryAssertions.assertThat(metric)
+        assertThat(metric)
           .hasDescription("Number of active HTTP server requests.")
           .hasUnit("{requests}")
           .hasLongSumSatisfying { sum ->
             sum.hasPointsSatisfying({ point ->
               point.hasValue(0)
                 .hasAttributesSatisfying {
-                  OpenTelemetryAssertions.equalTo(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
-                  OpenTelemetryAssertions.equalTo(UrlAttributes.URL_PATH, endpoint.path)
+                  equalTo(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
+                  equalTo(UrlAttributes.URL_PATH, endpoint.path)
                 }
             })
           }