Fix parent/child issues with inferred spans (#1117)

Author: felixbarny

CHANGELOG.asciidoc

@@ -39,6 +39,7 @@ This is not a breaking change, so former
   When set together with one of the more specific thresholds - `trace_methods_duration_threshold` or `profiling_inferred_spans_min_duration`,
   the higher threshold will determine which spans will be discarded.
 * Automatically instrument quartz jobs from the quartz-jobs artifact {pull}1170[#1170]
+* Perform re-parenting of regular spans to be a child of profiler-inferred spans. Requires APM Server and Kibana 7.8.0. {pull}1117[#1117]
 
 [float]
 ===== Bug fixes

apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/ProfilerBenchmark.java

@@ -62,7 +62,6 @@ public void setUp() throws Exception {
             new SystemNanoClock(),
             new File(getClass().getClassLoader().getResource("apm-activation-events.bin").toURI()),
             new File(getClass().getClassLoader().getResource("apm-traces.jfr").toURI()));
-        samplingProfiler.skipToEndOfActivationEventsFile();
     }
 
     @TearDown
@@ -72,6 +71,7 @@ public void tearDownProfilerBenchmark() throws Exception {
 
     @Benchmark
     public void processTraces() throws IOException {
+        samplingProfiler.skipToEndOfActivationEventsFile();
         samplingProfiler.processTraces();
         samplingProfiler.clearProfiledThreads();
     }

apm-agent-core/src/main/java/co/elastic/apm/agent/collections/LongList.java

@@ -0,0 +1,129 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2020 Elastic and contributors
+ * %%
+ * 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.
+ * #L%
+ */
+package co.elastic.apm.agent.collections;
+
+import java.util.Arrays;
+
+public class LongList {
+    private static final int DEFAULT_CAPACITY = 16;
+    private long[] longs;
+    private int size;
+
+    public LongList() {
+        this(DEFAULT_CAPACITY);
+    }
+
+    public LongList(int initialCapacity) {
+        longs = new long[initialCapacity];
+    }
+
+    public static LongList of(long... values) {
+        LongList list = new LongList(values.length);
+        for (long value : values) {
+            list.add(value);
+        }
+        return list;
+    }
+
+    public void add(long l) {
+        ensureCapacity(size + 1);
+        longs[size++] = l;
+    }
+
+    public void addAll(LongList other) {
+        ensureCapacity(size + other.size);
+        System.arraycopy(other.longs, 0, longs, size, other.size);
+        size += other.size;
+    }
+
+    private void ensureCapacity(int size) {
+        if (longs.length < size) {
+            longs = Arrays.copyOf(longs, longs.length * 2);
+        }
+    }
+
+    public int getSize() {
+        return size;
+    }
+
+    public long get(int i) {
+        if (i >= size) {
+            throw new IndexOutOfBoundsException();
+        }
+        return longs[i];
+    }
+
+    public boolean contains(long l) {
+        for (int i = 0; i < size; i++) {
+            if (longs[i] == l) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public boolean remove(long l) {
+        for (int i = size - 1; i >= 0; i--) {
+            if (longs[i] == l) {
+                remove(i);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public long remove(int i) {
+        long previousValue = get(i);
+        size--;
+        if (size > i) {
+            System.arraycopy(longs, i + 1 , longs, i, size - i);
+        }
+        longs[size] = 0;
+        return previousValue;
+    }
+
+    public void clear() {
+        Arrays.fill(longs, 0);
+        size = 0;
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append('[');
+        for (int i = 0; i < size; i++) {
+            if (i > 0) {
+                sb.append(',');
+            }
+            sb.append(longs[i]);
+        }
+        sb.append(']');
+        return sb.toString();
+    }
+
+    public long[] toArray() {
+        return Arrays.copyOfRange(longs, 0, size);
+    }
+}

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java

@@ -24,6 +24,7 @@
  */
 package co.elastic.apm.agent.impl.transaction;
 
+import co.elastic.apm.agent.collections.LongList;
 import co.elastic.apm.agent.configuration.CoreConfiguration;
 import co.elastic.apm.agent.impl.ElasticApmTracer;
 import co.elastic.apm.agent.impl.Scope;
@@ -69,6 +70,32 @@ public abstract class AbstractSpan<T extends AbstractSpan<T>> implements Recycla
      * Flag to mark a span as representing an exit event
      */
     private boolean isExit;
+    /**
+     * <p>
+     * This use case for child ids is modifying parent/child relationships for profiler-inferred spans.
+     * Inferred spans are sent after a profiling session ends (5s by default) and after stack traces have been processed into inferred spans.
+     * Regular spans are sent right after the event (for example a DB call) has occurred.
+     * The effect is that a regular span cannot have a {@link TraceContext#parentId} pointing to an inferred span.
+     * That is because the latter did not exist at the time the regular span has been created.
+     * </p>
+     * <p>
+     * To work around this problem, inferred spans can point to their children.
+     * The UI does an operation known as "transitive reduction".
+     * What this does in this scenario is that it ignores the parent ID of a regular span if there's an inferred span
+     * with a {@code child_id} for this span.
+     * </p>
+     * <pre>
+     * ██████████████████████████████  transaction
+     * ↑ ↑ parent_id
+     * ╷ └──████████████████████       inferred span
+     * ╷         ↓ child_id
+     * └╶╶╶╶╶╶╶╶╶██████████            DB span
+     *  parent_id
+     *  (removed via transitive reduction)
+     * </pre>
+     */
+    @Nullable
+    private LongList childIds;
 
     public int getReferenceCount() {
         return references.get();
@@ -254,7 +281,7 @@ public T appendToName(CharSequence cs, int priority) {
             this.name.append(cs);
             this.namePriority = priority;
         }
-        return (T) this;
+        return thiz();
     }
 
     public T withName(@Nullable String name) {
@@ -272,7 +299,7 @@ public T withName(@Nullable String name, int priority, boolean overrideIfSamePri
             this.name.append(name);
             this.namePriority = priority;
         }
-        return (T) this;
+        return thiz();
     }
 
     /**
@@ -298,10 +325,7 @@ public void resetState() {
         namePriority = PRIO_DEFAULT;
         discardRequested = false;
         isExit = false;
-    }
-
-    public boolean isChildOf(AbstractSpan<?> parent) {
-        return traceContext.isChildOf(parent.traceContext);
+        childIds = null;
     }
 
     public Span createSpan() {
@@ -407,6 +431,17 @@ public final void end(long epochMicros) {
 
     protected abstract void afterEnd();
 
+    public boolean isChildOf(AbstractSpan<?> parent) {
+        return traceContext.isChildOf(parent.traceContext) || parent.hasChildId(traceContext.getId());
+    }
+
+    private boolean hasChildId(Id spanId) {
+        if (childIds != null) {
+            return childIds.contains(spanId.readLong(0));
+        }
+        return false;
+    }
+
     public T activate() {
         tracer.activate(this);
         return (T) this;
@@ -559,4 +594,15 @@ public boolean isSampled() {
         return getTraceContext().isSampled();
     }
 
+    public T withChildIds(@Nullable LongList childIds) {
+        this.childIds = childIds;
+        return thiz();
+    }
+
+    @Nullable
+    public LongList getChildIds() {
+        return childIds;
+    }
+
+    protected abstract T thiz();
 }

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Id.java

@@ -101,9 +101,7 @@ public void fromLongs(long... values) {
 
     @Override
     public void resetState() {
-        for (int i = 0; i < data.length; i++) {
-            data[i] = 0;
-        }
+        Arrays.fill(data, (byte) 0);
         onMutation(true);
     }
 

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Span.java

@@ -296,6 +296,11 @@ protected void recycle() {
         tracer.recycle(this);
     }
 
+    @Override
+    protected Span thiz() {
+        return this;
+    }
+
     public void setStackTrace(List<StackFrame> stackTrace) {
         this.stackFrames = stackTrace;
     }

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java

@@ -75,7 +75,6 @@
  *                      2, 1]
  * </pre>
  */
-@SuppressWarnings({"rawtypes"})
 public class TraceContext implements Recyclable {
 
     public static final String ELASTIC_TRACE_PARENT_TEXTUAL_HEADER_NAME = "elastic-apm-traceparent";
@@ -675,6 +674,12 @@ public boolean equals(Object o) {
             traceId.equals(that.traceId);
     }
 
+    public boolean idEquals(@Nullable TraceContext o) {
+        if (this == o) return true;
+        if (o == null) return false;
+        return id.equals(o.id);
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(traceId, id, parentId, flags);
@@ -745,6 +750,14 @@ public void deserialize(byte[] buffer, @Nullable String serviceName) {
         onMutation();
     }
 
+    public static void deserializeSpanId(Id id, byte[] buffer) {
+        id.fromBytes(buffer, 16);
+    }
+
+    public static long getSpanId(byte[] serializedTraceContext) {
+        return ByteUtils.getLong(serializedTraceContext, 16);
+    }
+
     public boolean traceIdAndIdEquals(byte[] serialized) {
         return id.dataEquals(serialized, traceId.getLength()) && traceId.dataEquals(serialized, 0);
     }

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java

@@ -298,6 +298,11 @@ protected void recycle() {
         tracer.recycle(this);
     }
 
+    @Override
+    protected Transaction thiz() {
+        return this;
+    }
+
     void incrementTimer(@Nullable String type, @Nullable String subtype, long duration) {
         long criticalValueAtEnter = phaser.writerCriticalSectionEnter();
         try {

apm-agent-core/src/main/java/co/elastic/apm/agent/report/serialize/DslJsonSerializer.java

@@ -24,6 +24,7 @@
  */
 package co.elastic.apm.agent.report.serialize;
 
+import co.elastic.apm.agent.collections.LongList;
 import co.elastic.apm.agent.impl.MetaData;
 import co.elastic.apm.agent.impl.context.AbstractContext;
 import co.elastic.apm.agent.impl.context.Db;
@@ -58,6 +59,7 @@
 import co.elastic.apm.agent.metrics.MetricRegistry;
 import co.elastic.apm.agent.metrics.MetricSet;
 import co.elastic.apm.agent.report.ApmServerClient;
+import co.elastic.apm.agent.util.HexUtils;
 import co.elastic.apm.agent.util.PotentiallyMultiValuedMap;
 import com.dslplatform.json.BoolConverter;
 import com.dslplatform.json.DslJson;
@@ -85,6 +87,7 @@
 import static com.dslplatform.json.JsonWriter.COMMA;
 import static com.dslplatform.json.JsonWriter.OBJECT_END;
 import static com.dslplatform.json.JsonWriter.OBJECT_START;
+import static com.dslplatform.json.JsonWriter.QUOTE;
 
 public class DslJsonSerializer implements PayloadSerializer, MetricRegistry.MetricsReporter {
 
@@ -537,6 +540,7 @@ private void serializeSpan(final Span span) {
             serializeStackTrace(span.getStackFrames());
         }
         serializeSpanContext(span.getContext(), span.getTraceContext());
+        writeHexArray("child_ids", span.getChildIds());
         serializeSpanType(span);
         jw.writeByte(OBJECT_END);
     }
@@ -1258,4 +1262,21 @@ private void writeTimestamp(final long epochMicros) {
         NumberConverter.serialize(epochMicros, jw);
         jw.writeByte(COMMA);
     }
+
+    private void writeHexArray(String fieldName, @Nullable LongList longList) {
+        if (longList != null && longList.getSize() > 0) {
+            writeFieldName(fieldName);
+            jw.writeByte(ARRAY_START);
+            for (int i = 0, size = longList.getSize(); i < size; i++) {
+                if (i > 0) {
+                    jw.writeByte(COMMA);
+                }
+                jw.writeByte(QUOTE);
+                HexUtils.writeAsHex(longList.get(i), jw);
+                jw.writeByte(QUOTE);
+            }
+            jw.writeByte(ARRAY_END);
+            jw.writeByte(COMMA);
+        }
+    }
 }