diff --git a/inferred-spans/src/main/java/co/elastic/otel/profiler/CallTree.java b/inferred-spans/src/main/java/co/elastic/otel/profiler/CallTree.java index 7d2d89a1a..6c3ef62f8 100644 --- a/inferred-spans/src/main/java/co/elastic/otel/profiler/CallTree.java +++ b/inferred-spans/src/main/java/co/elastic/otel/profiler/CallTree.java @@ -24,7 +24,6 @@ import co.elastic.otel.common.ElasticAttributes; import co.elastic.otel.common.util.HexUtils; import co.elastic.otel.profiler.collections.LongHashSet; -import co.elastic.otel.profiler.collections.LongList; import co.elastic.otel.profiler.pooling.ObjectPool; import co.elastic.otel.profiler.pooling.Recyclable; import io.opentelemetry.api.common.Attributes; @@ -84,9 +83,9 @@ public class CallTree implements Recyclable { private boolean isSpan; private int depth; - @Nullable private LongList childIds; + @Nullable private ChildList childIds; - @Nullable private LongList maybeChildIds; + @Nullable private ChildList maybeChildIds; public CallTree() {} @@ -504,7 +503,8 @@ protected Span asSpan( .setStartTimestamp( clock.toEpochNanos(parentContext.getClockAnchor(), this.start), TimeUnit.NANOSECONDS); - insertChildIdLinks(spanBuilder, Span.fromContext(parentOtelCtx).getSpanContext(), tempBuilder); + insertChildIdLinks( + spanBuilder, Span.fromContext(parentOtelCtx).getSpanContext(), parentContext, tempBuilder); // we're not interested in the very bottom of the stack which contains things like accepting and // handling connections @@ -524,20 +524,27 @@ protected Span asSpan( } private void insertChildIdLinks( - SpanBuilder span, SpanContext parentContext, StringBuilder tempBuilder) { + SpanBuilder span, + SpanContext parentContext, + TraceContext nonInferredParent, + StringBuilder tempBuilder) { if (childIds == null || childIds.isEmpty()) { return; } for (int i = 0; i < childIds.getSize(); i++) { - tempBuilder.setLength(0); - HexUtils.appendLongAsHex(childIds.get(i), tempBuilder); - SpanContext spanContext = - SpanContext.create( - parentContext.getTraceId(), - tempBuilder.toString(), - parentContext.getTraceFlags(), - parentContext.getTraceState()); - span.addLink(spanContext, CHILD_LINK_ATTRIBUTES); + // to avoid cycles, we only insert child-ids if the parent of the child is also + // the parent of the stack of inferred spans inserted + if (nonInferredParent.getSpanId() == childIds.getParentId(i)) { + tempBuilder.setLength(0); + HexUtils.appendLongAsHex(childIds.getId(i), tempBuilder); + SpanContext spanContext = + SpanContext.create( + parentContext.getTraceId(), + tempBuilder.toString(), + parentContext.getTraceFlags(), + parentContext.getTraceState()); + span.addLink(spanContext, CHILD_LINK_ATTRIBUTES); + } } } @@ -623,18 +630,18 @@ public void resetState() { * * @param id the child span id to add to this call tree element */ - public void addMaybeChildId(long id) { + public void addMaybeChildId(long id, long parentId) { if (maybeChildIds == null) { - maybeChildIds = new LongList(); + maybeChildIds = new ChildList(); } - maybeChildIds.add(id); + maybeChildIds.add(id, parentId); } - public void addChildId(long id) { + public void addChildId(long id, long parentId) { if (childIds == null) { - childIds = new LongList(); + childIds = new ChildList(); } - childIds.add(id); + childIds.add(id, parentId); } public boolean hasChildIds() { @@ -664,7 +671,11 @@ void giveChildIdsTo(CallTree giveTo) { void giveLastChildIdTo(CallTree giveTo) { if (childIds != null && !childIds.isEmpty()) { - giveTo.addChildId(childIds.remove(childIds.getSize() - 1)); + int size = childIds.getSize(); + long id = childIds.getId(size - 1); + long parentId = childIds.getParentId(size - 1); + giveTo.addChildId(id, parentId); + childIds.removeLast(); } } @@ -743,7 +754,7 @@ public void onActivation(byte[] active, long timestamp) { long spanId = TraceContext.getSpanId(active); activeSet.add(spanId); if (!isNestedActivation(topOfStack)) { - topOfStack.addMaybeChildId(spanId); + topOfStack.addMaybeChildId(spanId, TraceContext.getParentId(active)); } } } @@ -752,12 +763,12 @@ private boolean isNestedActivation(CallTree topOfStack) { return isAnyActive(topOfStack.childIds) || isAnyActive(topOfStack.maybeChildIds); } - private boolean isAnyActive(@Nullable LongList spanIds) { + private boolean isAnyActive(@Nullable ChildList spanIds) { if (spanIds == null) { return false; } for (int i = 0, size = spanIds.getSize(); i < size; i++) { - if (activeSet.contains(spanIds.get(i))) { + if (activeSet.contains(spanIds.getId(i))) { return true; } } diff --git a/inferred-spans/src/main/java/co/elastic/otel/profiler/ChildList.java b/inferred-spans/src/main/java/co/elastic/otel/profiler/ChildList.java new file mode 100644 index 000000000..7b7a62db9 --- /dev/null +++ b/inferred-spans/src/main/java/co/elastic/otel/profiler/ChildList.java @@ -0,0 +1,63 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.otel.profiler; + +import co.elastic.otel.profiler.collections.LongList; + +/** List for maintaining pairs of (spanId,parentIds) both represented as longs. */ +public class ChildList { + + // this list contains the (spanId,parentIds) flattened + private LongList idsWithParentIds = new LongList(); + + public void add(long id, long parentId) { + idsWithParentIds.add(id); + idsWithParentIds.add(parentId); + } + + public long getId(int index) { + return idsWithParentIds.get(index * 2); + } + + public long getParentId(int index) { + return idsWithParentIds.get(index * 2 + 1); + } + + public int getSize() { + return idsWithParentIds.getSize() / 2; + } + + public void addAll(ChildList other) { + idsWithParentIds.addAll(other.idsWithParentIds); + } + + public void clear() { + idsWithParentIds.clear(); + } + + public boolean isEmpty() { + return getSize() == 0; + } + + public void removeLast() { + int size = idsWithParentIds.getSize(); + idsWithParentIds.remove(size - 1); + idsWithParentIds.remove(size - 2); + } +} diff --git a/inferred-spans/src/main/java/co/elastic/otel/profiler/SamplingProfiler.java b/inferred-spans/src/main/java/co/elastic/otel/profiler/SamplingProfiler.java index cdd75cb40..89dea3716 100644 --- a/inferred-spans/src/main/java/co/elastic/otel/profiler/SamplingProfiler.java +++ b/inferred-spans/src/main/java/co/elastic/otel/profiler/SamplingProfiler.java @@ -862,15 +862,12 @@ private void set( @Nullable Span previousContext, long nanoTime, SpanAnchoredClock clock) { - TraceContext.serialize( - traceContext.getSpanContext(), clock.getAnchor(traceContext), traceContextBuffer); + TraceContext.serialize(traceContext, clock.getAnchor(traceContext), traceContextBuffer); this.threadId = threadId; this.activation = activation; if (previousContext != null) { TraceContext.serialize( - previousContext.getSpanContext(), - clock.getAnchor(previousContext), - previousContextBuffer); + previousContext, clock.getAnchor(previousContext), previousContextBuffer); rootContext = false; } else { rootContext = true; diff --git a/inferred-spans/src/main/java/co/elastic/otel/profiler/TraceContext.java b/inferred-spans/src/main/java/co/elastic/otel/profiler/TraceContext.java index aeb0e0a30..0c0b0ad31 100644 --- a/inferred-spans/src/main/java/co/elastic/otel/profiler/TraceContext.java +++ b/inferred-spans/src/main/java/co/elastic/otel/profiler/TraceContext.java @@ -25,6 +25,7 @@ import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.TraceFlags; import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.sdk.trace.ReadableSpan; import javax.annotation.Nullable; /** @@ -34,10 +35,13 @@ */ public class TraceContext implements Recyclable { - public static final int SERIALIZED_LENGTH = 16 + 8 + 1 + 8; + public static final int SERIALIZED_LENGTH = 16 + 8 + 1 + 1 + 8 + 8; private long traceIdLow; private long traceIdHigh; private long id; + + private boolean hasParentId; + private long parentId; private byte flags; private long clockAnchor; @@ -45,17 +49,24 @@ public class TraceContext implements Recyclable { public TraceContext() {} // For testing only - static TraceContext fromSpanContextWithZeroClockAnchor(SpanContext ctx) { + static TraceContext fromSpanContextWithZeroClockAnchor( + SpanContext ctx, @Nullable String parentSpanId) { TraceContext result = new TraceContext(); - result.filLFromSpanContext(ctx); + result.fillFromSpanContext(ctx, parentSpanId); result.clockAnchor = 0L; return result; } - private void filLFromSpanContext(SpanContext ctx) { + private void fillFromSpanContext(SpanContext ctx, @Nullable String parentSpanId) { id = HexUtils.hexToLong(ctx.getSpanId(), 0); traceIdHigh = HexUtils.hexToLong(ctx.getTraceId(), 0); traceIdLow = HexUtils.hexToLong(ctx.getTraceId(), 16); + if (parentSpanId != null) { + hasParentId = true; + parentId = HexUtils.hexToLong(parentSpanId, 0); + } else { + hasParentId = false; + } flags = ctx.getTraceFlags().asByte(); } @@ -73,6 +84,10 @@ public SpanContext toOtelSpanContext(StringBuilder temporaryBuilder) { traceIdStr, idStr, TraceFlags.fromByte(flags), TraceState.getDefault()); } + public long getSpanId() { + return id; + } + public boolean idEquals(@Nullable TraceContext o) { if (o == null) { return false; @@ -89,7 +104,17 @@ public void deserialize(byte[] serialized) { traceIdHigh = ByteUtils.getLong(serialized, 8); id = ByteUtils.getLong(serialized, 16); flags = serialized[24]; - clockAnchor = ByteUtils.getLong(serialized, 25); + hasParentId = serialized[25] != 0; + parentId = ByteUtils.getLong(serialized, 26); + clockAnchor = ByteUtils.getLong(serialized, 34); + } + + public static long getParentId(byte[] serializedTraceContext) { + boolean hasParent = serializedTraceContext[25] != 0; + if (!hasParent) { + return 0L; + } + return ByteUtils.getLong(serializedTraceContext, 26); } public boolean traceIdAndIdEquals(byte[] otherSerialized) { @@ -105,7 +130,13 @@ public boolean traceIdAndIdEquals(byte[] otherSerialized) { return id == otherId; } - public static void serialize(SpanContext ctx, long clockAnchor, byte[] buffer) { + public static void serialize(Span span, long clockAnchor, byte[] buffer) { + SpanContext ctx = span.getSpanContext(); + SpanContext parentSpanCtx = SpanContext.getInvalid(); + if (span instanceof ReadableSpan) { + parentSpanCtx = ((ReadableSpan) span).getParentSpanContext(); + } + long id = HexUtils.hexToLong(ctx.getSpanId(), 0); long traceIdHigh = HexUtils.hexToLong(ctx.getTraceId(), 0); long traceIdLow = HexUtils.hexToLong(ctx.getTraceId(), 16); @@ -114,7 +145,14 @@ public static void serialize(SpanContext ctx, long clockAnchor, byte[] buffer) { ByteUtils.putLong(buffer, 8, traceIdHigh); ByteUtils.putLong(buffer, 16, id); buffer[24] = flags; - ByteUtils.putLong(buffer, 25, clockAnchor); + if (parentSpanCtx.isValid()) { + buffer[25] = 1; + ByteUtils.putLong(buffer, 26, HexUtils.hexToLong(parentSpanCtx.getSpanId(), 0)); + } else { + buffer[25] = 0; + ByteUtils.putLong(buffer, 26, 0); + } + ByteUtils.putLong(buffer, 34, clockAnchor); } public void serialize(byte[] buffer) { @@ -122,7 +160,14 @@ public void serialize(byte[] buffer) { ByteUtils.putLong(buffer, 8, traceIdHigh); ByteUtils.putLong(buffer, 16, id); buffer[24] = flags; - ByteUtils.putLong(buffer, 25, clockAnchor); + if (hasParentId) { + buffer[25] = 1; + ByteUtils.putLong(buffer, 26, parentId); + } else { + buffer[25] = 0; + ByteUtils.putLong(buffer, 26, 0); + } + ByteUtils.putLong(buffer, 34, clockAnchor); } public byte[] serialize() { diff --git a/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeSpanifyTest.java b/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeSpanifyTest.java index 4259c4788..25fa3e7e1 100644 --- a/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeSpanifyTest.java +++ b/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeSpanifyTest.java @@ -107,7 +107,8 @@ void testCallTreeWithActiveSpan() { TraceContext rootContext = TraceContext.fromSpanContextWithZeroClockAnchor( SpanContext.create( - traceId, rootSpanId, TraceFlags.getSampled(), TraceState.getDefault())); + traceId, rootSpanId, TraceFlags.getSampled(), TraceState.getDefault()), + null); ObjectPool rootPool = ObjectPool.createRecyclable(2, CallTree.Root::new); ObjectPool childPool = ObjectPool.createRecyclable(2, CallTree::new); @@ -119,7 +120,8 @@ void testCallTreeWithActiveSpan() { TraceContext spanContext = TraceContext.fromSpanContextWithZeroClockAnchor( SpanContext.create( - traceId, childSpanId, TraceFlags.getSampled(), TraceState.getDefault())); + traceId, childSpanId, TraceFlags.getSampled(), TraceState.getDefault()), + rootSpanId); root.onActivation(spanContext.serialize(), TimeUnit.MILLISECONDS.toNanos(5)); root.addStackTrace( @@ -142,8 +144,6 @@ void testCallTreeWithActiveSpan() { 0); root.end(childPool, 0); - System.out.println(root); - assertThat(root.getCount()).isEqualTo(4); assertThat(root.getDurationUs()).isEqualTo(30_000); assertThat(root.getChildren()).hasSize(1); @@ -179,4 +179,97 @@ void testCallTreeWithActiveSpan() { assertThat(spans.get(1)).hasTraceId(traceId).hasParentSpanId(childSpanId); } } + + @Test + void testSpanWithInvertedActivation() { + FixedClock nanoClock = new FixedClock(); + + String traceId = "0af7651916cd43dd8448eb211c80319c"; + String rootSpanId = "77ad6b7169203331"; + TraceContext rootContext = + TraceContext.fromSpanContextWithZeroClockAnchor( + SpanContext.create( + traceId, rootSpanId, TraceFlags.getSampled(), TraceState.getDefault()), + null); + + String childSpanId = "11b2c3d4e5f64242"; + TraceContext childSpanContext = + TraceContext.fromSpanContextWithZeroClockAnchor( + SpanContext.create( + traceId, childSpanId, TraceFlags.getSampled(), TraceState.getDefault()), + rootSpanId); + + ObjectPool rootPool = ObjectPool.createRecyclable(2, CallTree.Root::new); + ObjectPool childPool = ObjectPool.createRecyclable(2, CallTree::new); + + CallTree.Root root = CallTree.createRoot(rootPool, childSpanContext.serialize(), 0); + root.addStackTrace(Collections.singletonList(StackFrame.of("A", "a")), 10_000, childPool, 0); + + root.onActivation(rootContext.serialize(), 20_000); + root.onDeactivation(rootContext.serialize(), childSpanContext.serialize(), 30_000); + + root.addStackTrace(Collections.singletonList(StackFrame.of("A", "a")), 40_000, childPool, 0); + root.end(childPool, 0); + + InMemorySpanExporter exporter = InMemorySpanExporter.create(); + OpenTelemetrySdkBuilder sdkBuilder = + OpenTelemetrySdk.builder() + .setTracerProvider( + SdkTracerProvider.builder() + .addSpanProcessor(SimpleSpanProcessor.create(exporter)) + .build()); + try (OpenTelemetrySdk outputSdk = sdkBuilder.build()) { + root.spanify(nanoClock, outputSdk.getTracer("dummy-tracer")); + + List spans = exporter.getFinishedSpanItems(); + assertThat(spans).hasSize(1); + assertThat(spans.get(0)).hasTraceId(traceId).hasParentSpanId(childSpanId); + // the inferred span should not have any span links because this + // span link would cause a cycle in the trace + assertThat(spans.get(0).getLinks()).isEmpty(); + } + } + + @Test + void testSpanWithNestedActivation() { + FixedClock nanoClock = new FixedClock(); + + String traceId = "0af7651916cd43dd8448eb211c80319c"; + String rootSpanId = "77ad6b7169203331"; + TraceContext rootContext = + TraceContext.fromSpanContextWithZeroClockAnchor( + SpanContext.create( + traceId, rootSpanId, TraceFlags.getSampled(), TraceState.getDefault()), + null); + + ObjectPool rootPool = ObjectPool.createRecyclable(2, CallTree.Root::new); + ObjectPool childPool = ObjectPool.createRecyclable(2, CallTree::new); + + CallTree.Root root = CallTree.createRoot(rootPool, rootContext.serialize(), 0); + root.addStackTrace(Collections.singletonList(StackFrame.of("A", "a")), 10_000, childPool, 0); + + root.onActivation(rootContext.serialize(), 20_000); + root.onDeactivation(rootContext.serialize(), rootContext.serialize(), 30_000); + + root.addStackTrace(Collections.singletonList(StackFrame.of("A", "a")), 40_000, childPool, 0); + root.end(childPool, 0); + + InMemorySpanExporter exporter = InMemorySpanExporter.create(); + OpenTelemetrySdkBuilder sdkBuilder = + OpenTelemetrySdk.builder() + .setTracerProvider( + SdkTracerProvider.builder() + .addSpanProcessor(SimpleSpanProcessor.create(exporter)) + .build()); + try (OpenTelemetrySdk outputSdk = sdkBuilder.build()) { + root.spanify(nanoClock, outputSdk.getTracer("dummy-tracer")); + + List spans = exporter.getFinishedSpanItems(); + assertThat(spans).hasSize(1); + assertThat(spans.get(0)).hasTraceId(traceId).hasParentSpanId(rootSpanId); + // the inferred span should not have any span links because this + // span link would cause a cycle in the trace + assertThat(spans.get(0).getLinks()).isEmpty(); + } + } } diff --git a/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeTest.java b/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeTest.java index 304828569..821feb957 100644 --- a/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeTest.java +++ b/inferred-spans/src/test/java/co/elastic/otel/profiler/CallTreeTest.java @@ -123,7 +123,7 @@ void testCallTree() { @Test void testGiveEmptyChildIdsTo() { CallTree rich = new CallTree(); - rich.addChildId(42); + rich.addChildId(42, 0L); CallTree robinHood = new CallTree(); CallTree poor = new CallTree();