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

Avoid use of trace context static methods in favor of Tracer-defined behavior #3082

Merged
merged 13 commits into from
Apr 17, 2023
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 @@ -64,6 +64,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ScheduledThreadPoolExecutor;
Expand Down Expand Up @@ -916,4 +917,9 @@ public <T extends co.elastic.apm.agent.tracer.Tracer> T require(Class<T> type) {
}
return cast;
}

@Override
public Set<String> getTraceHeaderNames() {
return TraceContext.TRACE_TEXTUAL_HEADERS;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@
import java.util.concurrent.atomic.AtomicLong;

public abstract class AbstractSpan<T extends AbstractSpan<T>> implements Recyclable, ElasticContext<T>, co.elastic.apm.agent.tracer.AbstractSpan<T> {
public static final int PRIO_USER_SUPPLIED = 1000;
public static final int PRIO_HIGH_LEVEL_FRAMEWORK = 100;
public static final int PRIO_METHOD_SIGNATURE = 100;
public static final int PRIO_LOW_LEVEL_FRAMEWORK = 10;
public static final int PRIO_DEFAULT = 0;
private static final Logger logger = LoggerFactory.getLogger(AbstractSpan.class);
private static final Logger oneTimeDuplicatedEndLogger = LoggerUtils.logOnce(logger);
private static final Logger oneTimeMaxSpanLinksLogger = LoggerUtils.logOnce(logger);
Expand All @@ -69,7 +64,7 @@ public abstract class AbstractSpan<T extends AbstractSpan<T>> implements Recycla
private ChildDurationTimer childDurations = new ChildDurationTimer();
protected AtomicInteger references = new AtomicInteger();
protected volatile boolean finished = true;
private int namePriority = PRIO_DEFAULT;
private int namePriority = PRIORITY_DEFAULT;
private boolean discardRequested = false;
/**
* Flag to mark a span as representing an exit event
Expand Down Expand Up @@ -272,7 +267,7 @@ public StringBuilder getAndOverrideName(int namePriority, boolean overrideIfSame
* @param methodName the method that should be part of this span's name
*/
public void updateName(Class<?> clazz, String methodName) {
StringBuilder spanName = getAndOverrideName(PRIO_DEFAULT);
StringBuilder spanName = getAndOverrideName(PRIORITY_DEFAULT);
if (spanName != null) {
String className = clazz.getName();
spanName.append(className, className.lastIndexOf('.') + 1, className.length());
Expand All @@ -291,7 +286,7 @@ public String getNameAsString() {

@Override
public T appendToName(CharSequence cs) {
return appendToName(cs, PRIO_DEFAULT);
return appendToName(cs, PRIORITY_DEFAULT);
}

@Override
Expand All @@ -310,7 +305,7 @@ public T appendToName(CharSequence cs, int priority, int startIndex, int endInde

@Override
public T withName(@Nullable String name) {
return withName(name, PRIO_DEFAULT);
return withName(name, PRIORITY_DEFAULT);
}

@Override
Expand Down Expand Up @@ -451,7 +446,7 @@ public void resetState() {
traceContext.resetState();
childDurations.resetState();
references.set(0);
namePriority = PRIO_DEFAULT;
namePriority = PRIORITY_DEFAULT;
discardRequested = false;
isExit = false;
childIds = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@

import javax.annotation.Nullable;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;

/**
* This is an implementation of the
Expand Down Expand Up @@ -82,7 +85,6 @@ public class TraceContext implements Recyclable, co.elastic.apm.agent.tracer.Tra
private static final int TEXT_HEADER_PARENT_ID_OFFSET = 36;
private static final int TEXT_HEADER_FLAGS_OFFSET = 53;

public static final String TRACE_PARENT_BINARY_HEADER_NAME = "elasticapmtraceparent";
public static final int BINARY_FORMAT_EXPECTED_LENGTH = 29;
private static final byte BINARY_FORMAT_CURRENT_VERSION = (byte) 0b0000_0000;
// one byte for the trace-id field id (0x00), followed by 16 bytes of the actual ID
Expand All @@ -98,6 +100,16 @@ public class TraceContext implements Recyclable, co.elastic.apm.agent.tracer.Tra

private static final Double SAMPLE_RATE_ZERO = 0d;

public static final Set<String> TRACE_TEXTUAL_HEADERS;

static {
Set<String> traceParentTextualHeaders = new HashSet<>();
traceParentTextualHeaders.add(ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME);
traceParentTextualHeaders.add(W3C_TRACE_PARENT_TEXTUAL_HEADER_NAME);
traceParentTextualHeaders.add(TRACESTATE_HEADER_NAME);
TRACE_TEXTUAL_HEADERS = Collections.unmodifiableSet(traceParentTextualHeaders);
}

private static final ChildContextCreator<TraceContext> FROM_PARENT_CONTEXT = new ChildContextCreator<TraceContext>() {
@Override
public boolean asChildOf(TraceContext child, TraceContext parent) {
Expand Down Expand Up @@ -157,7 +169,7 @@ public boolean asChildOf(TraceContext child, @Nullable Object carrier, HeaderGet
if (carrier == null) {
return false;
}
byte[] traceparent = traceContextHeaderGetter.getFirstHeader(TRACE_PARENT_BINARY_HEADER_NAME, carrier);
byte[] traceparent = traceContextHeaderGetter.getFirstHeader(ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME, carrier);
if (traceparent != null) {
return child.asChildOf(traceparent);
}
Expand All @@ -184,14 +196,14 @@ public boolean asChildOf(TraceContext child, Object ignore) {


public static <C> boolean containsTraceContextTextHeaders(C carrier, TextHeaderGetter<C> headerGetter) {
// We assume that this header is always present if we found any of the other headers.
return headerGetter.getFirstHeader(W3C_TRACE_PARENT_TEXTUAL_HEADER_NAME, carrier) != null;
}

public static <C> void removeTraceContextHeaders(C carrier, HeaderRemover<C> headerRemover) {
headerRemover.remove(W3C_TRACE_PARENT_TEXTUAL_HEADER_NAME, carrier);
headerRemover.remove(ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME, carrier);
headerRemover.remove(TRACESTATE_HEADER_NAME, carrier);
headerRemover.remove(TRACE_PARENT_BINARY_HEADER_NAME, carrier);
}

public static <S, D> void copyTraceContextTextHeaders(S source, TextHeaderGetter<S> headerGetter, D destination, TextHeaderSetter<D> headerSetter) {
Expand Down Expand Up @@ -576,15 +588,15 @@ <C> boolean propagateTraceContext(C carrier, BinaryHeaderSetter<C> headerSetter)
logger.debug("Outgoing TraceContext header injection is disabled");
return false;
}
byte[] buffer = headerSetter.getFixedLengthByteArray(TRACE_PARENT_BINARY_HEADER_NAME, BINARY_FORMAT_EXPECTED_LENGTH);
byte[] buffer = headerSetter.getFixedLengthByteArray(ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME, BINARY_FORMAT_EXPECTED_LENGTH);
if (buffer == null || buffer.length != BINARY_FORMAT_EXPECTED_LENGTH) {
logger.warn("Header setter {} failed to provide a byte buffer with the proper length. Allocating a buffer for each header.",
headerSetter.getClass().getName());
buffer = new byte[BINARY_FORMAT_EXPECTED_LENGTH];
}
boolean headerBufferFilled = fillOutgoingTraceParentBinaryHeader(buffer);
if (headerBufferFilled) {
headerSetter.setHeader(TRACE_PARENT_BINARY_HEADER_NAME, buffer, carrier);
headerSetter.setHeader(ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME, buffer, carrier);
}
return headerBufferFilled;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ protected Labels.Mutable initialValue() {
}
};

public static final String TYPE_REQUEST = "request";

/**
* Context
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,6 @@ void testTraceContextTextHeadersRemoval() {
assertThat(headerMap.get(TraceContext.TRACESTATE_HEADER_NAME)).isNull();
}

@Test
void testTraceContextBinaryHeadersRemoval() {
Map<String, byte[]> headerMap = new HashMap<>();
headerMap.put(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME, "00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01".getBytes(StandardCharsets.UTF_8));
TraceContext.removeTraceContextHeaders(headerMap, BinaryHeaderMapAccessor.INSTANCE);
assertThat(headerMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME)).isNull();
}

@Test
void testTraceContextHeadersCopy() {
Map<String, String> original = new HashMap<>();
Expand Down Expand Up @@ -287,9 +279,9 @@ void testBinaryHeaderCaching() {
assertThat(TraceContext.<Map<String, String>>getFromTraceContextTextHeaders().asChildOf(child, headerMap, TextHeaderMapAccessor.INSTANCE)).isTrue();
HashMap<String, byte[]> binaryHeaderMap = new HashMap<>();
assertThat(child.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE)).isTrue();
byte[] outgoingHeader = binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME);
byte[] outgoingHeader = binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME);
assertThat(child.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE)).isTrue();
assertThat(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME)).isSameAs(outgoingHeader);
assertThat(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME)).isSameAs(outgoingHeader);
}

@Test
Expand All @@ -310,9 +302,9 @@ public void setHeader(String headerName, byte[] headerValue, Map<String, byte[]>
};
HashMap<String, byte[]> binaryHeaderMap = new HashMap<>();
assertThat(child.propagateTraceContext(binaryHeaderMap, headerSetter)).isTrue();
byte[] outgoingHeader = binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME);
byte[] outgoingHeader = binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME);
assertThat(child.propagateTraceContext(binaryHeaderMap, headerSetter)).isTrue();
assertThat(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME)).isNotSameAs(outgoingHeader);
assertThat(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME)).isNotSameAs(outgoingHeader);
}

private void verifyTraceContextContents(String traceContext, String expectedTraceId, String expectedParentId,
Expand Down Expand Up @@ -349,7 +341,7 @@ void outgoingHeader() {
"0af7651916cd43dd8448eb211c80319c", parentId, "00", "03");
Map<String, byte[]> headerMap = new HashMap<>();
assertThat(traceContext.propagateTraceContext(headerMap, BinaryHeaderMapAccessor.INSTANCE)).isTrue();
verifyTraceContextContents(headerMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME),
verifyTraceContextContents(headerMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME),
"0af7651916cd43dd8448eb211c80319c", parentId, (byte) 0x00, (byte) 0x03);
}

Expand All @@ -364,7 +356,7 @@ void outgoingHeaderRootSpan() {
traceContext.getId().toString(), "00", "01");
Map<String, byte[]> headerMap = new HashMap<>();
assertThat(traceContext.propagateTraceContext(headerMap, BinaryHeaderMapAccessor.INSTANCE)).isTrue();
verifyTraceContextContents(headerMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME), traceContext.getTraceId().toString(),
verifyTraceContextContents(headerMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME), traceContext.getTraceId().toString(),
traceContext.getId().toString(), (byte) 0x00, (byte) 0x01);
}

Expand Down Expand Up @@ -398,7 +390,7 @@ void testResetOutgoingBinaryHeader() {
final TraceContext traceContext = TraceContext.with64BitId(tracer);
Map<String, byte[]> headerMap = new HashMap<>();
assertThat(traceContext.propagateTraceContext(headerMap, BinaryHeaderMapAccessor.INSTANCE)).isTrue();
byte[] outgoingByteHeader = headerMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME);
byte[] outgoingByteHeader = headerMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME);
byte[] tmp = new byte[outgoingByteHeader.length];
System.arraycopy(outgoingByteHeader, 0, tmp, 0, outgoingByteHeader.length);
traceContext.asChildOf("00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-00");
Expand Down Expand Up @@ -429,20 +421,20 @@ void testCopyFrom() {
assertThat(first.getOutgoingTraceParentTextHeader()).isNotEqualTo(second.getOutgoingTraceParentTextHeader());
Map<String, byte[]> binaryHeaderMap = new HashMap<>();
first.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
byte[] outgoingHeader = binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME);
byte[] outgoingHeader = binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME);
// We must copy because of the byte array caching in BinaryHeaderMapAccessor
byte[] firstOutgoingHeader = new byte[outgoingHeader.length];
System.arraycopy(outgoingHeader, 0, firstOutgoingHeader, 0, outgoingHeader.length);
second.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
assertThat(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME)).isNotEqualTo(firstOutgoingHeader);
assertThat(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME)).isNotEqualTo(firstOutgoingHeader);

second.copyFrom(first);
assertThat(first.getTraceId()).isEqualTo(second.getTraceId());
assertThat(first.getParentId()).isEqualTo(second.getParentId());
assertThat(first.isSampled()).isEqualTo(second.isSampled());
assertThat(first.getOutgoingTraceParentTextHeader().toString()).isEqualTo(second.getOutgoingTraceParentTextHeader().toString());
second.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
assertThat(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME)).isEqualTo(firstOutgoingHeader);
assertThat(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME)).isEqualTo(firstOutgoingHeader);
}

@Test
Expand Down Expand Up @@ -593,7 +585,7 @@ void testPropagateTransactionIdForUnsampledSpan() {
childContext.getTraceId().toString(), rootContext.getId().toString(), "00", "00");
Map<String, byte[]> binaryHeaderMap = new HashMap<>();
childContext.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
verifyTraceContextContents(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME),
verifyTraceContextContents(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME),
childContext.getTraceId().toString(), rootContext.getId().toString(), (byte) 0x00, (byte) 0x00);
}

Expand All @@ -609,7 +601,7 @@ void testPropagateSpanIdForSampledSpan() {
childContext.getTraceId().toString(), childContext.getId().toString(), "00", "01");
Map<String, byte[]> binaryHeaderMap = new HashMap<>();
childContext.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
verifyTraceContextContents(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME),
verifyTraceContextContents(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME),
childContext.getTraceId().toString(), childContext.getId().toString(), (byte) 0x00, (byte) 0x01);
}

Expand Down Expand Up @@ -705,7 +697,7 @@ private void assertValid(byte[] binaryHeader) {
assertThat(traceContext.asChildOf(binaryHeader)).isTrue();
Map<String, byte[]> binaryHeaderMap = new HashMap<>();
traceContext.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
verifyTraceContextContents(binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME),
verifyTraceContextContents(binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME),
traceContext.getTraceId().toString(), traceContext.getId().toString(), (byte) 0x00, binaryHeader[28]);
}

Expand All @@ -714,7 +706,7 @@ private byte[] convertToBinary(String textHeader) {
traceContext.asChildOf(textHeader);
Map<String, byte[]> binaryHeaderMap = new HashMap<>();
traceContext.propagateTraceContext(binaryHeaderMap, BinaryHeaderMapAccessor.INSTANCE);
byte[] binaryHeader = binaryHeaderMap.get(TraceContext.TRACE_PARENT_BINARY_HEADER_NAME);
byte[] binaryHeader = binaryHeaderMap.get(TraceContext.ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME);
// replace the version and parent ID
HexUtils.decode(textHeader, 0, 2, binaryHeader, 0);
HexUtils.decode(textHeader, 36, 16, binaryHeader, 19);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@

import co.elastic.apm.agent.bci.TracerAwareInstrumentation;
import co.elastic.apm.agent.httpclient.HttpClientHelper;
import co.elastic.apm.agent.impl.transaction.TraceContext;
import co.elastic.apm.agent.sdk.logging.Logger;
import co.elastic.apm.agent.sdk.logging.LoggerFactory;
import co.elastic.apm.agent.tracer.AbstractSpan;
import co.elastic.apm.agent.tracer.Outcome;
import co.elastic.apm.agent.tracer.Span;
import co.elastic.apm.agent.tracer.dispatch.HeaderUtils;
import co.elastic.apm.agent.util.LoggerUtils;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
Expand Down Expand Up @@ -121,10 +121,10 @@ public static Object onEnter(@Advice.Argument(0) HttpMethod httpMethod,
span.activate();
}

if (!TraceContext.containsTraceContextTextHeaders(httpMethod, HttpClient3RequestHeaderAccessor.INSTANCE)) {
if (!HeaderUtils.containsAny(tracer.getTraceHeaderNames(), httpMethod, HttpClient3RequestHeaderAccessor.INSTANCE)) {
if (span != null) {
span.propagateTraceContext(httpMethod, HttpClient3RequestHeaderAccessor.INSTANCE);
} else if (!TraceContext.containsTraceContextTextHeaders(httpMethod, HttpClient3RequestHeaderAccessor.INSTANCE)) {
} else if (!HeaderUtils.containsAny(tracer.getTraceHeaderNames(), httpMethod, HttpClient3RequestHeaderAccessor.INSTANCE)) {
// re-adds the header on redirects
parent.propagateTraceContext(httpMethod, HttpClient3RequestHeaderAccessor.INSTANCE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
package co.elastic.apm.agent.httpclient.v4;

import co.elastic.apm.agent.httpclient.v4.helper.RequestHeaderAccessor;
import co.elastic.apm.agent.impl.transaction.TraceContext;
import co.elastic.apm.agent.tracer.dispatch.HeaderUtils;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.NamedElement;
import net.bytebuddy.description.method.MethodDescription;
Expand Down Expand Up @@ -49,8 +49,8 @@ public static void onAfterExecute(@Advice.Argument(value = 0) HttpRequest origin
return;
}
// org.apache.http.HttpMessage#containsHeader implementations do not allocate iterator since 4.0.1
if (TraceContext.containsTraceContextTextHeaders(original, RequestHeaderAccessor.INSTANCE) && !TraceContext.containsTraceContextTextHeaders(redirect, RequestHeaderAccessor.INSTANCE)) {
TraceContext.copyTraceContextTextHeaders(original, RequestHeaderAccessor.INSTANCE, redirect, RequestHeaderAccessor.INSTANCE);
if (HeaderUtils.containsAny(tracer.getTraceHeaderNames(), original, RequestHeaderAccessor.INSTANCE) && !HeaderUtils.containsAny(tracer.getTraceHeaderNames(), redirect, RequestHeaderAccessor.INSTANCE)) {
HeaderUtils.copy(tracer.getTraceHeaderNames(), original, RequestHeaderAccessor.INSTANCE, redirect, RequestHeaderAccessor.INSTANCE);
}
}
}
Expand Down
Loading