Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a few missing @Nullables in instrumentation-api #3806

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import io.opentelemetry.instrumentation.api.caching.Cache;
import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* This class is responsible for masking potentially sensitive parameters in SQL (and SQL-like)
Expand All @@ -21,7 +22,7 @@ public final class SqlStatementSanitizer {
private static final Cache<String, SqlStatementInfo> sqlToStatementInfoCache =
Cache.newBuilder().setMaximumSize(1000).build();

public static SqlStatementInfo sanitize(String statement) {
public static SqlStatementInfo sanitize(@Nullable String statement) {
if (!isStatementSanitizationEnabled() || statement == null) {
return SqlStatementInfo.create(statement, null, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ protected abstract void onEnd(
* Sets the {@code value} with the given {@code key} to the {@link AttributesBuilder} if {@code
* value} is not {@code null}.
*/
protected static <T> void set(AttributesBuilder attributes, AttributeKey<T> key, T value) {
protected static <T> void set(
AttributesBuilder attributes, AttributeKey<T> key, @Nullable T value) {
if (value != null) {
attributes.put(key, value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.instrumentation.api.instrumenter;

import java.time.Instant;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Extractor of the end time of response processing. An {@link EndTimeExtractor} should always use
Expand All @@ -16,5 +17,5 @@
public interface EndTimeExtractor<RESPONSE> {

/** Returns the timestamp marking the end of the response processing. */
Instant extract(RESPONSE response);
Instant extract(@Nullable RESPONSE response);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import io.opentelemetry.instrumentation.api.tracer.ClassNames;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* A helper {@link SpanNameExtractor} implementation for instrumentations that target specific Java
Expand Down Expand Up @@ -37,7 +38,7 @@ public String extract(REQUEST request) {
return className + "." + methodName;
}

private static String defaultString(String s) {
private static String defaultString(@Nullable String s) {
return s == null ? "<unknown>" : s;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
}

@Override
protected final void onEnd(AttributesBuilder attributes, REQUEST request, RESPONSE response) {
protected final void onEnd(
AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
// No response attributes
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -68,6 +69,7 @@ public static void debugContextLeakIfEnabled() {
}
}

@Nullable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to return an empty list instead of null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but it'd result in a bit more code being added - and since it's a private, internal utility method I think it's fine to keep @Nullable here.

private static List<Propagation> getPropagations(Context context) {
return context.get(THREAD_PROPAGATION_LOCATIONS);
}
Expand Down