diff --git a/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java b/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java index ac110ca2e15..3de19f68088 100644 --- a/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java +++ b/dd-trace-core/src/main/java/datadog/trace/civisibility/writer/ddintake/CiTestCycleMapperV1.java @@ -3,6 +3,7 @@ import static datadog.communication.http.OkHttpUtils.gzippedMsgpackRequestBodyOf; import static datadog.communication.http.OkHttpUtils.msgpackRequestBodyOf; import static datadog.json.JsonMapper.toJson; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import static datadog.trace.api.civisibility.CIConstants.MAX_META_STRING_VALUE_LENGTH; import static datadog.trace.util.Strings.truncate; @@ -333,7 +334,7 @@ public void accept(Metadata metadata) { int metaSize = metadata.getBaggage().size() + tags.size() - + (null == metadata.getHttpStatusCode() ? 0 : 1); + + (UNSET_STATUS == metadata.getHttpStatusCode() ? 0 : 1); int metricsSize = 0; for (Map.Entry tag : tags.entrySet()) { if (tag.getValue() instanceof Number) { @@ -359,9 +360,10 @@ public void accept(Metadata metadata) { writable.writeString(entry.getKey(), null); writable.writeString(truncate(entry.getValue(), MAX_META_STRING_VALUE_LENGTH), null); } - if (null != metadata.getHttpStatusCode()) { + if (UNSET_STATUS != metadata.getHttpStatusCode()) { writable.writeUTF8(HTTP_STATUS); - writable.writeUTF8(truncate(metadata.getHttpStatusCode(), MAX_META_STRING_VALUE_LENGTH)); + writable.writeUTF8( + truncate(metadata.getHttpStatusCodeString(), MAX_META_STRING_VALUE_LENGTH)); } for (Map.Entry entry : tags.entrySet()) { Object value = entry.getValue(); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/FileBasedPayloadDispatcher.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/FileBasedPayloadDispatcher.java index 810952292b8..0733d9a9a04 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/FileBasedPayloadDispatcher.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/FileBasedPayloadDispatcher.java @@ -1,6 +1,7 @@ package datadog.trace.common.writer; import static datadog.json.JsonMapper.toJson; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import static datadog.trace.api.civisibility.CIConstants.MAX_META_STRING_VALUE_LENGTH; import static datadog.trace.util.Strings.truncate; @@ -388,9 +389,11 @@ public void accept(Metadata metadata) { w.name(entry.getKey()).value(truncate(entry.getValue(), MAX_META_STRING_VALUE_LENGTH)); } } - if (metadata.getHttpStatusCode() != null) { + if (metadata.getHttpStatusCode() != UNSET_STATUS) { w.name(Tags.HTTP_STATUS) - .value(truncate(metadata.getHttpStatusCode().toString(), MAX_META_STRING_VALUE_LENGTH)); + .value( + truncate( + metadata.getHttpStatusCodeString().toString(), MAX_META_STRING_VALUE_LENGTH)); } for (Map.Entry entry : tags.entrySet()) { Object value = entry.getValue(); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java index 0473010d3d1..58fd278cf43 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java @@ -1,6 +1,7 @@ package datadog.trace.common.writer.ddagent; import static datadog.communication.http.OkHttpUtils.msgpackRequestBodyOf; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import datadog.communication.serialization.Codec; import datadog.communication.serialization.GenerationalUtf8Cache; @@ -103,7 +104,7 @@ public void accept(Metadata metadata) { int metaSize = metadata.getBaggage().size() + tags.size() - + (null == metadata.getHttpStatusCode() ? 0 : 1) + + (UNSET_STATUS == metadata.getHttpStatusCode() ? 0 : 1) + (null == metadata.getOrigin() ? 0 : 1) + (null == processTags ? 0 : 1) + 1; @@ -193,9 +194,9 @@ public void accept(Metadata metadata) { } writable.writeUTF8(THREAD_NAME); writable.writeUTF8(metadata.getThreadName()); - if (null != metadata.getHttpStatusCode()) { + if (UNSET_STATUS != metadata.getHttpStatusCode()) { writable.writeUTF8(HTTP_STATUS); - writable.writeUTF8(metadata.getHttpStatusCode()); + writable.writeUTF8(metadata.getHttpStatusCodeString()); } if (null != metadata.getOrigin()) { writable.writeUTF8(ORIGIN_KEY); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java index 0e8644bdee9..60f221402d5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java @@ -1,6 +1,7 @@ package datadog.trace.common.writer.ddagent; import static datadog.communication.http.OkHttpUtils.msgpackRequestBodyOf; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import datadog.communication.serialization.GrowableBuffer; import datadog.communication.serialization.Mapper; @@ -225,7 +226,7 @@ public void accept(Metadata metadata) { int metaSize = metadata.getBaggage().size() + tags.size() - + (null == metadata.getHttpStatusCode() ? 0 : 1) + + (UNSET_STATUS == metadata.getHttpStatusCode() ? 0 : 1) + (null == metadata.getOrigin() ? 0 : 1) + (null == processTags ? 0 : 1) + 1; @@ -259,9 +260,9 @@ public void accept(Metadata metadata) { } writeDictionaryEncoded(writable, THREAD_NAME); writeDictionaryEncoded(writable, metadata.getThreadName()); - if (null != metadata.getHttpStatusCode()) { + if (UNSET_STATUS != metadata.getHttpStatusCode()) { writeDictionaryEncoded(writable, HTTP_STATUS); - writeDictionaryEncoded(writable, metadata.getHttpStatusCode()); + writeDictionaryEncoded(writable, metadata.getHttpStatusCodeString()); } if (null != metadata.getOrigin()) { writeDictionaryEncoded(writable, ORIGIN_KEY); diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV1.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV1.java index 8e43e43fa67..6a0ac4d7b3b 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV1.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV1.java @@ -1,6 +1,7 @@ package datadog.trace.common.writer.ddagent; import static datadog.communication.http.OkHttpUtils.msgpackRequestBodyOf; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; @@ -378,8 +379,9 @@ private void encodeSpanAttributes( Writable writable, int fieldId, Metadata meta, Map metaStruct) { TagMap tags = meta.getTags(); Map baggage = meta.getBaggage(); + // Kept as a String: writeAttribute below logs a debug line for any non-String value. String httpStatusCode = - meta.getHttpStatusCode() == null ? null : meta.getHttpStatusCode().toString(); + meta.getHttpStatusCode() == UNSET_STATUS ? null : meta.getHttpStatusCodeString().toString(); boolean writeHttpStatus = httpStatusCode != null && tags.getString(HTTP_STATUS) == null; boolean writeTopLevel = meta.topLevel(); int tagCount = 0; diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index adf4cd66156..8520a222424 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -2,7 +2,6 @@ import static datadog.trace.api.DDTags.PARENT_ID; import static datadog.trace.api.DDTags.SPAN_LINKS; -import static datadog.trace.api.cache.RadixTreeCache.HTTP_STATUSES; import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.UNSET; import static datadog.trace.bootstrap.instrumentation.api.ServiceNameSources.MANUAL; @@ -1356,7 +1355,7 @@ void processTagsAndBaggage( samplingPriority != PrioritySampling.UNSET ? samplingPriority : getSamplingPriority(), measured, topLevel, - httpStatusCode == 0 ? null : HTTP_STATUSES.get(httpStatusCode), + httpStatusCode, // Get origin from rootSpan.context getOrigin(), longRunningVersion, diff --git a/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java b/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java index b7f9a6b2cc2..d957358d73a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/Metadata.java @@ -4,6 +4,7 @@ import static java.util.Collections.emptyList; import datadog.trace.api.TagMap; +import datadog.trace.api.cache.RadixTreeCache; import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import java.util.List; @@ -12,7 +13,7 @@ public final class Metadata { private final long threadId; private final UTF8BytesString threadName; - private final UTF8BytesString httpStatusCode; + private final int httpStatusCode; private final TagMap tags; private final Map baggage; @@ -32,7 +33,7 @@ public Metadata( int samplingPriority, boolean measured, boolean topLevel, - UTF8BytesString httpStatusCode, + int httpStatusCode, CharSequence origin, int longRunningVersion, UTF8BytesString processTags, @@ -51,10 +52,31 @@ public Metadata( this.spanLinks = spanLinks == null ? emptyList() : spanLinks; } - public UTF8BytesString getHttpStatusCode() { + /** + * The intercepted HTTP status, or {@link RadixTreeCache#UNSET_STATUS} when the span carries none. + * + *

Held as an int rather than as its rendering, so a serializer that encodes the status + * numerically -- OTLP, whose semantic conventions type it as an integer -- never pays for a + * string it will not send, and a serializer that needs the string asks for it explicitly. + */ + public int getHttpStatusCode() { return httpStatusCode; } + /** + * The intercepted HTTP status rendered for the string-typed protocols (the Datadog msgpack + * payloads and the CI Visibility intake), or null when the span carries none. + * + *

Backed by {@link RadixTreeCache#HTTP_STATUSES}, so a repeated status costs a lookup rather + * than an allocation. Call it once per span and hold the result: nothing memoizes it here, since + * a Metadata is consumed by exactly one serializer. + */ + public UTF8BytesString getHttpStatusCodeString() { + return httpStatusCode == RadixTreeCache.UNSET_STATUS + ? null + : RadixTreeCache.HTTP_STATUSES.get(httpStatusCode); + } + public CharSequence getOrigin() { return origin; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java index d9c5e9c3d90..a56498d0a23 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJson.java @@ -1,5 +1,6 @@ package datadog.trace.core.otlp.trace; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DD_MEASURED; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DD_PARTIAL_VERSION; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DD_TOP_LEVEL; @@ -207,8 +208,8 @@ public void accept(Metadata metadata) { writeSpanTag(writer, THREAD_ID, metadata.getThreadId()); writeSpanTag(writer, THREAD_NAME, metadata.getThreadName()); - if (metadata.getHttpStatusCode() != null) { - writeSpanTag(writer, HTTP_STATUS, metadata.getHttpStatusCode()); + if (metadata.getHttpStatusCode() != UNSET_STATUS) { + writeSpanTag(writer, HTTP_STATUS, metadata.getHttpStatusCodeString()); } if (metadata.getOrigin() != null) { writeSpanTag(writer, ORIGIN_KEY, metadata.getOrigin()); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java index f97d05c388d..258cfb73669 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProto.java @@ -1,5 +1,6 @@ package datadog.trace.core.otlp.trace; +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DD_MEASURED; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DD_PARTIAL_VERSION; import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DD_TOP_LEVEL; @@ -284,8 +285,8 @@ public void accept(Metadata metadata) { writeSpanTag(buf, THREAD_ID, metadata.getThreadId()); writeSpanTag(buf, THREAD_NAME, metadata.getThreadName()); - if (metadata.getHttpStatusCode() != null) { - writeSpanTag(buf, HTTP_STATUS, metadata.getHttpStatusCode()); + if (metadata.getHttpStatusCode() != UNSET_STATUS) { + writeSpanTag(buf, HTTP_STATUS, metadata.getHttpStatusCodeString()); } if (metadata.getOrigin() != null) { writeSpanTag(buf, ORIGIN_KEY, metadata.getOrigin()); diff --git a/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java b/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java index 2040f685fdc..25c26bcd10a 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/writer/FileBasedPayloadDispatcherTest.java @@ -309,7 +309,7 @@ private static CoreSpan mockSpan(CharSequence type, Map tags) 0, false, false, - null, + 0, null, 0, null, diff --git a/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java b/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java index c8fad4ab0ee..e777235fef3 100644 --- a/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java +++ b/dd-trace-core/src/test/java/datadog/trace/common/writer/TraceGenerator.java @@ -238,7 +238,7 @@ public PojoSpan( samplingPriority, measured, isTopLevel(), - statusCode == 0 ? null : UTF8BytesString.create(Integer.toString(statusCode)), + statusCode, origin, 0, ProcessTags.getTagsForSerialization(), diff --git a/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java b/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java new file mode 100644 index 00000000000..2815cac6730 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/MetadataTest.java @@ -0,0 +1,67 @@ +package datadog.trace.core; + +import static datadog.trace.api.cache.RadixTreeCache.UNSET_STATUS; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import datadog.trace.api.TagMap; +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * The HTTP status is carried as an int and rendered only on demand, so that a serializer encoding + * it numerically never pays for a string it will not send. + */ +class MetadataTest { + + @ParameterizedTest + @ValueSource(ints = {200, 201, 404, 500, 599}) + void rendersTheStatusOnlyWhenAsked(int status) { + Metadata metadata = metadataWithStatus(status); + + assertEquals(status, metadata.getHttpStatusCode()); + assertEquals(Integer.toString(status), metadata.getHttpStatusCodeString().toString()); + } + + @Test + void reportsNoStatusAsUnsetRatherThanZeroString() { + Metadata metadata = metadataWithStatus(UNSET_STATUS); + + assertEquals(UNSET_STATUS, metadata.getHttpStatusCode()); + assertNull( + metadata.getHttpStatusCodeString(), + "an absent status must not render as \"0\": the string protocols write the key only when" + + " the span carries a status"); + } + + @Test + void reusesTheRenderedStatusAcrossSpans() { + // The point of routing through RadixTreeCache rather than rendering per span: two spans with + // the same status share one UTF8BytesString instead of allocating one apiece. + UTF8BytesString first = metadataWithStatus(404).getHttpStatusCodeString(); + UTF8BytesString second = metadataWithStatus(404).getHttpStatusCodeString(); + + assertSame(first, second); + } + + private static Metadata metadataWithStatus(int status) { + return new Metadata( + Thread.currentThread().getId(), + UTF8BytesString.create("main"), + TagMap.fromMap(emptyMap()), + emptyMap(), + 0, + false, + false, + status, + null, + 0, + null, + emptyList()); + } +} diff --git a/dd-trace-core/src/traceAgentTest/groovy/TraceGenerator.groovy b/dd-trace-core/src/traceAgentTest/groovy/TraceGenerator.groovy index a0c81cbd0cb..bf6623b53d2 100644 --- a/dd-trace-core/src/traceAgentTest/groovy/TraceGenerator.groovy +++ b/dd-trace-core/src/traceAgentTest/groovy/TraceGenerator.groovy @@ -161,7 +161,7 @@ class TraceGenerator { this.type = type this.measured = measured this.metadata = new Metadata(currentThread().getId(), - UTF8BytesString.create(currentThread().getName()), fromMap(tags), baggage, UNSET, measured, topLevel, null, null, 0, + UTF8BytesString.create(currentThread().getName()), fromMap(tags), baggage, UNSET, measured, topLevel, 0, null, 0, tagsForSerialization, emptyList()) }