[Profiling] Fix stacktraces total_frames response value and improve…

… tests (elastic#103062) * Fix flamegraph total_frame value and improve tests * Make inline aggregation more expressive
n1v0lg · Dec 7, 2023 · c6eff19 · c6eff19
1 parent c7406ed
commit c6eff19
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 15 deletions.
diff --git a/...src/internalClusterTest/java/org/elasticsearch/xpack/profiling/GetFlameGraphActionIT.java b/...src/internalClusterTest/java/org/elasticsearch/xpack/profiling/GetFlameGraphActionIT.java
@@ -14,8 +14,12 @@ public void testGetStackTracesUnfiltered() throws Exception {
         // only spot-check top level properties - detailed tests are done in unit tests
         assertEquals(994, response.getSize());
         assertEquals(1.0d, response.getSamplingRate(), 0.001d);
-        assertEquals(44, response.getSelfCPU());
-        assertEquals(1865, response.getTotalCPU());
-        assertEquals(44, response.getTotalSamples());
+        assertEquals(46, response.getSelfCPU());
+        assertEquals(1903, response.getTotalCPU());
+        assertEquals(46, response.getTotalSamples());
+
+        // The root node's values are the same as the top-level values.
+        assertEquals("", response.getFileIds().get(0));
+        assertEquals(response.getSelfCPU(), response.getCountInclusive().get(0).longValue());
     }
 }
diff --git a/...rc/internalClusterTest/java/org/elasticsearch/xpack/profiling/GetStackTracesActionIT.java b/...rc/internalClusterTest/java/org/elasticsearch/xpack/profiling/GetStackTracesActionIT.java
@@ -14,14 +14,14 @@
 
 public class GetStackTracesActionIT extends ProfilingTestCase {
     public void testGetStackTracesUnfiltered() throws Exception {
-        GetStackTracesRequest request = new GetStackTracesRequest(10, 1.0d, 1.0d, null, null, null, null, null, null, null, null);
+        GetStackTracesRequest request = new GetStackTracesRequest(1000, 600.0d, 1.0d, null, null, null, null, null, null, null, null);
         request.setAdjustSampleCount(true);
         GetStackTracesResponse response = client().execute(GetStackTracesAction.INSTANCE, request).get();
-        assertEquals(40, response.getTotalSamples());
-        assertEquals(473, response.getTotalFrames());
+        assertEquals(46, response.getTotalSamples());
+        assertEquals(1821, response.getTotalFrames());
 
         assertNotNull(response.getStackTraceEvents());
-        assertEquals(4L, response.getStackTraceEvents().get("L7kj7UvlKbT-vN73el4faQ").count);
+        assertEquals(3L, response.getStackTraceEvents().get("L7kj7UvlKbT-vN73el4faQ").count);
 
         assertNotNull(response.getStackTraces());
         // just do a high-level spot check. Decoding is tested in unit-tests
@@ -30,8 +30,8 @@ public void testGetStackTracesUnfiltered() throws Exception {
         assertEquals(18, stackTrace.fileIds.size());
         assertEquals(18, stackTrace.frameIds.size());
         assertEquals(18, stackTrace.typeIds.size());
-        assertEquals(0.007903d, stackTrace.annualCO2Tons, 0.000001d);
-        assertEquals(74.46d, stackTrace.annualCostsUSD, 0.01d);
+        assertEquals(0.0000098789d, stackTrace.annualCO2Tons, 0.0000000001d);
+        assertEquals(0.093075d, stackTrace.annualCostsUSD, 0.000001d);
 
         assertNotNull(response.getStackFrames());
         StackFrame stackFrame = response.getStackFrames().get("8NlMClggx8jaziUTJXlmWAAAAAAAAIYI");
@@ -58,7 +58,7 @@ public void testGetStackTracesFromAPMWithMatch() throws Exception {
             null
         );
         GetStackTracesResponse response = client().execute(GetStackTracesAction.INSTANCE, request).get();
-        assertEquals(43, response.getTotalFrames());
+        assertEquals(49, response.getTotalFrames());
 
         assertNotNull(response.getStackTraceEvents());
         assertEquals(3L, response.getStackTraceEvents().get("Ce77w10WeIDow3kd1jowlA").count);

diff --git a/x-pack/plugin/profiling/src/internalClusterTest/resources/data/profiling-events-all.ndjson b/x-pack/plugin/profiling/src/internalClusterTest/resources/data/profiling-events-all.ndjson
@@ -71,6 +71,8 @@
 {"create": {"_index": "profiling-events-all"}}
 {"Stacktrace.count": [1], "profiling.project.id": ["100"], "os.kernel": ["9.9.9-0"], "tags": ["environment:qa", "region:eu-west-1"], "host.ip": ["192.168.1.2"], "@timestamp": ["1698624000"], "container.name": ["instance-0000000010"], "ecs.version": ["1.12.0"], "Stacktrace.id": ["XF9MchOwpePfa6_hYy-vZQ"], "agent.version": ["head-be593ef3-1688111067"], "host.name": ["ip-192-168-1-2"], "host.id": ["8457605156473051743"], "process.thread.name": ["497295213074376"]}
 {"create": {"_index": "profiling-events-all"}}
+{"Stacktrace.count": [2], "profiling.project.id": ["100"], "os.kernel": ["9.9.9-0"], "tags": ["environment:qa", "region:eu-west-1"], "host.ip": ["192.168.1.2"], "@timestamp": ["1698624000"], "container.name": ["instance-0000000010"], "ecs.version": ["1.12.0"], "Stacktrace.id": ["L7kj7UvlKbT-vN73el4faQ"], "agent.version": ["head-be593ef3-1688111067"], "host.name": ["ip-192-168-1-2"], "host.id": ["8457605156473051743"], "process.thread.name": ["497295213074376"]}
+{"create": {"_index": "profiling-events-all"}}
 {"Stacktrace.count": [1], "profiling.project.id": ["100"], "os.kernel": ["9.9.9-0"], "tags": ["environment:qa", "region:eu-west-1"], "host.ip": ["192.168.1.2"], "@timestamp": ["1698624000"], "container.name": ["instance-0000000010"], "ecs.version": ["1.12.0"], "Stacktrace.id": ["L7kj7UvlKbT-vN73el4faQ"], "agent.version": ["head-be593ef3-1688111067"], "host.name": ["ip-192-168-1-2"], "host.id": ["8457605156473051743"], "process.thread.name": ["497295213074376"]}
 {"create": {"_index": "profiling-events-all"}}
 {"Stacktrace.count": [1], "profiling.project.id": ["100"], "os.kernel": ["9.9.9-0"], "tags": ["environment:qa", "region:eu-west-1"], "host.ip": ["192.168.1.2"], "@timestamp": ["1698624000"], "container.name": ["instance-0000000010"], "ecs.version": ["1.12.0"], "Stacktrace.id": ["hRqQI2CBPiapzgFG9jrmDA"], "agent.version": ["head-be593ef3-1688111067"], "host.name": ["ip-192-168-1-2"], "host.id": ["8457605156473051743"], "process.thread.name": ["599103450330106"]}

diff --git a/...ugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/GetFlamegraphResponse.java b/...ugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/GetFlamegraphResponse.java
@@ -194,6 +194,14 @@ public List<Integer> getSourceLines() {
         return sourceLines;
     }
 
+    public List<Double> getAnnualCO2TonsInclusive() {
+        return annualCO2TonsInclusive;
+    }
+
+    public List<Double> getAnnualCostsUSDInclusive() {
+        return annualCostsUSDInclusive;
+    }
+
     public long getSelfCPU() {
         return selfCPU;
     }

diff --git a/...filing/src/main/java/org/elasticsearch/xpack/profiling/GetStackTracesResponseBuilder.java b/...filing/src/main/java/org/elasticsearch/xpack/profiling/GetStackTracesResponseBuilder.java
@@ -54,6 +54,10 @@ public void setTotalFrames(int totalFrames) {
         this.totalFrames = totalFrames;
     }
 
+    public void addTotalFrames(int numFrames) {
+        this.totalFrames += numFrames;
+    }
+
     public void setStackFrames(Map<String, StackFrame> stackFrames) {
         this.stackFrames = stackFrames;
     }

diff --git a/x-pack/plugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/StackFrame.java b/x-pack/plugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/StackFrame.java
@@ -30,17 +30,28 @@ final class StackFrame implements ToXContentObject {
         this.lineNumber = listOf(lineNumber);
     }
 
+    public int size() {
+        return this.functionName.size(); // functionName is the only array that is always set
+    }
+
+    /**
+     * Returns the number of inlined frames in this stack frame.
+     * @return the number of inlined frames in this stack frame.
+     */
+    public int inlineFrameCount() {
+        return size() > 0 ? size() - 1 : 0;
+    }
+
     public void forEach(Consumer<Frame> action) {
-        int size = this.functionName.size(); // functionName is the only array that is always set
-        for (int i = 0; i < size; i++) {
+        for (int i = 0; i < size(); i++) {
             action.accept(
                 new Frame(
                     fileName.size() > i ? fileName.get(i) : "",
                     functionName.get(i),
                     functionOffset.size() > i ? functionOffset.get(i) : 0,
                     lineNumber.size() > i ? lineNumber.get(i) : 0,
                     i > 0,
-                    i == size - 1
+                    i == size() - 1
                 )
             );
         }
@@ -67,7 +78,7 @@ public static StackFrame fromSource(Map<String, Object> source) {
     }
 
     public boolean isEmpty() {
-        return fileName.isEmpty() && functionName.isEmpty() && functionOffset.isEmpty() && lineNumber.isEmpty();
+        return size() == 0;
     }
 
     @Override

diff --git a/...filing/src/main/java/org/elasticsearch/xpack/profiling/TransportGetStackTracesAction.java b/...filing/src/main/java/org/elasticsearch/xpack/profiling/TransportGetStackTracesAction.java
@@ -688,6 +688,7 @@ private static class DetailsHandler {
         private final Map<String, String> executables;
         private final Map<String, StackFrame> stackFrames;
         private final AtomicInteger expectedSlices;
+        private final AtomicInteger totalInlineFrames = new AtomicInteger();
         private final StopWatch watch = new StopWatch("retrieveStackTraceDetails");
 
         private DetailsHandler(
@@ -718,7 +719,9 @@ public void onStackFramesResponse(MultiGetResponse multiGetItemResponses) {
                     if (stackFrames.containsKey(frame.getId()) == false) {
                         StackFrame stackFrame = StackFrame.fromSource(frame.getResponse().getSource());
                         if (stackFrame.isEmpty() == false) {
-                            stackFrames.putIfAbsent(frame.getId(), stackFrame);
+                            if (stackFrames.putIfAbsent(frame.getId(), stackFrame) == null) {
+                                totalInlineFrames.addAndGet(stackFrame.inlineFrameCount());
+                            }
                         } else {
                             log.trace("Stack frame with id [{}] has no properties.", frame.getId());
                         }
@@ -757,6 +760,7 @@ public void mayFinish() {
             if (expectedSlices.decrementAndGet() == 0) {
                 builder.setExecutables(executables);
                 builder.setStackFrames(stackFrames);
+                builder.addTotalFrames(totalInlineFrames.get());
                 log.debug("retrieveStackTraceDetails found [{}] stack frames, [{}] executables.", stackFrames.size(), executables.size());
                 log.debug(watch::report);
                 submitListener.onResponse(builder.build());