From 34f92a2c6e714708b860077c86e1d2b0522b2374 Mon Sep 17 00:00:00 2001
From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Date: Tue, 24 Oct 2023 12:16:12 -0400
Subject: [PATCH] [8.11] [Profiling] Fix Diff TopN bug (#169549) (#169648)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Backport

This will backport the following commits from `main` to `8.11`:
- [[Profiling] Fix Diff TopN bug
(#169549)](https://github.com/elastic/kibana/pull/169549)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Cauê
Marcondes","email":"55978943+cauemarcondes@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-10-24T12:53:14Z","message":"[Profiling]
Fix Diff TopN bug (#169549)\n\nFixing this error that was happening when
hovering over the grid.\r\n<img width=\"1637\" alt=\"Screenshot
2023-10-23 at 17 14
26\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/5baf2679-6803-4576-bb75-a52b41a902a0\">","sha":"0a25b39df25b61537c7c5aec9c441195f99845fd","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v8.11.0","v8.12.0","v8.10.5"],"number":169549,"url":"https://github.com/elastic/kibana/pull/169549","mergeCommit":{"message":"[Profiling]
Fix Diff TopN bug (#169549)\n\nFixing this error that was happening when
hovering over the grid.\r\n<img width=\"1637\" alt=\"Screenshot
2023-10-23 at 17 14
26\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/5baf2679-6803-4576-bb75-a52b41a902a0\">","sha":"0a25b39df25b61537c7c5aec9c441195f99845fd"}},"sourceBranch":"main","suggestedTargetBranches":["8.11","8.10"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169549","number":169549,"mergeCommit":{"message":"[Profiling]
Fix Diff TopN bug (#169549)\n\nFixing this error that was happening when
hovering over the grid.\r\n<img width=\"1637\" alt=\"Screenshot
2023-10-23 at 17 14
26\"\r\nsrc=\"https://github.com/elastic/kibana/assets/55978943/5baf2679-6803-4576-bb75-a52b41a902a0\">","sha":"0a25b39df25b61537c7c5aec9c441195f99845fd"}},{"branch":"8.10","label":"v8.10.5","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>
---
 .../observability/server/ui_settings.ts       |   6 +-
 .../e2e/profiling_views/functions.cy.ts       |   4 +
 .../topn_functions/function_row.tsx           | 100 ++++++++++++------
 3 files changed, 73 insertions(+), 37 deletions(-)

diff --git a/x-pack/plugins/observability/server/ui_settings.ts b/x-pack/plugins/observability/server/ui_settings.ts
index 134ad77046dbc..1facce3f09984 100644
--- a/x-pack/plugins/observability/server/ui_settings.ts
+++ b/x-pack/plugins/observability/server/ui_settings.ts
@@ -396,7 +396,7 @@ export const uiSettings: Record<string, UiSettings> = {
       defaultMessage: `The average amortized per-core power consumption (based on 100% CPU utilization).`,
     }),
     schema: schema.number({ min: 0 }),
-    requiresPageReload: false,
+    requiresPageReload: true,
   },
   [profilingDatacenterPUE]: {
     category: [observabilityFeatureId],
@@ -425,7 +425,7 @@ export const uiSettings: Record<string, UiSettings> = {
       },
     }),
     schema: schema.number({ min: 0 }),
-    requiresPageReload: false,
+    requiresPageReload: true,
   },
   [profilingCo2PerKWH]: {
     category: [observabilityFeatureId],
@@ -448,7 +448,7 @@ export const uiSettings: Record<string, UiSettings> = {
       },
     }),
     schema: schema.number({ min: 0 }),
-    requiresPageReload: false,
+    requiresPageReload: true,
   },
 };
 
diff --git a/x-pack/plugins/profiling/e2e/cypress/e2e/profiling_views/functions.cy.ts b/x-pack/plugins/profiling/e2e/cypress/e2e/profiling_views/functions.cy.ts
index ca972d0605e22..ac678e4650d02 100644
--- a/x-pack/plugins/profiling/e2e/cypress/e2e/profiling_views/functions.cy.ts
+++ b/x-pack/plugins/profiling/e2e/cypress/e2e/profiling_views/functions.cy.ts
@@ -203,6 +203,10 @@ describe.skip('Functions page', () => {
         .clear()
         .type('20');
       cy.contains('Save changes').click();
+      cy.getByTestSubj('kbnLoadingMessage').should('exist');
+      cy.getByTestSubj('kbnLoadingMessage').should('not.exist', {
+        timeout: 50000,
+      });
       cy.go('back');
       cy.wait('@getTopNFunctions');
       cy.get(firstRowSelector).eq(5).contains('24.22k lbs / 10.99k');
diff --git a/x-pack/plugins/profiling/public/components/topn_functions/function_row.tsx b/x-pack/plugins/profiling/public/components/topn_functions/function_row.tsx
index 0099cbc82ffec..6cf0159dcdc01 100644
--- a/x-pack/plugins/profiling/public/components/topn_functions/function_row.tsx
+++ b/x-pack/plugins/profiling/public/components/topn_functions/function_row.tsx
@@ -15,8 +15,8 @@ import {
   useEuiTheme,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import React from 'react';
 import { TopNFunctionSortField } from '@kbn/profiling-utils';
+import React, { useEffect } from 'react';
 import { asCost } from '../../utils/formatters/as_cost';
 import { asWeight } from '../../utils/formatters/as_weight';
 import { StackFrameSummary } from '../stack_frame_summary';
@@ -39,38 +39,8 @@ export function FunctionRow({
   onFrameClick,
   setCellProps,
 }: Props) {
-  const theme = useEuiTheme();
-  const successColor = useEuiBackgroundColor('success');
-  const dangerColor = useEuiBackgroundColor('danger');
-
   if (columnId === TopNFunctionSortField.Diff) {
-    if (!functionRow.diff) {
-      return (
-        <EuiText size="xs" color={theme.euiTheme.colors.primaryText}>
-          {i18n.translate('xpack.profiling.functionsView.newLabel', {
-            defaultMessage: 'New',
-          })}
-        </EuiText>
-      );
-    }
-
-    if (functionRow.diff.rank === 0) {
-      return null;
-    }
-
-    const color = functionRow.diff.rank > 0 ? 'success' : 'danger';
-    setCellProps({ style: { backgroundColor: color === 'success' ? successColor : dangerColor } });
-
-    return (
-      <EuiFlexGroup direction="row" gutterSize="xs">
-        <EuiFlexItem grow={false}>
-          <EuiIcon type={functionRow.diff.rank > 0 ? 'sortUp' : 'sortDown'} color={color} />
-        </EuiFlexItem>
-        <EuiFlexItem grow={false}>
-          <EuiText size="s">{Math.abs(functionRow.diff.rank)}</EuiText>
-        </EuiFlexItem>
-      </EuiFlexGroup>
-    );
+    return <DiffColumn diff={functionRow.diff} setCellProps={setCellProps} />;
   }
 
   if (columnId === TopNFunctionSortField.Rank) {
@@ -82,12 +52,12 @@ export function FunctionRow({
   }
 
   if (columnId === TopNFunctionSortField.Samples) {
-    setCellProps({ css: { textAlign: 'right' } });
     return (
-      <SampleStat
+      <SamplesColumn
         samples={functionRow.samples}
         diffSamples={functionRow.diff?.samples}
         totalSamples={totalCount}
+        setCellProps={setCellProps}
       />
     );
   }
@@ -116,3 +86,65 @@ export function FunctionRow({
 
   return null;
 }
+
+interface SamplesColumnProps {
+  samples: number;
+  diffSamples?: number;
+  totalSamples: number;
+  setCellProps: EuiDataGridCellValueElementProps['setCellProps'];
+}
+
+function SamplesColumn({ samples, totalSamples, diffSamples, setCellProps }: SamplesColumnProps) {
+  useEffect(() => {
+    setCellProps({ css: { textAlign: 'right' } });
+  }, [setCellProps]);
+  return <SampleStat samples={samples} diffSamples={diffSamples} totalSamples={totalSamples} />;
+}
+
+interface DiffColumnProps {
+  diff: IFunctionRow['diff'];
+  setCellProps: EuiDataGridCellValueElementProps['setCellProps'];
+}
+
+function DiffColumn({ diff, setCellProps }: DiffColumnProps) {
+  const theme = useEuiTheme();
+  const successColor = useEuiBackgroundColor('success');
+  const dangerColor = useEuiBackgroundColor('danger');
+
+  useEffect(() => {
+    if (diff && diff.rank > 0) {
+      const color = diff.rank > 0 ? 'success' : 'danger';
+      setCellProps({
+        style: { backgroundColor: color === 'success' ? successColor : dangerColor },
+      });
+    }
+  }, [dangerColor, diff, setCellProps, successColor]);
+
+  if (!diff) {
+    return (
+      <EuiText size="xs" color={theme.euiTheme.colors.primaryText}>
+        {i18n.translate('xpack.profiling.functionsView.newLabel', {
+          defaultMessage: 'New',
+        })}
+      </EuiText>
+    );
+  }
+
+  if (diff.rank === 0) {
+    return null;
+  }
+
+  return (
+    <EuiFlexGroup direction="row" gutterSize="xs">
+      <EuiFlexItem grow={false}>
+        <EuiIcon
+          type={diff.rank > 0 ? 'sortUp' : 'sortDown'}
+          color={diff.rank > 0 ? 'success' : 'danger'}
+        />
+      </EuiFlexItem>
+      <EuiFlexItem grow={false}>
+        <EuiText size="s">{Math.abs(diff.rank)}</EuiText>
+      </EuiFlexItem>
+    </EuiFlexGroup>
+  );
+}