Traces/Services - Query optimization / UI setting / Bugfix (#2310)

* improve service node query and related services query Signed-off-by: Adam Tackett <[email protected]> * update url redirection for services for the table and expanded view actions Signed-off-by: Adam Tackett <[email protected]> * remove extra toast messages from handleServiceMapRequest Signed-off-by: Adam Tackett <[email protected]> * add observability setting for enabling custom source as the default mode Signed-off-by: Adam Tackett <[email protected]> * fix bug returning errors on spans that have a status.code === 1 Signed-off-by: Adam Tackett <[email protected]> * fix bug causing infinite refrshed if mds disabled and services trace link clicked Signed-off-by: Adam Tackett <[email protected]> * add the mode param to url for services - trace redirection Signed-off-by: Adam Tackett <[email protected]> * fix flaky cypress test for services Signed-off-by: Adam Tackett <[email protected]> * address comments Signed-off-by: Adam Tackett <[email protected]> * remove test, add comment, remove related service param Signed-off-by: Adam Tackett <[email protected]> * place related services back Signed-off-by: Adam Tackett <[email protected]> * add type for serviceMapParams, im prove readability, remove related servics Signed-off-by: Adam Tackett <[email protected]> * update service map test for the new type Signed-off-by: Adam Tackett <[email protected]> --------- Signed-off-by: Adam Tackett <[email protected]> Co-authored-by: Adam Tackett <[email protected]>
opensearch-project · Jan 17, 2025 · 8016a73 · 8016a73
1 parent 453cca8
commit 8016a73
Show file tree

Hide file tree

Showing 15 changed files with 201 additions and 150 deletions.
diff --git a/.cypress/utils/constants.js b/.cypress/utils/constants.js
@@ -87,6 +87,7 @@ export const setTimeFilter = (setEndTime = false, refresh = true) => {
 };
 
 export const expandServiceView = (rowIndex = 0) => {
+  cy.get('[data-test-subj="globalLoadingIndicator"]').should('not.exist');//Replaces wait
   cy.get('*[data-test-subj^="service-flyout-action-btntrace_service"]').eq(rowIndex).click();
   cy.get('[data-test-subj="ActionContextMenu"]').click();
   cy.get('[data-test-subj="viewServiceButton"]').click();

diff --git a/common/constants/trace_analytics.ts b/common/constants/trace_analytics.ts
@@ -24,6 +24,7 @@ export const TRACE_ANALYTICS_DSL_ROUTE = '/api/observability/trace_analytics/que
 
 export const TRACE_CUSTOM_SPAN_INDEX_SETTING = 'observability:traceAnalyticsSpanIndices';
 export const TRACE_CUSTOM_SERVICE_INDEX_SETTING = 'observability:traceAnalyticsServiceIndices';
+export const TRACE_CUSTOM_MODE_DEFAULT_SETTING = 'observability:traceAnalyticsCustomModeDefault';
 
 export enum TRACE_TABLE_TITLES {
   all_spans = 'All Spans',

diff --git a/.../trace_analytics/components/common/__tests__/__snapshots__/helper_functions.test.tsx.snap b/.../trace_analytics/components/common/__tests__/__snapshots__/helper_functions.test.tsx.snap
@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Helper functions renders benchmark 1`] = `
+exports[`Trace analytics helper functions renders benchmark 1`] = `
 <EuiText
   size="s"
   style={
@@ -22,7 +22,7 @@ exports[`Helper functions renders benchmark 1`] = `
 </EuiText>
 `;
 
-exports[`Helper functions renders benchmark 2`] = `
+exports[`Trace analytics helper functions renders benchmark 2`] = `
 <EuiText
   size="s"
   style={
@@ -44,7 +44,7 @@ exports[`Helper functions renders benchmark 2`] = `
 </EuiText>
 `;
 
-exports[`Helper functions renders benchmark 3`] = `
+exports[`Trace analytics helper functions renders benchmark 3`] = `
 <EuiText
   size="s"
   style={
@@ -66,7 +66,7 @@ exports[`Helper functions renders benchmark 3`] = `
 </EuiText>
 `;
 
-exports[`Helper functions renders no match and missing configuration messages 1`] = `
+exports[`Trace analytics helper functions renders no match and missing configuration messages 1`] = `
 <Fragment>
   <EuiSpacer
     size="s"
@@ -91,7 +91,7 @@ exports[`Helper functions renders no match and missing configuration messages 1`
 </Fragment>
 `;
 
-exports[`Helper functions renders no match and missing configuration messages 2`] = `
+exports[`Trace analytics helper functions renders no match and missing configuration messages 2`] = `
 <Fragment>
   <EuiEmptyPrompt
     actions={
@@ -120,7 +120,7 @@ exports[`Helper functions renders no match and missing configuration messages 2`
 </Fragment>
 `;
 
-exports[`Helper functions renders panel title 1`] = `
+exports[`Trace analytics helper functions renders panel title 1`] = `
 <EuiText
   size="m"
 >
@@ -137,7 +137,7 @@ exports[`Helper functions renders panel title 1`] = `
 </EuiText>
 `;
 
-exports[`Helper functions renders panel title 2`] = `
+exports[`Trace analytics helper functions renders panel title 2`] = `
 <EuiText
   size="m"
 >

diff --git a/public/components/trace_analytics/components/common/__tests__/helper_functions.test.tsx b/public/components/trace_analytics/components/common/__tests__/helper_functions.test.tsx
@@ -22,7 +22,6 @@ import {
   getPercentileFilter,
   getServiceMapGraph,
   getServiceMapScaleColor,
-  getServiceMapTargetResources,
   getTimestampPrecision,
   milliToNanoSec,
   minFixedInterval,
@@ -33,7 +32,7 @@ import {
   renderBenchmark,
 } from '../helper_functions';
 
-describe('Helper functions', () => {
+describe('Trace analytics helper functions', () => {
   configure({ adapter: new Adapter() });
 
   it('renders panel title', () => {
@@ -83,22 +82,14 @@ describe('Helper functions', () => {
   });
 
   it('returns service map graph', () => {
-    const serviceMapGraph = getServiceMapGraph(TEST_SERVICE_MAP, 'latency', [
-      0,
-      50,
-      100,
-      150,
-      200,
-      250,
-    ]);
+    const serviceMapGraph = getServiceMapGraph({
+      map: TEST_SERVICE_MAP,
+      idSelected: 'latency',
+      ticks: [0, 50, 100, 150, 200, 250],
+    });
     expect(serviceMapGraph).toEqual(TEST_SERVICE_MAP_GRAPH);
   });
 
-  it('returns target resources by service name', () => {
-    const targetResources = getServiceMapTargetResources(TEST_SERVICE_MAP, 'order');
-    expect(targetResources).toEqual(['clear_order', 'update_order', 'get_order', 'pay_order']);
-  });
-
   it('calculates ticks', () => {
     const ticks = calculateTicks(500, 200);
     const ticks2 = calculateTicks(0, 200, 10);

diff --git a/public/components/trace_analytics/components/common/custom_index_flyout.tsx b/public/components/trace_analytics/components/common/custom_index_flyout.tsx
@@ -5,6 +5,7 @@
 
 import {
   EuiCallOut,
+  EuiCheckbox,
   EuiCompressedFieldText,
   EuiDescribedFormGroup,
   EuiFlexGroup,
@@ -23,6 +24,7 @@ import React, { Fragment, useEffect, useState } from 'react';
 import {
   TRACE_CUSTOM_SERVICE_INDEX_SETTING,
   TRACE_CUSTOM_SPAN_INDEX_SETTING,
+  TRACE_CUSTOM_MODE_DEFAULT_SETTING,
 } from '../../../../../common/constants/trace_analytics';
 import { uiSettingsService } from '../../../../../common/utils';
 import { useToast } from '../../../common/toast';
@@ -39,6 +41,7 @@ export const CustomIndexFlyout = ({
   const { setToast } = useToast();
   const [spanIndices, setSpanIndices] = useState('');
   const [serviceIndices, setServiceIndices] = useState('');
+  const [customModeDefault, setCustomModeDefault] = useState(false);
   const [isLoading, setIsLoading] = useState(false);
 
   const onChangeSpanIndices = (e: { target: { value: React.SetStateAction<string> } }) => {
@@ -49,21 +52,27 @@ export const CustomIndexFlyout = ({
     setServiceIndices(e.target.value);
   };
 
+  const onToggleCustomModeDefault = (e: { target: { checked: boolean } }) => {
+    setCustomModeDefault(e.target.checked);
+  };
+
   useEffect(() => {
     setSpanIndices(uiSettingsService.get(TRACE_CUSTOM_SPAN_INDEX_SETTING));
     setServiceIndices(uiSettingsService.get(TRACE_CUSTOM_SERVICE_INDEX_SETTING));
+    setCustomModeDefault(uiSettingsService.get(TRACE_CUSTOM_MODE_DEFAULT_SETTING) || false);
   }, [uiSettingsService]);
 
-  const onSaveIndices = async () => {
+  const onSaveSettings = async () => {
     try {
       setIsLoading(true);
       await uiSettingsService.set(TRACE_CUSTOM_SPAN_INDEX_SETTING, spanIndices);
       await uiSettingsService.set(TRACE_CUSTOM_SERVICE_INDEX_SETTING, serviceIndices);
+      await uiSettingsService.set(TRACE_CUSTOM_MODE_DEFAULT_SETTING, customModeDefault);
       setIsLoading(false);
-      setToast('Updated trace analytics sources successfully', 'success');
+      setToast('Updated trace analytics settings successfully', 'success');
     } catch (error) {
       console.error(error);
-      setToast('Failed to update trace analytics sources', 'danger');
+      setToast('Failed to update trace analytics settings', 'danger');
     }
     setIsLoading(false);
   };
@@ -134,6 +143,23 @@ export const CustomIndexFlyout = ({
               />
             </EuiFormRow>
           </EuiDescribedFormGroup>
+          <EuiDescribedFormGroup
+            title={<h3>Set default mode</h3>}
+            description={
+              <Fragment>
+                Enable this to set &quot;Custom source&quot; as the default mode for trace analytics
+              </Fragment>
+            }
+          >
+            <EuiFormRow>
+              <EuiCheckbox
+                id="customModeDefault"
+                label="Enable custom source as default mode"
+                checked={customModeDefault}
+                onChange={onToggleCustomModeDefault}
+              />
+            </EuiFormRow>
+          </EuiDescribedFormGroup>
         </EuiFlyoutBody>
         <EuiFlyoutFooter>
           <EuiFlexGroup justifyContent="spaceBetween">
@@ -149,7 +175,7 @@ export const CustomIndexFlyout = ({
             <EuiFlexItem grow={false}>
               <EuiSmallButton
                 onClick={async () => {
-                  await onSaveIndices();
+                  await onSaveSettings();
                   setIsFlyoutVisible(false);
                 }}
                 fill

diff --git a/public/components/trace_analytics/components/common/helper_functions.tsx b/public/components/trace_analytics/components/common/helper_functions.tsx
@@ -208,17 +208,22 @@ function filterGraphBySelectedNode(
   return { graph: { nodes: connectedNodes, edges: connectedEdges } };
 }
 
-// construct vis-js graph from ServiceObject
-export function getServiceMapGraph(
-  map: ServiceObject,
-  idSelected: 'latency' | 'error_rate' | 'throughput',
-  ticks: number[],
-  currService?: string,
-  relatedServices?: string[],
-  filterByCurrService?: boolean
-) {
-  if (!relatedServices) relatedServices = Object.keys(map);
+interface ServiceMapParams {
+  map: ServiceObject;
+  idSelected: 'latency' | 'error_rate' | 'throughput';
+  ticks: number[];
+  currService?: string;
+  filterByCurrService?: boolean;
+}
 
+// construct vis-js graph from ServiceObject
+export function getServiceMapGraph({
+  map,
+  idSelected,
+  ticks,
+  currService,
+  filterByCurrService,
+}: ServiceMapParams) {
   const nodes = Object.keys(map).map((service) => {
     const value = map[service][idSelected];
     let styleOptions;
@@ -229,14 +234,10 @@ export function getServiceMapGraph(
         borderWidth: 3,
         color: {
           border: '#4A4A4A',
-          background:
-            relatedServices!.indexOf(service) >= 0 ? `rgba(${color}, 1)` : `rgba(${color}, 0.2)`,
+          background: `rgba(${color}, 1)`,
         },
         font: {
-          color:
-            relatedServices!.indexOf(service) >= 0
-              ? `rgba(72, 122, 180, 1)`
-              : `rgba(72, 122, 180, 0.2)`,
+          color: `rgba(72, 122, 180, 1)`,
         },
       };
     } else {
@@ -285,10 +286,7 @@ export function getServiceMapGraph(
         edges.push({
           from: map[service].id,
           to: map[target].id,
-          color:
-            relatedServices!.indexOf(service) >= 0 && relatedServices!.indexOf(target) >= 0
-              ? `rgba(${edgeColor}, 1)`
-              : `rgba(${edgeColor}, 0.2)`,
+          color: `rgba(${edgeColor}, 1)`,
         });
       });
   });
@@ -299,14 +297,6 @@ export function getServiceMapGraph(
   return { graph: { nodes, edges } };
 }
 
-// returns flattened targetResource as an array for all traceGroups
-export function getServiceMapTargetResources(map: ServiceObject, serviceName: string) {
-  return ([] as string[]).concat.apply(
-    [],
-    [...map[serviceName].traceGroups.map((traceGroup) => [...traceGroup.targetResource])]
-  );
-}
-
 export function calculateTicks(min: number, max: number, numTicks = 5): number[] {
   if (min >= max) return calculateTicks(0, Math.max(1, max), numTicks);
   min = Math.floor(min);
@@ -615,14 +605,22 @@ export const getServiceIndices = (mode: TraceAnalyticsMode) => {
   }
 };
 
-export const generateServiceUrl = (service: string, dataSourceId: string) => {
-  const url = `#/services?serviceId=${encodeURIComponent(service)}`;
+export const generateServiceUrl = (
+  service: string,
+  dataSourceId: string,
+  mode?: TraceAnalyticsMode
+): string => {
+  let url = `#/services?serviceId=${encodeURIComponent(service)}`;
 
   if (dataSourceId && dataSourceId !== '') {
-    return `${url}&datasourceId=${encodeURIComponent(dataSourceId)}`;
+    url += `&datasourceId=${encodeURIComponent(dataSourceId)}`;
+  }
+
+  if (mode) {
+    url += `&mode=${encodeURIComponent(mode)}`;
   }
 
-  return `${url}&datasourceId=`;
+  return url;
 };
 
 interface FullScreenWrapperProps {

diff --git a/public/components/trace_analytics/components/common/plots/service_map.tsx b/public/components/trace_analytics/components/common/plots/service_map.tsx
@@ -296,28 +296,25 @@ export function ServiceMap({
       if (focusedService !== null) {
         removeFilter('serviceName', focusedService);
         setItems(
-          getServiceMapGraph(
-            serviceMap,
+          getServiceMapGraph({
+            map: serviceMap,
             idSelected,
             ticks,
-            undefined,
-            undefined,
-            false // Show the entire graph without filtering
-          )
+            filterByCurrService: false,
+          })
         );
         setFocusedService(null);
         setInvalid(false);
       }
     } else if (serviceMap[service]) {
       if (focusedService !== service) {
-        const filteredGraph = getServiceMapGraph(
-          serviceMap,
+        const filteredGraph = getServiceMapGraph({
+          map: serviceMap,
           idSelected,
           ticks,
-          service,
-          serviceMap[service]?.relatedServices,
-          true // Enable filtering to focus on connected nodes
-        );
+          currService: service,
+          filterByCurrService: true,
+        });
         setItems(filteredGraph);
         setFocusedService(service);
       }
@@ -369,14 +366,13 @@ export function ServiceMap({
     // Adjust graph rendering logic to ensure related services are visible
     const showRelatedServices = focusedService ? true : filterByCurrService;
     setItems(
-      getServiceMapGraph(
-        serviceMap,
+      getServiceMapGraph({
+        map: serviceMap,
         idSelected,
-        calculatedTicks,
-        focusedService ?? currService,
-        serviceMap[currService!]?.relatedServices,
-        showRelatedServices
-      )
+        ticks: calculatedTicks,
+        currService: focusedService ?? currService,
+        filterByCurrService: showRelatedServices,
+      })
     );
   }, [serviceMap, idSelected, focusedService, filterByCurrService]);
 

diff --git a/public/components/trace_analytics/components/services/__tests__/services_table.test.tsx b/public/components/trace_analytics/components/services/__tests__/services_table.test.tsx
@@ -118,6 +118,7 @@ describe('Services table component', () => {
 
   it('redirects to the correct URL when the service link is clicked', () => {
     const mockDataSourceId = 'mock-data-source-id';
+    const mockMode = 'data_prepper';
     const tableItems = [
       {
         name: 'checkoutservice',
@@ -161,7 +162,7 @@ describe('Services table component', () => {
 
     serviceLink.simulate('click');
 
-    const expectedUrl = generateServiceUrl('checkoutservice', mockDataSourceId);
+    const expectedUrl = generateServiceUrl('checkoutservice', mockDataSourceId, mockMode);
     expect(window.location.href).toBe(expectedUrl);
 
     window.location = originalLocation;

diff --git a/public/components/trace_analytics/components/services/service_view.tsx b/public/components/trace_analytics/components/services/service_view.tsx
@@ -144,7 +144,7 @@ export function ServiceView(props: ServiceViewProps) {
   }, [props.serviceName, props.setDataSourceMenuSelectable]);
 
   const redirectToServicePage = (service: string) => {
-    window.location.href = generateServiceUrl(service, props.dataSourceMDSId[0].id);
+    window.location.href = generateServiceUrl(service, props.dataSourceMDSId[0].id, mode);
   };
 
   const onClickConnectedService = (service: string) => {