Record internal metric for SQL cache misses
* Record internal metric for SQL cache misses
And use `SupportabilityMetrics` in `Instrumenter`
* Fix broken shouldStart() logic
* Code review comments
MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementSanitizer.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementSanitizer.java
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 org.slf4j.LoggerFactory;
+import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
/**
* 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);
});
}

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ClientInstrumenter.java
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,

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java
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
- * the operation being instrumented, or if {@code false} execute the operation directly without
- * calling those methods.
+ * call {@link #start(Context, Object)} and {@link #end(Context, Object, Object, Throwable)}
+ * around the operation being instrumented, or if {@code false} execute the operation directly
+ * 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;
+ }
+ if (suppressed) {
+ supportability.recordSuppressedSpan(spanKind, instrumentationName);
}
- return true;
+ return !suppressed;
}
/**

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java
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,

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java
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,

RENAME
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/SupportabilityMetrics.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java
* 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) {
- this(config, log::debug);
+ private static final SupportabilityMetrics INSTANCE =
+ 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();

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseTracer.java
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 =
- new SupportabilityMetrics(Config.get()).start();
+ private static final SupportabilityMetrics supportability = SupportabilityMetrics.instance();
private final Tracer tracer;
private final ContextPropagators propagators;

RENAME
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/tracer/SupportabilityMetricsTest.java
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetricsTest.java
* 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()
- .hasSize(3)
- .hasSameElementsAs(
- Arrays.asList(
+ .containsExactlyInAnyOrder(
"Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 2",
"Suppressed Spans by 'favoriteInstrumentation' (SERVER) : 1",
- "Suppressed Spans by 'otherInstrumentation' (INTERNAL) : 1"));
+ "Suppressed Spans by 'otherInstrumentation' (INTERNAL) : 1",
+ "Counter 'some counter' : 2",
+ "Counter 'another counter' : 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()
- .hasSize(1)
- .hasSameElementsAs(
- singletonList("Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 1"));
+ .containsExactlyInAnyOrder(
+ "Suppressed Spans by 'favoriteInstrumentation' (CLIENT) : 1",
+ "Counter 'some counter' : 1");
}
}

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