[GEN-2178]: fix undefined with potential destinations (#2156)

This pull request includes several changes to the `frontend/webapp` codebase, focusing on improvements to type imports, interface renaming, and code formatting. Improvements to type imports and interface renaming: * [`potential-destinations-list/index.tsx`](diffhunk://#diff-41a63071c7c4c133a8cffb05f29bf703d8aa2891b38a836baa04b3cf3e543fe9L4-R4): Changed the import style for `DestinationTypeItem` to use the `type` keyword. * [`usePotentialDestinations.ts`](diffhunk://#diff-f8f74ac55108e060a066d0719294ae7842bba038561feeb066c0b09e4b40b037L7-R49): Renamed the `DestinationDetails` interface to `PotentialDestination` and updated related code accordingly. Code formatting and property modification: * [`destinations-list/index.tsx`](diffhunk://#diff-0fc3893b30022c16fec91cb64d38fa36de0dc4e8eabbbbb7a3b94701b45a4badL58-R58): Removed the fallback for `supportedSignals` in the `monitors` property of `DestinationsList`. * [`usePotentialDestinations.ts`](diffhunk://#diff-f8f74ac55108e060a066d0719294ae7842bba038561feeb066c0b09e4b40b037L7-R49): Reformatted the code for better readability and consistency, including combining multiple `useQuery` calls into single lines and improving the mapping logic for potential destinations.
odigos-io · Jan 8, 2025 · 0168951 · 0168951
1 parent a1c17d2
commit 0168951
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 79 deletions.
diff --git a/frontend/services/destination_recognition/destination_finder.go b/frontend/services/destination_recognition/destination_finder.go
@@ -2,6 +2,7 @@ package destination_recognition
 
 import (
 	"context"
+	"strings"
 
 	odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
 	"github.com/odigos-io/odigos/common"
@@ -11,8 +12,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-var SupportedDestinationType = []common.DestinationType{common.JaegerDestinationType, common.ElasticsearchDestinationType}
-
 type DestinationDetails struct {
 	Type   common.DestinationType `json:"type"`
 	Fields map[string]string      `json:"fields"`
@@ -25,43 +24,42 @@ type IDestinationFinder interface {
 }
 
 func GetAllPotentialDestinationDetails(ctx context.Context, namespaces []k8s.Namespace, dests *odigosv1.DestinationList) ([]DestinationDetails, error) {
-	var destinationFinder IDestinationFinder
 	var destinationDetails []DestinationDetails
-	var err error
 
 	for _, ns := range namespaces {
-		err = client.ListWithPages(client.DefaultPageSize, kube.DefaultClient.CoreV1().Services(ns.Name).List,
-			ctx, metav1.ListOptions{}, func(services *k8s.ServiceList) error {
-				for _, service := range services.Items {
-					for _, destinationType := range SupportedDestinationType {
-						destinationFinder = getDestinationFinder(destinationType)
-
-						if destinationFinder.isPotentialService(service) {
-							potentialDestination := destinationFinder.fetchDestinationDetails(service)
-
-							if !destinationExist(dests, potentialDestination, destinationFinder) {
-								destinationDetails = append(destinationDetails, potentialDestination)
-							}
-							break
+		err := client.ListWithPages(client.DefaultPageSize, kube.DefaultClient.CoreV1().Services(ns.Name).List, ctx, metav1.ListOptions{},
+			func(svc *k8s.ServiceList) error {
+				for _, service := range svc.Items {
+					df := getDestinationFinder(service.Name)
+
+					if df != nil && df.isPotentialService(service) {
+						pd := df.fetchDestinationDetails(service)
+
+						if !destinationExist(dests, pd, df) {
+							destinationDetails = append(destinationDetails, pd)
 						}
+						break
 					}
 				}
+
 				return nil
-			})
-	}
+			},
+		)
 
-	if err != nil {
-		return nil, err
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	return destinationDetails, nil
 }
 
-func getDestinationFinder(destinationType common.DestinationType) IDestinationFinder {
-	switch destinationType {
-	case common.JaegerDestinationType:
+func getDestinationFinder(serviceName string) IDestinationFinder {
+	if strings.Contains(serviceName, string(common.JaegerDestinationType)) {
 		return &JaegerDestinationFinder{}
-	case common.ElasticsearchDestinationType:
+	}
+
+	if strings.Contains(serviceName, string(common.ElasticsearchDestinationType)) {
 		return &ElasticSearchDestinationFinder{}
 	}
 

diff --git a/...ebapp/containers/main/destinations/add-destination/configured-destinations-list/index.tsx b/...ebapp/containers/main/destinations/add-destination/configured-destinations-list/index.tsx
@@ -19,7 +19,7 @@ const Container = styled.div`
   overflow-y: scroll;
 `;
 
-const ListItem: React.FC<{ item: ConfiguredDestination; isLastItem: boolean }> = ({ item, isLastItem }) => {
+const ListItem: React.FC<{ item: ConfiguredDestination; isLastItem: boolean }> = ({ item, isLastItem, ...props }) => {
   const { removeConfiguredDestination } = useAppStore((state) => state);
   const [deleteWarning, setDeleteWarning] = useState(false);
 
@@ -37,6 +37,7 @@ const ListItem: React.FC<{ item: ConfiguredDestination; isLastItem: boolean }> =
             <TrashIcon />
           </IconButton>
         )}
+        {...props}
       />
 
       <DeleteWarning
@@ -54,8 +55,8 @@ const ListItem: React.FC<{ item: ConfiguredDestination; isLastItem: boolean }> =
 export const ConfiguredDestinationsList: React.FC<{ data: IAppState['configuredDestinations'] }> = ({ data }) => {
   return (
     <Container>
-      {data.map(({ stored }) => (
-        <ListItem key={stored.displayName} item={stored} isLastItem={data.length === 1} />
+      {data.map(({ stored }, idx) => (
+        <ListItem key={`selected-destination-${stored.type}-${idx}`} data-id={`selected-destination-${stored.type}`} item={stored} isLastItem={data.length === 1} />
       ))}
     </Container>
   );

diff --git a/...s/main/destinations/destination-modal/choose-destination-body/destinations-list/index.tsx b/...s/main/destinations/destination-modal/choose-destination-body/destinations-list/index.tsx
@@ -48,16 +48,16 @@ export const DestinationsList: React.FC<DestinationsListProps> = ({ items, setSe
       return (
         <ListsWrapper key={`category-${categoryItem.name}`}>
           <SectionTitle size='small' title={capitalizeFirstLetter(categoryItem.name)} description={categoryItem.description} />
-          {categoryItem.items.map((destinationItem) => (
+          {categoryItem.items.map((item, idx) => (
             <DataTab
-              key={`destination-${destinationItem.type}`}
-              data-id={`destination-${destinationItem.displayName}`}
-              title={destinationItem.displayName}
-              iconSrc={destinationItem.imageUrl}
+              key={`select-destination-${item.type}-${idx}`}
+              data-id={`select-destination-${item.type}`}
+              title={item.displayName}
+              iconSrc={item.imageUrl}
               hoverText='Select'
-              monitors={Object.keys(destinationItem.supportedSignals || {}).filter((signal) => destinationItem.supportedSignals[signal].supported)}
+              monitors={Object.keys(item.supportedSignals).filter((signal) => item.supportedSignals[signal].supported)}
               monitorsWithLabels
-              onClick={() => setSelectedItems(destinationItem)}
+              onClick={() => setSelectedItems(item)}
             />
           ))}
         </ListsWrapper>

diff --git a/...ion-modal/choose-destination-body/destinations-list/potential-destinations-list/index.tsx b/...ion-modal/choose-destination-body/destinations-list/potential-destinations-list/index.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import { OdigosLogo } from '@/assets';
 import styled from 'styled-components';
-import { DestinationTypeItem } from '@/types';
+import type { DestinationTypeItem } from '@/types';
 import { usePotentialDestinations } from '@/hooks';
 import { DataTab, SectionTitle, SkeletonLoader } from '@/reuseable-components';
 
@@ -31,10 +31,10 @@ export const PotentialDestinationsList: React.FC<Props> = ({ setSelectedItems })
       {loading ? (
         <SkeletonLoader size={1} />
       ) : (
-        data.map((item) => (
+        data.map((item, idx) => (
           <DataTab
-            key={`destination-${item.type}`}
-            data-id={`destination-${item.displayName}`}
+            key={`select-potential-destination-${item.type}-${idx}`}
+            data-id={`select-potential-destination-${item.type}`}
             title={item.displayName}
             iconSrc={item.imageUrl}
             hoverText='Select'

diff --git a/frontend/webapp/cypress/constants/index.ts b/frontend/webapp/cypress/constants/index.ts
@@ -27,15 +27,17 @@ export const NAMESPACES = {
 export const SELECTED_ENTITIES = {
   NAMESPACE: NAMESPACES.DEFAULT,
   SOURCE: 'frontend',
-  DESTINATION: 'Jaeger',
+  DESTINATION_TYPE: 'jaeger',
+  DESTINATION_DISPLAY_NAME: 'Jaeger',
   DESTINATION_AUTOFILL_FIELD: 'JAEGER_URL',
+  DESTINATION_AUTOFILL_VALUE: 'jaeger.tracing:4317',
   ACTION: 'PiiMasking',
   INSTRUMENTATION_RULE: 'PayloadCollection',
 };
 
 export const DATA_IDS = {
   SELECT_NAMESPACE: `[data-id=namespace-${SELECTED_ENTITIES.NAMESPACE}]`,
-  SELECT_DESTINATION: `[data-id=destination-${SELECTED_ENTITIES.DESTINATION}]`,
+  SELECT_DESTINATION: `[data-id=select-potential-destination-${SELECTED_ENTITIES.DESTINATION_TYPE}]`,
   SELECT_DESTINATION_AUTOFILL_FIELD: `[data-id=${SELECTED_ENTITIES.DESTINATION_AUTOFILL_FIELD}]`,
 
   ADD_ENTITY: '[data-id=add-entity]',

diff --git a/frontend/webapp/cypress/e2e/02-onboarding.cy.ts b/frontend/webapp/cypress/e2e/02-onboarding.cy.ts
@@ -14,8 +14,8 @@ describe('Onboarding', () => {
     cy.visit(ROUTES.CHOOSE_DESTINATION);
     cy.contains('button', BUTTONS.ADD_DESTINATION).click();
     cy.wait('@gql').then(() => {
-      cy.get(DATA_IDS.SELECT_DESTINATION).contains(SELECTED_ENTITIES.DESTINATION).should('exist').click();
-      expect(DATA_IDS.SELECT_DESTINATION_AUTOFILL_FIELD).to.not.be.empty;
+      cy.get(DATA_IDS.SELECT_DESTINATION).contains(SELECTED_ENTITIES.DESTINATION_DISPLAY_NAME).should('exist').click();
+      cy.get(DATA_IDS.SELECT_DESTINATION_AUTOFILL_FIELD).should('have.value', SELECTED_ENTITIES.DESTINATION_AUTOFILL_VALUE);
     });
   });
 

diff --git a/frontend/webapp/cypress/e2e/04-destinations.cy.ts b/frontend/webapp/cypress/e2e/04-destinations.cy.ts
@@ -18,8 +18,8 @@ describe('Destinations CRUD', () => {
       cy.get(DATA_IDS.ADD_ENTITY).click();
       cy.get(DATA_IDS.ADD_DESTINATION).click();
       cy.get(DATA_IDS.MODAL_ADD_DESTINATION).should('exist');
-      cy.get(DATA_IDS.SELECT_DESTINATION).contains(SELECTED_ENTITIES.DESTINATION).click();
-      expect(DATA_IDS.SELECT_DESTINATION_AUTOFILL_FIELD).to.not.be.empty;
+      cy.get(DATA_IDS.SELECT_DESTINATION).contains(SELECTED_ENTITIES.DESTINATION_DISPLAY_NAME).should('exist').click();
+      cy.get(DATA_IDS.SELECT_DESTINATION_AUTOFILL_FIELD).should('have.value', SELECTED_ENTITIES.DESTINATION_AUTOFILL_VALUE);
       cy.get('button').contains(BUTTONS.DONE).click();
 
       cy.wait('@gql').then(() => {
@@ -35,7 +35,7 @@ describe('Destinations CRUD', () => {
       updateEntity(
         {
           nodeId: DATA_IDS.DESTINATION_NODE,
-          nodeContains: SELECTED_ENTITIES.DESTINATION,
+          nodeContains: SELECTED_ENTITIES.DESTINATION_DISPLAY_NAME,
           fieldKey: DATA_IDS.TITLE,
           fieldValue: TEXTS.UPDATED_NAME,
         },
@@ -58,7 +58,7 @@ describe('Destinations CRUD', () => {
       deleteEntity(
         {
           nodeId: DATA_IDS.DESTINATION_NODE,
-          nodeContains: SELECTED_ENTITIES.DESTINATION,
+          nodeContains: SELECTED_ENTITIES.DESTINATION_DISPLAY_NAME,
           warnModalTitle: TEXTS.DESTINATION_WARN_MODAL_TITLE,
           warnModalNote: TEXTS.DESTINATION_WARN_MODAL_NOTE,
         },

diff --git a/frontend/webapp/hooks/destinations/usePotentialDestinations.ts b/frontend/webapp/hooks/destinations/usePotentialDestinations.ts
@@ -1,56 +1,74 @@
 import { useMemo } from 'react';
 import { safeJsonParse } from '@/utils';
 import { useQuery } from '@apollo/client';
+import { IAppState, useAppStore } from '@/store';
 import { GetDestinationTypesResponse } from '@/types';
 import { GET_DESTINATION_TYPE, GET_POTENTIAL_DESTINATIONS } from '@/graphql';
 
-interface DestinationDetails {
+interface PotentialDestination {
   type: string;
   fields: string;
 }
 
 interface GetPotentialDestinationsData {
-  potentialDestinations: DestinationDetails[];
+  potentialDestinations: PotentialDestination[];
 }
 
+const checkIfConfigured = (configuredDest: IAppState['configuredDestinations'][0], potentialDest: PotentialDestination, autoFilledFields: Record<string, any>) => {
+  const typesMatch = configuredDest.stored.type === potentialDest.type;
+  if (!typesMatch) return false;
+
+  let fieldsMatch = false;
+
+  for (const { key, value } of configuredDest.form.fields) {
+    if (Object.hasOwn(autoFilledFields, key)) {
+      if (autoFilledFields[key] === value) {
+        fieldsMatch = true;
+      } else {
+        fieldsMatch = false;
+        break;
+      }
+    }
+  }
+
+  return fieldsMatch;
+};
+
 export const usePotentialDestinations = () => {
-  const { data: destinationTypesData } =
-    useQuery<GetDestinationTypesResponse>(GET_DESTINATION_TYPE);
-  const { loading, error, data } = useQuery<GetPotentialDestinationsData>(
-    GET_POTENTIAL_DESTINATIONS
-  );
+  const { configuredDestinations } = useAppStore();
+  const { data: { destinationTypes } = {} } = useQuery<GetDestinationTypesResponse>(GET_DESTINATION_TYPE);
+  const { loading, error, data: { potentialDestinations } = {} } = useQuery<GetPotentialDestinationsData>(GET_POTENTIAL_DESTINATIONS);
 
   const mappedPotentialDestinations = useMemo(() => {
-    if (!destinationTypesData || !data) return [];
+    if (!destinationTypes || !potentialDestinations) return [];
 
     // Create a deep copy of destination types to manipulate
-    const destinationTypesCopy = JSON.parse(
-      JSON.stringify(destinationTypesData.destinationTypes.categories)
-    );
+    const categories: GetDestinationTypesResponse['destinationTypes']['categories'] = JSON.parse(JSON.stringify(destinationTypes.categories));
 
     // Map over the potential destinations
-    return data.potentialDestinations.map((destination) => {
-      for (const category of destinationTypesCopy) {
-        const index = category.items.findIndex(
-          (item) => item.type === destination.type
-        );
-        if (index !== -1) {
-          // Spread the matched destination type data into the potential destination
-          const matchedType = category.items[index];
-          category.items.splice(index, 1); // Remove the matched item from destination types
-          return {
-            ...destination,
-            ...matchedType,
-            fields: safeJsonParse<{ [key: string]: string }>(
-              destination.fields,
-              {}
-            ),
-          };
+    return potentialDestinations
+      .map((pd) => {
+        for (const category of categories) {
+          const autoFilledFields = safeJsonParse<{ [key: string]: string }>(pd.fields, {});
+          const alreadyConfigured = !!configuredDestinations.find((cd) => checkIfConfigured(cd, pd, autoFilledFields));
+
+          if (!alreadyConfigured) {
+            const idx = category.items.findIndex((item) => item.type === pd.type);
+
+            if (idx !== -1) {
+              return {
+                // Spread the matched destination type data into the potential destination
+                ...category.items[idx],
+                fields: autoFilledFields,
+              };
+            }
+          }
         }
-      }
-      return destination;
-    });
-  }, [destinationTypesData, data]);
+
+        return null;
+      })
+      .filter((pd) => !!pd);
+  }, [configuredDestinations, destinationTypes, potentialDestinations]);
 
   return {
     loading,