[Profiling] Diff topN functions impact estimate fix (elastic#163749)

Context: A few weeks ago I merged a PR that moved the calculation of the impact estimate to the server. But by doing so I missed an essential part to calculate it correct, the scale factor that is defined in the UI. I'm reverting that change and making the calcs in the client again. Along with that I fixed a problem and refactored the function that calculate the impact simplifying it. Before: <img width="1313" alt="Screenshot 2023-08-14 at 10 06 39 AM" src="https://github.com/elastic/kibana/assets/55978943/c4cda975-4659-4570-afbe-4510f4e6c4fa"> After: <img width="1320" alt="Screenshot 2023-08-14 at 10 07 29 AM" src="https://github.com/elastic/kibana/assets/55978943/98e20fc3-00bd-4266-8c8d-07fb6662c9e9"> <img width="1704" alt="Screenshot 2023-08-16 at 10 03 52 AM" src="https://github.com/elastic/kibana/assets/55978943/9dc780b2-b674-4d83-90e3-7865936dbeaf">
oatkiller · Aug 16, 2023 · 1efec8e · 1efec8e
1 parent d725c28
commit 1efec8e
Show file tree

Hide file tree

Showing 15 changed files with 271 additions and 179 deletions.
diff --git a/...ck/plugins/profiling/common/calculate_impact_estimates/calculate_impact_estimates.test.ts b/...ck/plugins/profiling/common/calculate_impact_estimates/calculate_impact_estimates.test.ts
@@ -8,53 +8,79 @@ import { calculateImpactEstimates } from '.';
 
 describe('calculateImpactEstimates', () => {
   it('calculates impact when countExclusive is lower than countInclusive', () => {
-    expect(
-      calculateImpactEstimates({
-        countExclusive: 500,
-        countInclusive: 1000,
-        totalSamples: 10000,
-        totalSeconds: 15 * 60, // 15m
-      })
-    ).toEqual({
+    const { selfCPU, totalCPU, totalSamples } = calculateImpactEstimates({
+      countExclusive: 500,
+      countInclusive: 1000,
+      totalSamples: 10000,
+      totalSeconds: 15 * 60, // 15m
+    });
+
+    expect(totalCPU).toEqual({
       annualizedCo2: 17.909333333333336,
-      annualizedCo2NoChildren: 8.954666666666668,
       annualizedCoreSeconds: 1752000,
-      annualizedCoreSecondsNoChildren: 876000,
       annualizedDollarCost: 20.683333333333334,
-      annualizedDollarCostNoChildren: 10.341666666666667,
       co2: 0.0005111111111111112,
-      co2NoChildren: 0.0002555555555555556,
       coreSeconds: 50,
-      coreSecondsNoChildren: 25,
       dollarCost: 0.0005902777777777778,
-      dollarCostNoChildren: 0.0002951388888888889,
       percentage: 0.1,
-      percentageNoChildren: 0.05,
+    });
+
+    expect(selfCPU).toEqual({
+      annualizedCo2: 8.954666666666668,
+      annualizedCoreSeconds: 876000,
+      annualizedDollarCost: 10.341666666666667,
+      co2: 0.0002555555555555556,
+      coreSeconds: 25,
+      dollarCost: 0.0002951388888888889,
+      percentage: 0.05,
+    });
+
+    expect(totalSamples).toEqual({
+      percentage: 1,
+      coreSeconds: 500,
+      annualizedCoreSeconds: 17520000,
+      co2: 0.005111111111111111,
+      annualizedCo2: 179.09333333333333,
+      dollarCost: 0.0059027777777777785,
+      annualizedDollarCost: 206.83333333333337,
     });
   });
   it('calculates impact', () => {
-    expect(
-      calculateImpactEstimates({
-        countExclusive: 1000,
-        countInclusive: 1000,
-        totalSamples: 10000,
-        totalSeconds: 15 * 60, // 15m
-      })
-    ).toEqual({
+    const { selfCPU, totalCPU, totalSamples } = calculateImpactEstimates({
+      countExclusive: 1000,
+      countInclusive: 1000,
+      totalSamples: 10000,
+      totalSeconds: 15 * 60, // 15m
+    });
+
+    expect(totalCPU).toEqual({
       annualizedCo2: 17.909333333333336,
-      annualizedCo2NoChildren: 17.909333333333336,
       annualizedCoreSeconds: 1752000,
-      annualizedCoreSecondsNoChildren: 1752000,
       annualizedDollarCost: 20.683333333333334,
-      annualizedDollarCostNoChildren: 20.683333333333334,
       co2: 0.0005111111111111112,
-      co2NoChildren: 0.0005111111111111112,
       coreSeconds: 50,
-      coreSecondsNoChildren: 50,
       dollarCost: 0.0005902777777777778,
-      dollarCostNoChildren: 0.0005902777777777778,
       percentage: 0.1,
-      percentageNoChildren: 0.1,
+    });
+
+    expect(selfCPU).toEqual({
+      annualizedCo2: 17.909333333333336,
+      annualizedCoreSeconds: 1752000,
+      annualizedDollarCost: 20.683333333333334,
+      co2: 0.0005111111111111112,
+      coreSeconds: 50,
+      dollarCost: 0.0005902777777777778,
+      percentage: 0.1,
+    });
+
+    expect(totalSamples).toEqual({
+      percentage: 1,
+      coreSeconds: 500,
+      annualizedCoreSeconds: 17520000,
+      co2: 0.005111111111111111,
+      annualizedCo2: 179.09333333333333,
+      dollarCost: 0.0059027777777777785,
+      annualizedDollarCost: 206.83333333333337,
     });
   });
 });
diff --git a/x-pack/plugins/profiling/common/calculate_impact_estimates/index.ts b/x-pack/plugins/profiling/common/calculate_impact_estimates/index.ts
@@ -27,40 +27,52 @@ export function calculateImpactEstimates({
   totalSamples: number;
   totalSeconds: number;
 }) {
-  const annualizedScaleUp = ANNUAL_SECONDS / totalSeconds;
+  return {
+    totalSamples: calculateImpact({
+      samples: totalSamples,
+      totalSamples,
+      totalSeconds,
+    }),
+    totalCPU: calculateImpact({
+      samples: countInclusive,
+      totalSamples,
+      totalSeconds,
+    }),
+    selfCPU: calculateImpact({
+      samples: countExclusive,
+      totalSamples,
+      totalSeconds,
+    }),
+  };
+}
 
-  const percentage = countInclusive / totalSamples;
-  const percentageNoChildren = countExclusive / totalSamples;
+function calculateImpact({
+  samples,
+  totalSamples,
+  totalSeconds,
+}: {
+  samples: number;
+  totalSamples: number;
+  totalSeconds: number;
+}) {
+  const annualizedScaleUp = ANNUAL_SECONDS / totalSeconds;
   const totalCoreSeconds = totalSamples / 20;
+  const percentage = samples / totalSamples;
   const coreSeconds = totalCoreSeconds * percentage;
   const annualizedCoreSeconds = coreSeconds * annualizedScaleUp;
-  const coreSecondsNoChildren = totalCoreSeconds * percentageNoChildren;
-  const annualizedCoreSecondsNoChildren = coreSecondsNoChildren * annualizedScaleUp;
   const coreHours = coreSeconds / (60 * 60);
-  const coreHoursNoChildren = coreSecondsNoChildren / (60 * 60);
   const co2 = ((PER_CORE_WATT * coreHours) / 1000.0) * CO2_PER_KWH;
-  const co2NoChildren = ((PER_CORE_WATT * coreHoursNoChildren) / 1000.0) * CO2_PER_KWH;
   const annualizedCo2 = co2 * annualizedScaleUp;
-  const annualizedCo2NoChildren = co2NoChildren * annualizedScaleUp;
   const dollarCost = coreHours * CORE_COST_PER_HOUR;
   const annualizedDollarCost = dollarCost * annualizedScaleUp;
-  const dollarCostNoChildren = coreHoursNoChildren * CORE_COST_PER_HOUR;
-  const annualizedDollarCostNoChildren = dollarCostNoChildren * annualizedScaleUp;
 
   return {
     percentage,
-    percentageNoChildren,
     coreSeconds,
     annualizedCoreSeconds,
-    coreSecondsNoChildren,
-    annualizedCoreSecondsNoChildren,
     co2,
-    co2NoChildren,
     annualizedCo2,
-    annualizedCo2NoChildren,
     dollarCost,
     annualizedDollarCost,
-    dollarCostNoChildren,
-    annualizedDollarCostNoChildren,
   };
 }
diff --git a/x-pack/plugins/profiling/common/functions.test.ts b/x-pack/plugins/profiling/common/functions.test.ts
@@ -17,7 +17,7 @@ describe('TopN function operations', () => {
     const { events, stackTraces, stackFrames, executables, samplingRate } =
       decodeStackTraceResponse(response);
 
-    describe(`stacktraces from ${seconds} seconds and upsampled by ${upsampledBy}`, () => {
+    describe(`stacktraces upsampled by ${upsampledBy}`, () => {
       const maxTopN = 5;
       const topNFunctions = createTopNFunctions({
         events,
@@ -27,7 +27,6 @@ describe('TopN function operations', () => {
         startIndex: 0,
         endIndex: maxTopN,
         samplingRate,
-        totalSeconds: seconds,
       });
       const exclusiveCounts = topNFunctions.TopN.map((value) => value.CountExclusive);
 

diff --git a/x-pack/plugins/profiling/common/functions.ts b/x-pack/plugins/profiling/common/functions.ts
@@ -6,7 +6,6 @@
  */
 import * as t from 'io-ts';
 import { sumBy } from 'lodash';
-import { calculateImpactEstimates } from './calculate_impact_estimates';
 import { createFrameGroupID, FrameGroupID } from './frame_group';
 import {
   createStackFrameMetadata,
@@ -35,18 +34,14 @@ type TopNFunction = Pick<
 > & {
   Id: string;
   Rank: number;
-  impactEstimates?: ReturnType<typeof calculateImpactEstimates>;
-  selfCPUPerc: number;
-  totalCPUPerc: number;
 };
 
 export interface TopNFunctions {
   TotalCount: number;
   TopN: TopNFunction[];
   SamplingRate: number;
-  impactEstimates?: ReturnType<typeof calculateImpactEstimates>;
-  selfCPUPerc: number;
-  totalCPUPerc: number;
+  selfCPU: number;
+  totalCPU: number;
 }
 
 export function createTopNFunctions({
@@ -57,7 +52,6 @@ export function createTopNFunctions({
   stackFrames,
   stackTraces,
   startIndex,
-  totalSeconds,
 }: {
   endIndex: number;
   events: Map<StackTraceID, number>;
@@ -66,7 +60,6 @@ export function createTopNFunctions({
   stackFrames: Map<StackFrameID, StackFrame>;
   stackTraces: Map<StackTraceID, StackTrace>;
   startIndex: number;
-  totalSeconds: number;
 }): TopNFunctions {
   // The `count` associated with a frame provides the total number of
   // traces in which that node has appeared at least once. However, a
@@ -167,52 +160,25 @@ export function createTopNFunctions({
   const framesAndCountsAndIds = topN.slice(startIndex, endIndex).map((frameAndCount, i) => {
     const countExclusive = frameAndCount.CountExclusive;
     const countInclusive = frameAndCount.CountInclusive;
-    const totalCPUPerc = (countInclusive / totalCount) * 100;
-    const selfCPUPerc = (countExclusive / totalCount) * 100;
-
-    const impactEstimates =
-      totalSeconds > 0
-        ? calculateImpactEstimates({
-            countExclusive,
-            countInclusive,
-            totalSamples: totalCount,
-            totalSeconds,
-          })
-        : undefined;
+
     return {
       Rank: i + 1,
       Frame: frameAndCount.Frame,
       CountExclusive: countExclusive,
-      selfCPUPerc,
       CountInclusive: countInclusive,
-      totalCPUPerc,
       Id: frameAndCount.FrameGroupID,
-      impactEstimates,
     };
   });
 
   const sumSelfCPU = sumBy(framesAndCountsAndIds, 'CountExclusive');
-  const selfCPUPerc = (sumSelfCPU / totalCount) * 100;
   const sumTotalCPU = sumBy(framesAndCountsAndIds, 'CountInclusive');
-  const totalCPUPerc = (sumTotalCPU / totalCount) * 100;
-
-  const impactEstimates =
-    totalSeconds > 0
-      ? calculateImpactEstimates({
-          countExclusive: sumSelfCPU,
-          countInclusive: sumTotalCPU,
-          totalSamples: totalCount,
-          totalSeconds,
-        })
-      : undefined;
 
   return {
     TotalCount: totalCount,
     TopN: framesAndCountsAndIds,
     SamplingRate: samplingRate,
-    impactEstimates,
-    selfCPUPerc,
-    totalCPUPerc,
+    selfCPU: sumSelfCPU,
+    totalCPU: sumTotalCPU,
   };
 }
 

diff --git a/x-pack/plugins/profiling/public/components/flamegraph/flamegraph_tooltip.tsx b/x-pack/plugins/profiling/public/components/flamegraph/flamegraph_tooltip.tsx
@@ -99,8 +99,8 @@ export function FlameGraphTooltip({
                     labelStyle={{ fontWeight: 'bold' }}
                   />
                 }
-                value={impactEstimates.percentage}
-                comparison={comparisonImpactEstimates?.percentage}
+                value={impactEstimates.totalCPU.percentage}
+                comparison={comparisonImpactEstimates?.totalCPU.percentage}
                 formatValue={asPercentage}
                 showDifference
                 formatDifferenceAsPercentage
@@ -115,8 +115,8 @@ export function FlameGraphTooltip({
                     labelStyle={{ fontWeight: 'bold' }}
                   />
                 }
-                value={impactEstimates.percentageNoChildren}
-                comparison={comparisonImpactEstimates?.percentageNoChildren}
+                value={impactEstimates.selfCPU.percentage}
+                comparison={comparisonImpactEstimates?.selfCPU.percentage}
                 showDifference
                 formatDifferenceAsPercentage
                 formatValue={asPercentage}
@@ -144,8 +144,8 @@ export function FlameGraphTooltip({
             label={i18n.translate('xpack.profiling.flameGraphTooltip.annualizedCo2', {
               defaultMessage: `Annualized CO2`,
             })}
-            value={impactEstimates.annualizedCo2}
-            comparison={comparisonImpactEstimates?.annualizedCo2}
+            value={impactEstimates.totalCPU.annualizedCo2}
+            comparison={comparisonImpactEstimates?.totalCPU.annualizedCo2}
             formatValue={asWeight}
             showDifference
             formatDifferenceAsPercentage={false}
@@ -155,8 +155,8 @@ export function FlameGraphTooltip({
             label={i18n.translate('xpack.profiling.flameGraphTooltip.annualizedDollarCost', {
               defaultMessage: `Annualized dollar cost`,
             })}
-            value={impactEstimates.annualizedDollarCost}
-            comparison={comparisonImpactEstimates?.annualizedDollarCost}
+            value={impactEstimates.totalCPU.annualizedDollarCost}
+            comparison={comparisonImpactEstimates?.totalCPU.annualizedDollarCost}
             formatValue={asCost}
             showDifference
             formatDifferenceAsPercentage={false}