Record internal metric for SQL cache misses (#2747)

* Record internal metric for SQL cache misses

And use `SupportabilityMetrics` in `Instrumenter`

* Fix broken shouldStart() logic

* Code review comments

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementSanitizer.java

@@ -6,17 +6,17 @@
 package io.opentelemetry.instrumentation.api.db;
 
 import static io.opentelemetry.instrumentation.api.db.StatementSanitizationConfig.isStatementSanitizationEnabled;
+import static io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics.CounterNames.SQL_STATEMENT_SANITIZER_CACHE_MISS;
 
 import io.opentelemetry.instrumentation.api.caching.Cache;
-import org.slf4j.Logger;
+import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
-import org.slf4j.LoggerFactory;
 
 /**
  * This class is responsible for masking potentially sensitive parameters in SQL (and SQL-like)
  * statements and queries.
  */
 public final class SqlStatementSanitizer {
-  private static final Logger log = LoggerFactory.getLogger(SqlStatementSanitizer.class);
+  private static final SupportabilityMetrics supportability = SupportabilityMetrics.instance();
 
   private static final Cache<String, SqlStatementInfo> sqlToStatementInfoCache =
       Cache.newBuilder().setMaximumSize(1000).build();
@@ -28,7 +28,7 @@ public final class SqlStatementSanitizer {
     return sqlToStatementInfoCache.computeIfAbsent(
         statement,
         k -> {
-          log.trace("SQL statement cache miss");
+          supportability.incrementCounter(SQL_STATEMENT_SANITIZER_CACHE_MISS);
           return AutoSqlSanitizer.sanitize(statement);
         });
   }

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ClientInstrumenter.java

@@ -17,6 +17,7 @@ final class ClientInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
   private final TextMapSetter<REQUEST> setter;
 
   ClientInstrumenter(
+      String instrumentationName,
       Tracer tracer,
       SpanNameExtractor<? super REQUEST> spanNameExtractor,
       SpanKindExtractor<? super REQUEST> spanKindExtractor,
@@ -26,6 +27,7 @@ final class ClientInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
       ContextPropagators propagators,
       TextMapSetter<REQUEST> setter) {
     super(
+        instrumentationName,
         tracer,
         spanNameExtractor,
         spanKindExtractor,

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

@@ -14,6 +14,7 @@ import io.opentelemetry.api.trace.SpanBuilder;
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.Tracer;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
 import io.opentelemetry.instrumentation.api.tracer.ClientSpan;
 import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
 import java.util.List;
@@ -46,6 +47,9 @@ public class Instrumenter<REQUEST, RESPONSE> {
     return new InstrumenterBuilder<>(openTelemetry, instrumentationName, spanNameExtractor);
   }
 
+  private static final SupportabilityMetrics supportability = SupportabilityMetrics.instance();
+
+  private final String instrumentationName;
   private final Tracer tracer;
   private final SpanNameExtractor<? super REQUEST> spanNameExtractor;
   private final SpanKindExtractor<? super REQUEST> spanKindExtractor;
@@ -54,12 +58,14 @@ public class Instrumenter<REQUEST, RESPONSE> {
   private final ErrorCauseExtractor errorCauseExtractor;
 
   Instrumenter(
+      String instrumentationName,
       Tracer tracer,
       SpanNameExtractor<? super REQUEST> spanNameExtractor,
       SpanKindExtractor<? super REQUEST> spanKindExtractor,
       SpanStatusExtractor<? super REQUEST, ? super RESPONSE> spanStatusExtractor,
       List<? extends AttributesExtractor<? super REQUEST, ? super RESPONSE>> extractors,
       ErrorCauseExtractor errorCauseExtractor) {
+    this.instrumentationName = instrumentationName;
     this.tracer = tracer;
     this.spanNameExtractor = spanNameExtractor;
     this.spanKindExtractor = spanKindExtractor;
@@ -70,21 +76,27 @@ public class Instrumenter<REQUEST, RESPONSE> {
 
   /**
    * Returns whether instrumentation should be applied for the {@link REQUEST}. If {@code true},
-   * call {@link #start(Context, Object)} and {@link #end(Context, Object, Object, Throwable)} arond
+   * call {@link #start(Context, Object)} and {@link #end(Context, Object, Object, Throwable)}
-   * the operation being instrumented, or if {@code false} execute the operation directly without
+   * around the operation being instrumented, or if {@code false} execute the operation directly
-   * calling those methods.
+   * without calling those methods.
    */
   public boolean shouldStart(Context parentContext, REQUEST request) {
+    boolean suppressed = false;
     SpanKind spanKind = spanKindExtractor.extract(request);
     switch (spanKind) {
       case SERVER:
-        return ServerSpan.fromContextOrNull(parentContext) == null;
+        suppressed = ServerSpan.fromContextOrNull(parentContext) != null;
+        break;
       case CLIENT:
-        return ClientSpan.fromContextOrNull(parentContext) == null;
+        suppressed = ClientSpan.fromContextOrNull(parentContext) != null;
+        break;
       default:
-        // fallthrough
+        break;
     }
-    return true;
+    if (suppressed) {
+      supportability.recordSuppressedSpan(spanKind, instrumentationName);
+    }
+    return !suppressed;
   }
 
   /**

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java

@@ -114,6 +114,7 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
       InstrumenterConstructor<REQUEST, RESPONSE> constructor,
       SpanKindExtractor<? super REQUEST> spanKindExtractor) {
     return constructor.create(
+        instrumentationName,
         openTelemetry.getTracer(instrumentationName, InstrumentationVersion.VERSION),
         spanNameExtractor,
         spanKindExtractor,
@@ -124,6 +125,7 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
 
   private interface InstrumenterConstructor<RQ, RS> {
     Instrumenter<RQ, RS> create(
+        String instrumentationName,
         Tracer tracer,
         SpanNameExtractor<? super RQ> spanNameExtractor,
         SpanKindExtractor<? super RQ> spanKindExtractor,
@@ -137,8 +139,15 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
 
     static <RQ, RS> InstrumenterConstructor<RQ, RS> propagatingToDownstream(
         ContextPropagators propagators, TextMapSetter<RQ> setter) {
-      return (tracer, spanName, spanKind, spanStatus, attributes, errorCauseExtractor) ->
+      return (instrumentationName,
+          tracer,
+          spanName,
+          spanKind,
+          spanStatus,
+          attributes,
+          errorCauseExtractor) ->
           new ClientInstrumenter<>(
+              instrumentationName,
               tracer,
               spanName,
               spanKind,
@@ -151,8 +160,15 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
 
     static <RQ, RS> InstrumenterConstructor<RQ, RS> propagatingFromUpstream(
         ContextPropagators propagators, TextMapGetter<RQ> getter) {
-      return (tracer, spanName, spanKind, spanStatus, attributes, errorCauseExtractor) ->
+      return (instrumentationName,
+          tracer,
+          spanName,
+          spanKind,
+          spanStatus,
+          attributes,
+          errorCauseExtractor) ->
           new ServerInstrumenter<>(
+              instrumentationName,
               tracer,
               spanName,
               spanKind,

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java

@@ -17,6 +17,7 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
   private final TextMapGetter<REQUEST> getter;
 
   ServerInstrumenter(
+      String instrumentationName,
       Tracer tracer,
       SpanNameExtractor<? super REQUEST> spanNameExtractor,
       SpanKindExtractor<? super REQUEST> spanKindExtractor,
@@ -26,6 +27,7 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
       ContextPropagators propagators,
       TextMapGetter<REQUEST> getter) {
     super(
+        instrumentationName,
         tracer,
         spanNameExtractor,
         spanKindExtractor,

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.api.tracer;
+package io.opentelemetry.instrumentation.api.internal;
 
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.api.config.Config;
@@ -16,15 +16,19 @@ import java.util.function.Consumer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-class SupportabilityMetrics {
+public final class SupportabilityMetrics {
   private static final Logger log = LoggerFactory.getLogger(SupportabilityMetrics.class);
   private final boolean agentDebugEnabled;
   private final Consumer<String> reporter;
 
   private final ConcurrentMap<String, KindCounters> suppressionCounters = new ConcurrentHashMap<>();
+  private final ConcurrentMap<String, LongAdder> counters = new ConcurrentHashMap<>();
 
-  public SupportabilityMetrics(Config config) {
+  private static final SupportabilityMetrics INSTANCE =
-    this(config, log::debug);
+      new SupportabilityMetrics(Config.get(), log::debug).start();
+
+  public static SupportabilityMetrics instance() {
+    return INSTANCE;
   }
 
   // visible for testing
@@ -33,7 +37,7 @@ class SupportabilityMetrics {
     this.reporter = reporter;
   }
 
-  void recordSuppressedSpan(SpanKind kind, String instrumentationName) {
+  public void recordSuppressedSpan(SpanKind kind, String instrumentationName) {
     if (!agentDebugEnabled) {
       return;
     }
@@ -43,6 +47,14 @@ class SupportabilityMetrics {
         .increment(kind);
   }
 
+  public void incrementCounter(String counterName) {
+    if (!agentDebugEnabled) {
+      return;
+    }
+
+    counters.computeIfAbsent(counterName, k -> new LongAdder()).increment();
+  }
+
   // visible for testing
   void report() {
     suppressionCounters.forEach(
@@ -55,6 +67,13 @@ class SupportabilityMetrics {
             }
           }
         });
+    counters.forEach(
+        (counterName, counter) -> {
+          long value = counter.sumThenReset();
+          if (value > 0) {
+            reporter.accept("Counter '" + counterName + "' : " + value);
+          }
+        });
   }
 
   SupportabilityMetrics start() {
@@ -72,6 +91,13 @@ class SupportabilityMetrics {
     return this;
   }
 
+  public static final class CounterNames {
+    public static final String SQL_STATEMENT_SANITIZER_CACHE_MISS =
+        "SqlStatementSanitizer cache miss";
+
+    private CounterNames() {}
+  }
+
   // this class is threadsafe.
   private static class KindCounters {
     private final LongAdder server = new LongAdder();

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseTracer.java

@@ -18,8 +18,8 @@ import io.opentelemetry.context.propagation.TextMapGetter;
 import io.opentelemetry.context.propagation.TextMapPropagator;
 import io.opentelemetry.context.propagation.TextMapSetter;
 import io.opentelemetry.instrumentation.api.InstrumentationVersion;
-import io.opentelemetry.instrumentation.api.config.Config;
 import io.opentelemetry.instrumentation.api.context.ContextPropagationDebug;
+import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.UndeclaredThrowableException;
@@ -48,9 +48,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
  * are able to see those attributes in the {@code onStart()} method and can freely read/modify them.
  */
 public abstract class BaseTracer {
-  // should we make this injectable?
+  private static final SupportabilityMetrics supportability = SupportabilityMetrics.instance();
-  private static final SupportabilityMetrics supportability =
-      new SupportabilityMetrics(Config.get()).start();
 
   private final Tracer tracer;
   private final ContextPropagators propagators;

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetricsTest.java

@@ -3,15 +3,13 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.api.tracer;
+package io.opentelemetry.instrumentation.api.internal;
 
-import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.api.config.Config;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import org.junit.jupiter.api.Test;
@@ -28,6 +26,9 @@ class SupportabilityMetricsTest {
     metrics.recordSuppressedSpan(SpanKind.SERVER, "favoriteInstrumentation");
     metrics.recordSuppressedSpan(SpanKind.CLIENT, "favoriteInstrumentation");
     metrics.recordSuppressedSpan(SpanKind.INTERNAL, "otherInstrumentation");
+    metrics.incrementCounter("some counter");
+    metrics.incrementCounter("another counter");
+    metrics.incrementCounter("some counter");
 
     metrics.report();
 
@@ -45,17 +46,19 @@ class SupportabilityMetricsTest {
     metrics.recordSuppressedSpan(SpanKind.SERVER, "favoriteInstrumentation");
     metrics.recordSuppressedSpan(SpanKind.CLIENT, "favoriteInstrumentation");
     metrics.recordSuppressedSpan(SpanKind.INTERNAL, "otherInstrumentation");
+    metrics.incrementCounter("some counter");
+    metrics.incrementCounter("another counter");
+    metrics.incrementCounter("some counter");
 
     metrics.report();
 
     assertThat(reports)
-        .isNotEmpty()
+        .containsExactlyInAnyOrder(
-        .hasSize(3)
+            "Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 2",
-        .hasSameElementsAs(
+            "Suppressed Spans by 'favoriteInstrumentation' (SERVER) : 1",
-            Arrays.asList(
+            "Suppressed Spans by 'otherInstrumentation' (INTERNAL) : 1",
-                "Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 2",
+            "Counter 'some counter' : 2",
-                "Suppressed Spans by 'favoriteInstrumentation' (SERVER) : 1",
+            "Counter 'another counter' : 1");
-                "Suppressed Spans by 'otherInstrumentation' (INTERNAL) : 1"));
   }
 
   @Test
@@ -66,14 +69,14 @@ class SupportabilityMetricsTest {
             Config.create(Collections.singletonMap("otel.javaagent.debug", "true")), reports::add);
 
     metrics.recordSuppressedSpan(SpanKind.CLIENT, "favoriteInstrumentation");
+    metrics.incrementCounter("some counter");
 
     metrics.report();
     metrics.report();
 
     assertThat(reports)
-        .isNotEmpty()
+        .containsExactlyInAnyOrder(
-        .hasSize(1)
+            "Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 1",
-        .hasSameElementsAs(
+            "Counter 'some counter' : 1");
-            singletonList("Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 1"));
   }
 }