diff --git a/internal-api/src/main/java/datadog/trace/api/TagMap.java b/internal-api/src/main/java/datadog/trace/api/TagMap.java index f8f33f1c023..1610cb3a5f2 100644 --- a/internal-api/src/main/java/datadog/trace/api/TagMap.java +++ b/internal-api/src/main/java/datadog/trace/api/TagMap.java @@ -16,6 +16,8 @@ import java.util.function.Function; import java.util.stream.Stream; import java.util.stream.StreamSupport; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * A super simple hash map designed for... @@ -147,50 +149,53 @@ static Ledger ledger(int size) { * are implemented more efficiently. */ @Deprecated - Object put(String tag, Object value); + Object put(@Nonnull String tag, Object value); /** Sets value without returning prior value - more efficient than {@link Map#put} */ - void set(String tag, Object value); + void set(@Nonnull String tag, @Nonnull Object value); /** * Similar to {@link TagMap#set(String, Object)} but more efficient when working with * CharSequences and Strings. Depending on this situation, this methods avoids having to do type * resolution later on */ - void set(String tag, CharSequence value); + void set(@Nonnull String tag, @Nonnull CharSequence value); - void set(String tag, boolean value); + void set(@Nonnull String tag, boolean value); - void set(String tag, int value); + void set(@Nonnull String tag, int value); - void set(String tag, long value); + void set(@Nonnull String tag, long value); - void set(String tag, float value); + void set(@Nonnull String tag, float value); - void set(String tag, double value); + void set(@Nonnull String tag, double value); - void set(EntryReader newEntry); + /** A null reader is a no-op (see the null-tolerance contract on {@link #getAndSet(Entry)}). */ + void set(@Nullable EntryReader newEntry); /** sets the value while returning the prior Entry */ - Entry getAndSet(String tag, Object value); + Entry getAndSet(@Nonnull String tag, Object value); - Entry getAndSet(String tag, CharSequence value); + Entry getAndSet(@Nonnull String tag, CharSequence value); - Entry getAndSet(String tag, boolean value); + Entry getAndSet(@Nonnull String tag, boolean value); - Entry getAndSet(String tag, int value); + Entry getAndSet(@Nonnull String tag, int value); - Entry getAndSet(String tag, long value); + Entry getAndSet(@Nonnull String tag, long value); - Entry getAndSet(String tag, float value); + Entry getAndSet(@Nonnull String tag, float value); - Entry getAndSet(String tag, double value); + Entry getAndSet(@Nonnull String tag, double value); /** - * TagMap specific method that places an Entry directly into an optimized TagMap avoiding need to - * allocate a new Entry object + * Places an Entry directly into the map, avoiding a new Entry allocation. Null-tolerant: a null + * {@code newEntry} is a no-op returning null, so an Entry producer (e.g. {@link + * Entry#create(String, Object)} for a null/empty value) can emit "no tag" without the caller + * filtering. Contrast the strict {@link Nonnull} {@code set(String, value)} setters. */ - Entry getAndSet(Entry newEntry); + Entry getAndSet(@Nullable Entry newEntry); void putAll(Map map); @@ -368,16 +373,25 @@ final class Entry extends EntryChange implements Map.Entry, Entr */ static final byte ANY = 0; - /** If value is non-null, returns a new TagMap.Entry If value is null, returns null */ - public static final Entry create(String tag, Object value) { - // NOTE: From the static typing, it is possible that value is a primitive box, so need to call - // Any variant - - return (value == null) ? null : TagMap.Entry.newAnyEntry(tag, value); + /** + * Entry for {@code (tag, value)}, or null when {@code value} is null or an empty {@code + * CharSequence} -- checked by runtime type, so an empty String passed as {@code Object} skips + * the same as via the {@link #create(String, CharSequence)} overload. + */ + @Nullable + public static final Entry create(@Nonnull String tag, Object value) { + if (value == null) { + return null; + } + if (value instanceof CharSequence && ((CharSequence) value).length() == 0) { + return null; + } + return TagMap.Entry.newAnyEntry(tag, value); } /** If value is non-null, returns a new TagMap.Entry If value is null or empty, returns null */ - public static final Entry create(String tag, CharSequence value) { + @Nullable + public static final Entry create(@Nonnull String tag, CharSequence value) { // NOTE: From the static typing, we know that value is not a primitive box return (value == null || value.length() == 0) @@ -385,23 +399,23 @@ public static final Entry create(String tag, CharSequence value) { : TagMap.Entry.newObjectEntry(tag, value); } - public static final Entry create(String tag, boolean value) { + public static final Entry create(@Nonnull String tag, boolean value) { return TagMap.Entry.newBooleanEntry(tag, value); } - public static final Entry create(String tag, int value) { + public static final Entry create(@Nonnull String tag, int value) { return TagMap.Entry.newIntEntry(tag, value); } - public static final Entry create(String tag, long value) { + public static final Entry create(@Nonnull String tag, long value) { return TagMap.Entry.newLongEntry(tag, value); } - public static final Entry create(String tag, float value) { + public static final Entry create(@Nonnull String tag, float value) { return TagMap.Entry.newFloatEntry(tag, value); } - public static final Entry create(String tag, double value) { + public static final Entry create(@Nonnull String tag, double value) { return TagMap.Entry.newDoubleEntry(tag, value); } @@ -1357,6 +1371,9 @@ public Object put(String tag, Object value) { @Override public void set(TagMap.EntryReader newEntryReader) { + if (newEntryReader == null) { + return; + } this.getAndSet(newEntryReader.entry()); } @@ -1397,6 +1414,10 @@ public void set(String tag, double value) { @Override public Entry getAndSet(Entry newEntry) { + if (newEntry == null) { + return null; + } + this.checkWriteAccess(); Object[] thisBuckets = this.buckets; diff --git a/internal-api/src/test/java/datadog/trace/api/TagMapNullToleranceTest.java b/internal-api/src/test/java/datadog/trace/api/TagMapNullToleranceTest.java new file mode 100644 index 00000000000..b8764472847 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/TagMapNullToleranceTest.java @@ -0,0 +1,66 @@ +package datadog.trace.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** + * Pins the Entry-pathway null-tolerance contract: {@code create(...)} returns null for a null or + * empty value, and the {@code Entry} sinks ({@code set(Entry)} / {@code getAndSet(Entry)}) treat + * null as a no-op -- so a null/empty value flows through as "no tag" without any caller guarding. + */ +class TagMapNullToleranceTest { + + @Test + void createReturnsNullForNullOrEmptyValue() { + // CharSequence overload: null or empty -> null + assertNull(TagMap.Entry.create("k", (CharSequence) null)); + assertNull(TagMap.Entry.create("k", "")); + + // Object overload: null -> null, and an empty String typed as Object -> null (runtime check, + // so the convention holds regardless of the static type at the call site) + assertNull(TagMap.Entry.create("k", (Object) null)); + assertNull(TagMap.Entry.create("k", (Object) "")); + + // Non-empty values still produce an entry, whatever the static type + assertNotNull(TagMap.Entry.create("k", "v")); + assertNotNull(TagMap.Entry.create("k", (Object) "v")); + // Non-CharSequence Object has no notion of "empty" -> always an entry + assertNotNull(TagMap.Entry.create("k", (Object) Integer.valueOf(0))); + } + + @Test + void getAndSetToleratesNullEntry() { + TagMap map = TagMap.create(); + assertNull(map.getAndSet((TagMap.Entry) null), "null entry is a no-op returning null"); + assertEquals(0, map.size(), "no tag added"); + } + + @Test + void setToleratesNullReader() { + TagMap map = TagMap.create(); + map.set((TagMap.EntryReader) null); // must not throw + assertEquals(0, map.size(), "no tag added"); + } + + @Test + void nullOrEmptyFlowsThroughTheEntryPathwayAsNoTag() { + TagMap map = TagMap.create(); + + // The seamless case: create(...) -> set(Entry) with a null/empty value, no caller guard. + map.set(TagMap.Entry.create("empty", "")); + map.set(TagMap.Entry.create("emptyObj", (Object) "")); + map.set(TagMap.Entry.create("nul", (CharSequence) null)); + assertEquals(0, map.size(), "null/empty values leave no tags behind"); + assertFalse(map.containsKey("empty")); + + // A real value still lands. + map.set(TagMap.Entry.create("present", "v")); + assertEquals(1, map.size()); + assertTrue(map.containsKey("present")); + } +}