Skip to content

Commit 3f3780c

Browse files
authored
Add guidance for null checking, promote ApiUsageLogger to opentelemetry-common public API (#8318)
1 parent e224e19 commit 3f3780c

15 files changed

Lines changed: 299 additions & 164 deletions

File tree

api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableEntryMetadata.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
package io.opentelemetry.api.baggage;
77

88
import com.google.auto.value.AutoValue;
9-
import io.opentelemetry.api.internal.ApiUsageLogger;
9+
import io.opentelemetry.common.impl.ApiUsageLogger;
1010
import javax.annotation.concurrent.Immutable;
1111

1212
@Immutable
@@ -25,7 +25,7 @@ abstract class ImmutableEntryMetadata implements BaggageEntryMetadata {
2525
*/
2626
static ImmutableEntryMetadata create(String metadata) {
2727
if (metadata == null) {
28-
ApiUsageLogger.log("metadata is null");
28+
ApiUsageLogger.logNullParam(BaggageEntryMetadata.class, "create", "metadata");
2929
return EMPTY;
3030
}
3131
return new AutoValue_ImmutableEntryMetadata(metadata);

api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java

Lines changed: 0 additions & 42 deletions
This file was deleted.

api/all/src/main/java/io/opentelemetry/api/trace/DefaultTracer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import io.opentelemetry.api.common.AttributeKey;
99
import io.opentelemetry.api.common.Attributes;
10-
import io.opentelemetry.api.internal.ApiUsageLogger;
10+
import io.opentelemetry.common.impl.ApiUsageLogger;
1111
import io.opentelemetry.context.Context;
1212
import java.util.concurrent.TimeUnit;
1313
import javax.annotation.Nullable;
@@ -55,7 +55,7 @@ public Span startSpan() {
5555
@Override
5656
public NoopSpanBuilder setParent(Context context) {
5757
if (context == null) {
58-
ApiUsageLogger.log("context is null");
58+
ApiUsageLogger.logNullParam(SpanBuilder.class, "setParent", "context");
5959
return this;
6060
}
6161
spanContext = Span.fromContext(context).getSpanContext();

api/all/src/main/java/io/opentelemetry/api/trace/Span.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import io.opentelemetry.api.common.AttributeKey;
1212
import io.opentelemetry.api.common.Attributes;
13-
import io.opentelemetry.api.internal.ApiUsageLogger;
13+
import io.opentelemetry.common.impl.ApiUsageLogger;
1414
import io.opentelemetry.context.Context;
1515
import io.opentelemetry.context.ImplicitContextKeyed;
1616
import java.time.Instant;
@@ -43,7 +43,7 @@ static Span current() {
4343
*/
4444
static Span fromContext(Context context) {
4545
if (context == null) {
46-
ApiUsageLogger.log("context is null");
46+
ApiUsageLogger.logNullParam(Span.class, "fromContext", "context");
4747
return Span.getInvalid();
4848
}
4949
Span span = context.get(SpanContextKey.KEY);
@@ -57,7 +57,7 @@ static Span fromContext(Context context) {
5757
@Nullable
5858
static Span fromContextOrNull(Context context) {
5959
if (context == null) {
60-
ApiUsageLogger.log("context is null");
60+
ApiUsageLogger.logNullParam(Span.class, "fromContextOrNull", "context");
6161
return null;
6262
}
6363
return context.get(SpanContextKey.KEY);
@@ -78,7 +78,7 @@ static Span getInvalid() {
7878
*/
7979
static Span wrap(SpanContext spanContext) {
8080
if (spanContext == null) {
81-
ApiUsageLogger.log("context is null");
81+
ApiUsageLogger.logNullParam(Span.class, "wrap", "spanContext");
8282
return getInvalid();
8383
}
8484
return PropagatedSpan.create(spanContext);

api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55

66
package io.opentelemetry.api.trace;
77

8-
import io.opentelemetry.api.internal.ApiUsageLogger;
98
import io.opentelemetry.api.internal.OtelEncodingUtils;
109
import io.opentelemetry.api.internal.TemporaryBuffers;
10+
import io.opentelemetry.common.impl.ApiUsageLogger;
1111
import javax.annotation.concurrent.Immutable;
1212

1313
/**
@@ -73,7 +73,7 @@ public static boolean isValid(CharSequence spanId) {
7373
*/
7474
public static String fromBytes(byte[] spanIdBytes) {
7575
if (spanIdBytes == null || spanIdBytes.length < BYTES_LENGTH) {
76-
ApiUsageLogger.log("spanIdBytes is null or too short");
76+
ApiUsageLogger.logUsageIssue(SpanId.class, "fromBytes", "spanIdBytes is null or too short");
7777
return INVALID;
7878
}
7979
char[] result = TemporaryBuffers.chars(HEX_LENGTH);

api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55

66
package io.opentelemetry.api.trace;
77

8-
import io.opentelemetry.api.internal.ApiUsageLogger;
98
import io.opentelemetry.api.internal.OtelEncodingUtils;
109
import io.opentelemetry.api.internal.TemporaryBuffers;
10+
import io.opentelemetry.common.impl.ApiUsageLogger;
1111
import javax.annotation.concurrent.Immutable;
1212

1313
/**
@@ -77,7 +77,7 @@ public static boolean isValid(CharSequence traceId) {
7777
*/
7878
public static String fromBytes(byte[] traceIdBytes) {
7979
if (traceIdBytes == null || traceIdBytes.length < BYTES_LENGTH) {
80-
ApiUsageLogger.log("traceIdBytes is null or too short");
80+
ApiUsageLogger.logUsageIssue(TraceId.class, "fromBytes", "traceIdBytes is null or too short");
8181
return INVALID;
8282
}
8383
char[] result = TemporaryBuffers.chars(HEX_LENGTH);

api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java

Lines changed: 0 additions & 26 deletions
This file was deleted.

api/incubator/src/main/java/io/opentelemetry/api/incubator/trace/ExtendedDefaultTracer.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@
88
import io.opentelemetry.api.common.AttributeKey;
99
import io.opentelemetry.api.common.Attributes;
1010
import io.opentelemetry.api.incubator.propagation.ExtendedContextPropagators;
11-
import io.opentelemetry.api.internal.ApiUsageLogger;
1211
import io.opentelemetry.api.trace.Span;
12+
import io.opentelemetry.api.trace.SpanBuilder;
1313
import io.opentelemetry.api.trace.SpanContext;
1414
import io.opentelemetry.api.trace.SpanKind;
1515
import io.opentelemetry.api.trace.Tracer;
16+
import io.opentelemetry.common.impl.ApiUsageLogger;
1617
import io.opentelemetry.context.Context;
1718
import io.opentelemetry.context.propagation.ContextPropagators;
1819
import java.util.Map;
@@ -63,7 +64,7 @@ public Span startSpan() {
6364
@Override
6465
public NoopSpanBuilder setParent(Context context) {
6566
if (context == null) {
66-
ApiUsageLogger.log("context is null");
67+
ApiUsageLogger.logNullParam(SpanBuilder.class, "setParent", "context");
6768
return this;
6869
}
6970
spanContext = Span.fromContext(context).getSpanContext();
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.common.impl;
7+
8+
import java.util.concurrent.atomic.AtomicBoolean;
9+
import java.util.logging.Level;
10+
import java.util.logging.Logger;
11+
12+
/**
13+
* A utility for logging API misuse, allowing operators to diagnose invalid usage with a single
14+
* logging configuration entry.
15+
*
16+
* <p>Logs at {@link Level#FINEST} by default, so messages are silent in production unless
17+
* explicitly enabled. Each log record includes a {@link Throwable} to make the offending call site
18+
* visible in the stack trace without requiring the exception to be thrown.
19+
*
20+
* <p>The first time any API misuse is detected, a one-time {@link Level#WARNING} is emitted to the
21+
* {@code io.opentelemetry.usage} logger. This warning is visible under default logging
22+
* configuration and signals that API misuse is occurring. To see the full details of every misuse
23+
* event (message and stack trace), configure the {@code io.opentelemetry.usage} logger at {@link
24+
* Level#FINEST}.
25+
*
26+
* <p>This class is not intended for use by application developers. Its API is stable and will not
27+
* be changed or removed in a backwards-incompatible manner.
28+
*/
29+
public final class ApiUsageLogger {
30+
31+
/** The logger name used for all API-misuse diagnostics. */
32+
private static final Logger LOGGER = Logger.getLogger("io.opentelemetry.usage");
33+
34+
private static final AtomicBoolean WARN_ONCE = new AtomicBoolean();
35+
36+
/**
37+
* Log that {@code paramName} was null in {@code apiClass#methodName}.
38+
*
39+
* <p>Convenience overload of {@link #logUsageIssue(Class, String, String)} for the common case of
40+
* a null parameter that should not be null.
41+
*
42+
* @param apiClass the public API class where the misuse occurred
43+
* @param methodName the name of the method where the misuse occurred
44+
* @param paramName the name of the parameter that was null
45+
*/
46+
public static void logNullParam(Class<?> apiClass, String methodName, String paramName) {
47+
logUsageIssue(apiClass, methodName, paramName + " is null");
48+
}
49+
50+
/**
51+
* Log a misuse of {@code apiClass#methodName} with the given {@code message}.
52+
*
53+
* <p>Logs at {@link Level#FINEST} and includes a stack trace.
54+
*
55+
* @param apiClass the public API class where the misuse occurred
56+
* @param methodName the name of the method where the misuse occurred
57+
* @param message a brief description of the problem
58+
*/
59+
public static void logUsageIssue(Class<?> apiClass, String methodName, String message) {
60+
if (WARN_ONCE.compareAndSet(false, true)) {
61+
LOGGER.warning(
62+
"OpenTelemetry API usage issue detected. To see more details, enable FINEST logging for io.opentelemetry.usage. Stacktraces are includes to identify the offending call site.");
63+
}
64+
if (LOGGER.isLoggable(Level.FINEST)) {
65+
LOGGER.log(
66+
Level.FINEST,
67+
apiClass.getSimpleName() + "." + methodName + "(): " + message,
68+
new Throwable());
69+
}
70+
}
71+
72+
private ApiUsageLogger() {}
73+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.common.impl;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
10+
import io.github.netmikey.logunit.api.LogCapturer;
11+
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
12+
import java.lang.reflect.Field;
13+
import java.util.concurrent.atomic.AtomicBoolean;
14+
import org.junit.jupiter.api.BeforeEach;
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.api.extension.RegisterExtension;
17+
import org.slf4j.event.Level;
18+
19+
@SuppressLogger(loggerName = "io.opentelemetry.usage")
20+
class ApiUsageLoggerTest {
21+
22+
@RegisterExtension
23+
LogCapturer apiUsageLogs =
24+
LogCapturer.create().captureForLogger("io.opentelemetry.usage", Level.TRACE);
25+
26+
@BeforeEach
27+
void resetWarnOnce() throws Exception {
28+
Field warnOnce = ApiUsageLogger.class.getDeclaredField("WARN_ONCE");
29+
warnOnce.setAccessible(true);
30+
((AtomicBoolean) warnOnce.get(null)).set(false);
31+
}
32+
33+
@Test
34+
void log() {
35+
ApiUsageLogger.logUsageIssue(ApiUsageLoggerTest.class, "log", "thing went wrong");
36+
apiUsageLogs.assertContains("ApiUsageLoggerTest.log(): thing went wrong");
37+
}
38+
39+
@Test
40+
void logNullParam() {
41+
ApiUsageLogger.logNullParam(ApiUsageLoggerTest.class, "logNullParam", "myParam");
42+
apiUsageLogs.assertContains("ApiUsageLoggerTest.logNullParam(): myParam is null");
43+
}
44+
45+
@Test
46+
void warnOnce() {
47+
ApiUsageLogger.logUsageIssue(ApiUsageLoggerTest.class, "warnOnce", "first");
48+
ApiUsageLogger.logUsageIssue(ApiUsageLoggerTest.class, "warnOnce", "second");
49+
long count =
50+
apiUsageLogs.getEvents().stream()
51+
.filter(e -> e.getMessage().contains("OpenTelemetry API usage issue detected"))
52+
.count();
53+
assertEquals(1, count);
54+
}
55+
}

0 commit comments

Comments
 (0)