From 57f96574ff98dd4e535da5c86a5b8261e6f62779 Mon Sep 17 00:00:00 2001 From: Shaheer Kochai Date: Tue, 20 May 2025 19:44:05 +0430 Subject: [PATCH] fix: trace funnel bugfixes and improvements (#7922) * fix: display the inter-step latency type in step metrics table * chore: send latency type with n+1th step + make latency type optional * fix: fetch and format get funnel steps overview metrics * chore: remove dev env check * fix: overall fixes * fix: don't cache validate query + trigger validate on changing error and where clause as well * fix: display the latency type in step overview metrics table + p99_latency to latency * chore: revert dev env check removal (remove after BE changes are merged) * fix: adjust create API response * chore: useLocalStorage custom hook * feat: improve the run funnel flow - for the initial fetch of funnel results, require the user to run the funnel - subsequently change the run funnel button to a refresh button - display loading state while any of the funnel results APIs are being fetched * fix: fix the issue of add step details breaking * fix: refetch funnel details on rename success * fix: redirect 'learn more' to trace funnels docs * fix: handle potential undefined step in latency type calculation * fix: properly handle incomplete steps state * fix: fix the edge case of stale validation state on transitioning from invalid steps to valid steps * fix: remove the side effect from render and move to useEffect --- frontend/src/api/traceFunnels/index.ts | 55 ++++++--- frontend/src/constants/localStorage.ts | 1 + frontend/src/constants/reactQueryKeys.ts | 1 + .../TracesFunnels/useFunnelConfiguration.tsx | 27 ++--- .../hooks/TracesFunnels/useFunnelMetrics.ts | 88 ++++++++++++-- .../src/hooks/TracesFunnels/useFunnels.tsx | 83 ++++++++++--- frontend/src/hooks/useLocalStorage.ts | 83 +++++++++++++ .../AddFunnelStepDetailsModal.tsx | 2 +- .../FunnelConfiguration/InterStepConfig.tsx | 2 +- .../FunnelConfiguration/StepsContent.tsx | 3 +- .../StepsFooter.styles.scss | 16 ++- .../FunnelConfiguration/StepsFooter.tsx | 109 +++++++++++++++--- .../FunnelResults/EmptyFunnelResults.tsx | 2 +- .../FunnelResults/FunnelResults.tsx | 26 ++++- .../FunnelResults/FunnelTopTracesTable.tsx | 17 ++- .../FunnelResults/StepsTransitionMetrics.tsx | 4 +- .../components/FunnelResults/utils.tsx | 2 +- .../pages/TracesFunnelDetails/constants.ts | 2 +- .../src/pages/TracesFunnels/FunnelContext.tsx | 53 +++++++-- .../components/CreateFunnel/CreateFunnel.tsx | 19 ++- .../FunnelsEmptyState/FunnelsEmptyState.tsx | 2 +- .../components/RenameFunnel/RenameFunnel.tsx | 4 + frontend/src/types/api/traceFunnels/index.ts | 2 +- 23 files changed, 488 insertions(+), 115 deletions(-) create mode 100644 frontend/src/hooks/useLocalStorage.ts diff --git a/frontend/src/api/traceFunnels/index.ts b/frontend/src/api/traceFunnels/index.ts index 6790798316..d4c3b8ccca 100644 --- a/frontend/src/api/traceFunnels/index.ts +++ b/frontend/src/api/traceFunnels/index.ts @@ -22,7 +22,7 @@ export const createFunnel = async ( statusCode: 200, error: null, message: 'Funnel created successfully', - payload: response.data, + payload: response.data.data, }; }; @@ -196,7 +196,9 @@ export interface FunnelOverviewResponse { avg_rate: number; conversion_rate: number | null; errors: number; + // TODO(shaheer): remove p99_latency once we have support for latency p99_latency: number; + latency: number; }; }>; } @@ -222,13 +224,6 @@ export const getFunnelOverview = async ( }; }; -export interface SlowTracesPayload { - start_time: number; - end_time: number; - step_a_order: number; - step_b_order: number; -} - export interface SlowTraceData { status: string; data: Array<{ @@ -243,7 +238,7 @@ export interface SlowTraceData { export const getFunnelSlowTraces = async ( funnelId: string, - payload: SlowTracesPayload, + payload: FunnelOverviewPayload, signal?: AbortSignal, ): Promise | ErrorResponse> => { const response = await axios.post( @@ -261,12 +256,6 @@ export const getFunnelSlowTraces = async ( payload: response.data, }; }; -export interface ErrorTracesPayload { - start_time: number; - end_time: number; - step_a_order: number; - step_b_order: number; -} export interface ErrorTraceData { status: string; @@ -282,7 +271,7 @@ export interface ErrorTraceData { export const getFunnelErrorTraces = async ( funnelId: string, - payload: ErrorTracesPayload, + payload: FunnelOverviewPayload, signal?: AbortSignal, ): Promise | ErrorResponse> => { const response: AxiosResponse = await axios.post( @@ -337,3 +326,37 @@ export const getFunnelSteps = async ( payload: response.data, }; }; + +export interface FunnelStepsOverviewPayload { + start_time: number; + end_time: number; + step_start?: number; + step_end?: number; +} + +export interface FunnelStepsOverviewResponse { + status: string; + data: Array<{ + timestamp: string; + data: Record; + }>; +} + +export const getFunnelStepsOverview = async ( + funnelId: string, + payload: FunnelStepsOverviewPayload, + signal?: AbortSignal, +): Promise | ErrorResponse> => { + const response = await axios.post( + `${FUNNELS_BASE_PATH}/${funnelId}/analytics/steps/overview`, + payload, + { signal }, + ); + + return { + statusCode: 200, + error: null, + message: '', + payload: response.data, + }; +}; diff --git a/frontend/src/constants/localStorage.ts b/frontend/src/constants/localStorage.ts index cf9d71a0a1..ee3f1000d6 100644 --- a/frontend/src/constants/localStorage.ts +++ b/frontend/src/constants/localStorage.ts @@ -30,4 +30,5 @@ export enum LOCALSTORAGE { SHOW_EXCEPTIONS_QUICK_FILTERS = 'SHOW_EXCEPTIONS_QUICK_FILTERS', BANNER_DISMISSED = 'BANNER_DISMISSED', QUICK_FILTERS_SETTINGS_ANNOUNCEMENT = 'QUICK_FILTERS_SETTINGS_ANNOUNCEMENT', + UNEXECUTED_FUNNELS = 'UNEXECUTED_FUNNELS', } diff --git a/frontend/src/constants/reactQueryKeys.ts b/frontend/src/constants/reactQueryKeys.ts index 51fba40c47..00721dfb4a 100644 --- a/frontend/src/constants/reactQueryKeys.ts +++ b/frontend/src/constants/reactQueryKeys.ts @@ -75,6 +75,7 @@ export const REACT_QUERY_KEY = { VALIDATE_FUNNEL_STEPS: 'VALIDATE_FUNNEL_STEPS', UPDATE_FUNNEL_STEP_DETAILS: 'UPDATE_FUNNEL_STEP_DETAILS', GET_FUNNEL_OVERVIEW: 'GET_FUNNEL_OVERVIEW', + GET_FUNNEL_STEPS_OVERVIEW: 'GET_FUNNEL_STEPS_OVERVIEW', GET_FUNNEL_SLOW_TRACES: 'GET_FUNNEL_SLOW_TRACES', GET_FUNNEL_ERROR_TRACES: 'GET_FUNNEL_ERROR_TRACES', GET_FUNNEL_STEPS_GRAPH_DATA: 'GET_FUNNEL_STEPS_GRAPH_DATA', diff --git a/frontend/src/hooks/TracesFunnels/useFunnelConfiguration.tsx b/frontend/src/hooks/TracesFunnels/useFunnelConfiguration.tsx index d81200e6b6..5e2c7324b6 100644 --- a/frontend/src/hooks/TracesFunnels/useFunnelConfiguration.tsx +++ b/frontend/src/hooks/TracesFunnels/useFunnelConfiguration.tsx @@ -43,8 +43,7 @@ export default function useFunnelConfiguration({ const { steps, initialSteps, - setHasIncompleteStepFields, - setHasAllEmptyStepFields, + hasIncompleteStepFields, handleRestoreSteps, } = useFunnelContext(); @@ -74,14 +73,16 @@ export default function useFunnelConfiguration({ return !isEqual(normalizedDebouncedSteps, normalizedLastSavedSteps); }, [debouncedSteps]); - const hasStepServiceOrSpanNameChanged = useCallback( + const hasFunnelStepDefinitionsChanged = useCallback( (prevSteps: FunnelStepData[], nextSteps: FunnelStepData[]): boolean => { if (prevSteps.length !== nextSteps.length) return true; return prevSteps.some((step, index) => { const nextStep = nextSteps[index]; return ( step.service_name !== nextStep.service_name || - step.span_name !== nextStep.span_name + step.span_name !== nextStep.span_name || + !isEqual(step.filters, nextStep.filters) || + step.has_errors !== nextStep.has_errors ); }); }, @@ -106,12 +107,7 @@ export default function useFunnelConfiguration({ [funnel.funnel_id, selectedTime], ); useEffect(() => { - // Check if all steps have both service_name and span_name defined - const shouldUpdate = debouncedSteps.every( - (step) => step.service_name !== '' && step.span_name !== '', - ); - - if (hasStepsChanged() && shouldUpdate) { + if (hasStepsChanged() && !hasIncompleteStepFields) { updateStepsMutation.mutate(getUpdatePayload(), { onSuccess: (data) => { const updatedFunnelSteps = data?.payload?.steps; @@ -135,17 +131,10 @@ export default function useFunnelConfiguration({ (step) => step.service_name === '' || step.span_name === '', ); - const hasAllEmptyStepsData = updatedFunnelSteps.every( - (step) => step.service_name === '' && step.span_name === '', - ); - - setHasIncompleteStepFields(hasIncompleteStepFields); - setHasAllEmptyStepFields(hasAllEmptyStepsData); - // Only validate if service_name or span_name changed if ( !hasIncompleteStepFields && - hasStepServiceOrSpanNameChanged(lastValidatedSteps, debouncedSteps) + hasFunnelStepDefinitionsChanged(lastValidatedSteps, debouncedSteps) ) { queryClient.refetchQueries(validateStepsQueryKey); setLastValidatedSteps(debouncedSteps); @@ -171,7 +160,7 @@ export default function useFunnelConfiguration({ }, [ debouncedSteps, getUpdatePayload, - hasStepServiceOrSpanNameChanged, + hasFunnelStepDefinitionsChanged, hasStepsChanged, lastValidatedSteps, queryClient, diff --git a/frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts b/frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts index 53e4c95515..db4a13e446 100644 --- a/frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts +++ b/frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts @@ -2,8 +2,9 @@ import { getYAxisFormattedValue } from 'components/Graph/yAxisConfig'; import { MetricItem } from 'pages/TracesFunnelDetails/components/FunnelResults/FunnelMetricsTable'; import { useFunnelContext } from 'pages/TracesFunnels/FunnelContext'; import { useMemo } from 'react'; +import { LatencyOptions } from 'types/api/traceFunnels'; -import { useFunnelOverview } from './useFunnels'; +import { useFunnelOverview, useFunnelStepsOverview } from './useFunnels'; interface FunnelMetricsParams { funnelId: string; @@ -13,8 +14,6 @@ interface FunnelMetricsParams { export function useFunnelMetrics({ funnelId, - stepStart, - stepEnd, }: FunnelMetricsParams): { isLoading: boolean; isError: boolean; @@ -25,8 +24,6 @@ export function useFunnelMetrics({ const payload = { start_time: startTime, end_time: endTime, - ...(stepStart !== undefined && { step_start: stepStart }), - ...(stepEnd !== undefined && { step_end: stepEnd }), }; const { @@ -48,14 +45,18 @@ export function useFunnelMetrics({ { title: 'Errors', value: sourceData.errors }, { title: 'Avg. Duration', - value: getYAxisFormattedValue(sourceData.avg_duration.toString(), 'ns'), + value: getYAxisFormattedValue(sourceData.avg_duration.toString(), 'ms'), }, { - title: 'P99 Latency', - value: getYAxisFormattedValue(sourceData.p99_latency.toString(), 'ns'), + title: `P99 Latency`, + value: getYAxisFormattedValue( + // TODO(shaheer): remove p99_latency once we have support for latency + (sourceData.latency ?? sourceData.p99_latency).toString(), + 'ms', + ), }, ]; - }, [overviewData]); + }, [overviewData?.payload?.data]); const conversionRate = overviewData?.payload?.data?.[0]?.data?.conversion_rate ?? 0; @@ -67,3 +68,72 @@ export function useFunnelMetrics({ conversionRate, }; } +export function useFunnelStepsMetrics({ + funnelId, + stepStart, + stepEnd, +}: FunnelMetricsParams): { + isLoading: boolean; + isError: boolean; + metricsData: MetricItem[]; + conversionRate: number; +} { + const { startTime, endTime, steps } = useFunnelContext(); + + const payload = { + start_time: startTime, + end_time: endTime, + step_start: stepStart, + step_end: stepEnd, + }; + + const { + data: stepsOverviewData, + isLoading, + isFetching, + isError, + } = useFunnelStepsOverview(funnelId, payload); + + const latencyType = useMemo( + () => (stepStart ? steps[stepStart]?.latency_type : LatencyOptions.P99), + [stepStart, steps], + ); + + const metricsData = useMemo(() => { + const sourceData = stepsOverviewData?.payload?.data?.[0]?.data; + if (!sourceData) return []; + + return [ + { + title: 'Avg. Rate', + value: `${Number(sourceData.avg_rate.toFixed(2))} req/s`, + }, + { title: 'Errors', value: sourceData.errors }, + { + title: 'Avg. Duration', + value: getYAxisFormattedValue( + (sourceData.avg_duration * 1_000_000).toString(), + 'ns', + ), + }, + { + title: `${latencyType?.toUpperCase()} Latency`, + value: getYAxisFormattedValue( + // TODO(shaheer): remove p99_latency once we have support for latency + ((sourceData.latency ?? sourceData.p99_latency) * 1_000_000).toString(), + 'ns', + ), + }, + ]; + }, [stepsOverviewData, latencyType]); + + const conversionRate = + stepsOverviewData?.payload?.data?.[0]?.data?.conversion_rate ?? 0; + + return { + isLoading: isLoading || isFetching, + isError, + metricsData, + conversionRate, + }; +} diff --git a/frontend/src/hooks/TracesFunnels/useFunnels.tsx b/frontend/src/hooks/TracesFunnels/useFunnels.tsx index 73a8428931..7f85619a88 100644 --- a/frontend/src/hooks/TracesFunnels/useFunnels.tsx +++ b/frontend/src/hooks/TracesFunnels/useFunnels.tsx @@ -3,9 +3,10 @@ import { createFunnel, deleteFunnel, ErrorTraceData, - ErrorTracesPayload, FunnelOverviewPayload, FunnelOverviewResponse, + FunnelStepsOverviewPayload, + FunnelStepsOverviewResponse, FunnelStepsResponse, getFunnelById, getFunnelErrorTraces, @@ -13,11 +14,11 @@ import { getFunnelsList, getFunnelSlowTraces, getFunnelSteps, + getFunnelStepsOverview, renameFunnel, RenameFunnelPayload, saveFunnelDescription, SlowTraceData, - SlowTracesPayload, updateFunnelSteps, UpdateFunnelStepsPayload, ValidateFunnelResponse, @@ -115,11 +116,13 @@ export const useValidateFunnelSteps = ({ selectedTime, startTime, endTime, + enabled, }: { funnelId: string; selectedTime: string; startTime: number; endTime: number; + enabled: boolean; }): UseQueryResult< SuccessResponse | ErrorResponse, Error @@ -132,8 +135,8 @@ export const useValidateFunnelSteps = ({ signal, ), queryKey: [REACT_QUERY_KEY.VALIDATE_FUNNEL_STEPS, funnelId, selectedTime], - enabled: !!funnelId && !!selectedTime && !!startTime && !!endTime, - staleTime: 1000 * 60 * 5, + enabled, + staleTime: 0, }); interface SaveFunnelDescriptionPayload { @@ -157,7 +160,11 @@ export const useFunnelOverview = ( SuccessResponse | ErrorResponse, Error > => { - const { selectedTime, validTracesCount } = useFunnelContext(); + const { + selectedTime, + validTracesCount, + hasFunnelBeenExecuted, + } = useFunnelContext(); return useQuery({ queryFn: ({ signal }) => getFunnelOverview(funnelId, payload, signal), queryKey: [ @@ -167,31 +174,51 @@ export const useFunnelOverview = ( payload.step_start ?? '', payload.step_end ?? '', ], - enabled: !!funnelId && validTracesCount > 0, + enabled: !!funnelId && validTracesCount > 0 && hasFunnelBeenExecuted, }); }; export const useFunnelSlowTraces = ( funnelId: string, - payload: SlowTracesPayload, + payload: FunnelOverviewPayload, ): UseQueryResult | ErrorResponse, Error> => { - const { selectedTime, validTracesCount } = useFunnelContext(); + const { + selectedTime, + validTracesCount, + hasFunnelBeenExecuted, + } = useFunnelContext(); return useQuery | ErrorResponse, Error>({ queryFn: ({ signal }) => getFunnelSlowTraces(funnelId, payload, signal), - queryKey: [REACT_QUERY_KEY.GET_FUNNEL_SLOW_TRACES, funnelId, selectedTime], - enabled: !!funnelId && validTracesCount > 0, + queryKey: [ + REACT_QUERY_KEY.GET_FUNNEL_SLOW_TRACES, + funnelId, + selectedTime, + payload.step_start ?? '', + payload.step_end ?? '', + ], + enabled: !!funnelId && validTracesCount > 0 && hasFunnelBeenExecuted, }); }; export const useFunnelErrorTraces = ( funnelId: string, - payload: ErrorTracesPayload, + payload: FunnelOverviewPayload, ): UseQueryResult | ErrorResponse, Error> => { - const { selectedTime, validTracesCount } = useFunnelContext(); + const { + selectedTime, + validTracesCount, + hasFunnelBeenExecuted, + } = useFunnelContext(); return useQuery({ queryFn: ({ signal }) => getFunnelErrorTraces(funnelId, payload, signal), - queryKey: [REACT_QUERY_KEY.GET_FUNNEL_ERROR_TRACES, funnelId, selectedTime], - enabled: !!funnelId && validTracesCount > 0, + queryKey: [ + REACT_QUERY_KEY.GET_FUNNEL_ERROR_TRACES, + funnelId, + selectedTime, + payload.step_start ?? '', + payload.step_end ?? '', + ], + enabled: !!funnelId && validTracesCount > 0 && hasFunnelBeenExecuted, }); }; @@ -203,6 +230,7 @@ export function useFunnelStepsGraphData( endTime, selectedTime, validTracesCount, + hasFunnelBeenExecuted, } = useFunnelContext(); return useQuery({ @@ -217,6 +245,31 @@ export function useFunnelStepsGraphData( funnelId, selectedTime, ], - enabled: !!funnelId && validTracesCount > 0, + enabled: !!funnelId && validTracesCount > 0 && hasFunnelBeenExecuted, }); } + +export const useFunnelStepsOverview = ( + funnelId: string, + payload: FunnelStepsOverviewPayload, +): UseQueryResult< + SuccessResponse | ErrorResponse, + Error +> => { + const { + selectedTime, + validTracesCount, + hasFunnelBeenExecuted, + } = useFunnelContext(); + return useQuery({ + queryFn: ({ signal }) => getFunnelStepsOverview(funnelId, payload, signal), + queryKey: [ + REACT_QUERY_KEY.GET_FUNNEL_STEPS_OVERVIEW, + funnelId, + selectedTime, + payload.step_start ?? '', + payload.step_end ?? '', + ], + enabled: !!funnelId && validTracesCount > 0 && hasFunnelBeenExecuted, + }); +}; diff --git a/frontend/src/hooks/useLocalStorage.ts b/frontend/src/hooks/useLocalStorage.ts new file mode 100644 index 0000000000..f71e2676ba --- /dev/null +++ b/frontend/src/hooks/useLocalStorage.ts @@ -0,0 +1,83 @@ +import { useCallback, useEffect, useState } from 'react'; + +/** + * A React hook for interacting with localStorage. + * It allows getting, setting, and removing items from localStorage. + * + * @template T The type of the value to be stored. + * @param {string} key The localStorage key. + * @param {T | (() => T)} initialValue The initial value to use if no value is found in localStorage, + * @returns {[T, (value: T | ((prevState: T) => T)) => void, () => void]} + * A tuple containing: + * - The current value from state (and localStorage). + * - A function to set the value (updates state and localStorage). + * - A function to remove the value from localStorage and reset state to initialValue. + */ +export function useLocalStorage( + key: string, + initialValue: T | (() => T), +): [T, (value: T | ((prevState: T) => T)) => void, () => void] { + // This function resolves the initialValue if it's a function, + // and handles potential errors during localStorage access or JSON parsing. + const readValueFromStorage = useCallback((): T => { + const resolvedInitialValue = + initialValue instanceof Function ? initialValue() : initialValue; + + try { + const item = window.localStorage.getItem(key); + // If item exists, parse it, otherwise return the resolved initial value. + if (item) { + return JSON.parse(item) as T; + } + } catch (error) { + // Log error and fall back to initial value if reading/parsing fails. + console.warn(`Error reading localStorage key "${key}":`, error); + } + return resolvedInitialValue; + }, [key, initialValue]); + + // Initialize state by reading from localStorage. + const [storedValue, setStoredValue] = useState(readValueFromStorage); + + // This function updates both localStorage and the React state. + const setValue = useCallback( + (value: T | ((prevState: T) => T)) => { + try { + // If a function is passed to setValue, it receives the latest value from storage. + const latestValueFromStorage = readValueFromStorage(); + const valueToStore = + value instanceof Function ? value(latestValueFromStorage) : value; + + // Save to localStorage. + window.localStorage.setItem(key, JSON.stringify(valueToStore)); + // Update React state. + setStoredValue(valueToStore); + } catch (error) { + console.warn(`Error setting localStorage key "${key}":`, error); + } + }, + [key, readValueFromStorage], + ); + + // This function removes the item from localStorage and resets the React state. + const removeValue = useCallback(() => { + try { + window.localStorage.removeItem(key); + // Reset state to the (potentially resolved) initialValue. + setStoredValue( + initialValue instanceof Function ? initialValue() : initialValue, + ); + } catch (error) { + console.warn(`Error removing localStorage key "${key}":`, error); + } + }, [key, initialValue]); + + // useEffect to update the storedValue if the key changes, + // or if the initialValue prop changes causing readValueFromStorage to change. + // This ensures the hook reflects the correct localStorage item if its key prop dynamically changes. + useEffect(() => { + setStoredValue(readValueFromStorage()); + }, [key, readValueFromStorage]); // Re-run if key or the read function changes. + + return [storedValue, setValue, removeValue]; +} diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/AddFunnelStepDetailsModal.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/AddFunnelStepDetailsModal.tsx index 9ee93601d2..73f1fa45a5 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/AddFunnelStepDetailsModal.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/AddFunnelStepDetailsModal.tsx @@ -39,7 +39,7 @@ function AddFunnelStepDetailsModal({ setStepName(stepData?.name || ''); setDescription(stepData?.description || ''); } - }, [isOpen, stepData]); + }, [isOpen, stepData?.name, stepData?.description]); const handleCancel = (): void => { setStepName(''); diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/InterStepConfig.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/InterStepConfig.tsx index 9ff0a50697..6490a1f1b9 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/InterStepConfig.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/InterStepConfig.tsx @@ -26,7 +26,7 @@ function InterStepConfig({
onStepChange(index, { diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsContent.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsContent.tsx index 64eecb1549..7e6a18ddc1 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsContent.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsContent.tsx @@ -65,7 +65,8 @@ function StepsContent({
{/* Display InterStepConfig only between steps */} {index < steps.length - 1 && ( - + // the latency type should be sent with the n+1th step + )} } diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.styles.scss b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.styles.scss index 0191085a91..2dc6569851 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.styles.scss +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.styles.scss @@ -34,12 +34,16 @@ border: none; display: flex; align-items: center; - gap: 6px; - .ant-btn-icon { - margin-inline-end: 0 !important; - } - &--save { - background-color: var(--bg-slate-400); + font-size: 12px; + font-weight: 500; + line-height: 10px; /* 83.333% */ + letter-spacing: 0.12px; + border-radius: 2px; + + &--sync { + border: 1px solid var(--bg-slate-400); + background: var(--bg-ink-300); + color: var(--bg-vanilla-400); } &--run { background-color: var(--bg-robin-500); diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.tsx index 30b6e53e47..9aa70f2db8 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsFooter.tsx @@ -1,8 +1,49 @@ import './StepsFooter.styles.scss'; import { Button, Skeleton } from 'antd'; -import { Cone, Play } from 'lucide-react'; +import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; +import { Cone, Play, RefreshCcw } from 'lucide-react'; import { useFunnelContext } from 'pages/TracesFunnels/FunnelContext'; +import { useEffect, useMemo } from 'react'; +import { useIsFetching, useQueryClient } from 'react-query'; + +const useFunnelResultsLoading = (): boolean => { + const { funnelId } = useFunnelContext(); + + const isFetchingFunnelOverview = useIsFetching({ + queryKey: [REACT_QUERY_KEY.GET_FUNNEL_OVERVIEW, funnelId], + }); + + const isFetchingStepsGraphData = useIsFetching({ + queryKey: [REACT_QUERY_KEY.GET_FUNNEL_STEPS_GRAPH_DATA, funnelId], + }); + + const isFetchingErrorTraces = useIsFetching({ + queryKey: [REACT_QUERY_KEY.GET_FUNNEL_ERROR_TRACES, funnelId], + }); + + const isFetchingSlowTraces = useIsFetching({ + queryKey: [REACT_QUERY_KEY.GET_FUNNEL_SLOW_TRACES, funnelId], + }); + + return useMemo(() => { + if (!funnelId) { + return false; + } + return ( + !!isFetchingFunnelOverview || + !!isFetchingStepsGraphData || + !!isFetchingErrorTraces || + !!isFetchingSlowTraces + ); + }, [ + funnelId, + isFetchingFunnelOverview, + isFetchingStepsGraphData, + isFetchingErrorTraces, + isFetchingSlowTraces, + ]); +}; interface StepsFooterProps { stepsCount: number; @@ -14,10 +55,27 @@ function ValidTracesCount(): JSX.Element { isValidateStepsLoading, hasIncompleteStepFields, validTracesCount, + funnelId, + selectedTime, } = useFunnelContext(); - if (isValidateStepsLoading) { - return ; - } + const queryClient = useQueryClient(); + const validationQueryKey = useMemo( + () => [REACT_QUERY_KEY.VALIDATE_FUNNEL_STEPS, funnelId, selectedTime], + [funnelId, selectedTime], + ); + const validationStatus = queryClient.getQueryData(validationQueryKey); + + useEffect(() => { + // Show loading state immediately when fields become valid + if (hasIncompleteStepFields && validationStatus !== 'pending') { + queryClient.setQueryData(validationQueryKey, 'pending'); + } + }, [ + hasIncompleteStepFields, + queryClient, + validationQueryKey, + validationStatus, + ]); if (hasAllEmptyStepFields) { return ( @@ -33,6 +91,10 @@ function ValidTracesCount(): JSX.Element { ); } + if (isValidateStepsLoading || validationStatus === 'pending') { + return ; + } + if (validTracesCount === 0) { return ( @@ -45,7 +107,13 @@ function ValidTracesCount(): JSX.Element { } function StepsFooter({ stepsCount }: StepsFooterProps): JSX.Element { - const { validTracesCount, handleRunFunnel } = useFunnelContext(); + const { + validTracesCount, + handleRunFunnel, + hasFunnelBeenExecuted, + } = useFunnelContext(); + + const isFunnelResultsLoading = useFunnelResultsLoading(); return (
@@ -56,15 +124,28 @@ function StepsFooter({ stepsCount }: StepsFooterProps): JSX.Element {
- + {!hasFunnelBeenExecuted ? ( + + ) : ( + + )}
); diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/EmptyFunnelResults.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/EmptyFunnelResults.tsx index baaea42cbc..5eeb02226c 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/EmptyFunnelResults.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/EmptyFunnelResults.tsx @@ -18,7 +18,7 @@ function EmptyFunnelResults({
{title}
{description}
- +
diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelResults.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelResults.tsx index 50c45fe9d2..8c47d961e5 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelResults.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelResults.tsx @@ -1,7 +1,9 @@ import './FunnelResults.styles.scss'; import Spinner from 'components/Spinner'; +import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; import { useFunnelContext } from 'pages/TracesFunnels/FunnelContext'; +import { useQueryClient } from 'react-query'; import EmptyFunnelResults from './EmptyFunnelResults'; import FunnelGraph from './FunnelGraph'; @@ -14,11 +16,17 @@ function FunnelResults(): JSX.Element { isValidateStepsLoading, hasIncompleteStepFields, hasAllEmptyStepFields, + hasFunnelBeenExecuted, + funnelId, + selectedTime, } = useFunnelContext(); + const queryClient = useQueryClient(); - if (isValidateStepsLoading) { - return ; - } + const validateQueryData = queryClient.getQueryData([ + REACT_QUERY_KEY.VALIDATE_FUNNEL_STEPS, + funnelId, + selectedTime, + ]); if (hasAllEmptyStepFields) return ; @@ -30,6 +38,10 @@ function FunnelResults(): JSX.Element { /> ); + if (isValidateStepsLoading || validateQueryData === 'pending') { + return ; + } + if (validTracesCount === 0) { return ( ); } + if (!hasFunnelBeenExecuted) { + return ( + + ); + } return (
diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelTopTracesTable.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelTopTracesTable.tsx index 64973fb5f3..51797a13dc 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelTopTracesTable.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/FunnelTopTracesTable.tsx @@ -1,4 +1,8 @@ -import { ErrorTraceData, SlowTraceData } from 'api/traceFunnels'; +import { + ErrorTraceData, + FunnelOverviewPayload, + SlowTraceData, +} from 'api/traceFunnels'; import { useFunnelContext } from 'pages/TracesFunnels/FunnelContext'; import { useMemo } from 'react'; import { UseQueryResult } from 'react-query'; @@ -15,12 +19,7 @@ interface FunnelTopTracesTableProps { tooltip: string; useQueryHook: ( funnelId: string, - payload: { - start_time: number; - end_time: number; - step_a_order: number; - step_b_order: number; - }, + payload: FunnelOverviewPayload, ) => UseQueryResult< SuccessResponse | ErrorResponse, Error @@ -40,8 +39,8 @@ function FunnelTopTracesTable({ () => ({ start_time: startTime, end_time: endTime, - step_a_order: stepAOrder, - step_b_order: stepBOrder, + step_start: stepAOrder, + step_end: stepBOrder, }), [startTime, endTime, stepAOrder, stepBOrder], ); diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/StepsTransitionMetrics.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/StepsTransitionMetrics.tsx index 6e9d9791ed..8e538f90d5 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/StepsTransitionMetrics.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/StepsTransitionMetrics.tsx @@ -1,4 +1,4 @@ -import { useFunnelMetrics } from 'hooks/TracesFunnels/useFunnelMetrics'; +import { useFunnelStepsMetrics } from 'hooks/TracesFunnels/useFunnelMetrics'; import { useParams } from 'react-router-dom'; import FunnelMetricsTable from './FunnelMetricsTable'; @@ -22,7 +22,7 @@ function StepsTransitionMetrics({ (transition) => transition.value === selectedTransition, ); - const { isLoading, metricsData, conversionRate } = useFunnelMetrics({ + const { isLoading, metricsData, conversionRate } = useFunnelStepsMetrics({ funnelId: funnelId || '', stepStart: startStep, stepEnd: endStep, diff --git a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/utils.tsx b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/utils.tsx index 8d3bee421e..6b4cbb3dd3 100644 --- a/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/utils.tsx +++ b/frontend/src/pages/TracesFunnelDetails/components/FunnelResults/utils.tsx @@ -13,7 +13,7 @@ export const topTracesTableColumns = [ ), }, { - title: 'DURATION', + title: 'STEP TRANSITION DURATION', dataIndex: 'duration_ms', key: 'duration_ms', render: (value: string): string => getYAxisFormattedValue(value, 'ms'), diff --git a/frontend/src/pages/TracesFunnelDetails/constants.ts b/frontend/src/pages/TracesFunnelDetails/constants.ts index 3c7016549c..4cad0b3131 100644 --- a/frontend/src/pages/TracesFunnelDetails/constants.ts +++ b/frontend/src/pages/TracesFunnelDetails/constants.ts @@ -12,7 +12,7 @@ export const initialStepsData: FunnelStepData[] = [ op: 'and', }, latency_pointer: 'start', - latency_type: LatencyOptions.P95, + latency_type: undefined, has_errors: false, name: '', description: '', diff --git a/frontend/src/pages/TracesFunnels/FunnelContext.tsx b/frontend/src/pages/TracesFunnels/FunnelContext.tsx index 10d47779af..96d5a05f21 100644 --- a/frontend/src/pages/TracesFunnels/FunnelContext.tsx +++ b/frontend/src/pages/TracesFunnels/FunnelContext.tsx @@ -1,5 +1,6 @@ import logEvent from 'api/common/logEvent'; import { ValidateFunnelResponse } from 'api/traceFunnels'; +import { LOCALSTORAGE } from 'constants/localStorage'; import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; import { Time } from 'container/TopNav/DateTimeSelection/config'; import { @@ -7,6 +8,7 @@ import { Time as TimeV2, } from 'container/TopNav/DateTimeSelectionV2/config'; import { useValidateFunnelSteps } from 'hooks/TracesFunnels/useFunnels'; +import { useLocalStorage } from 'hooks/useLocalStorage'; import getStartEndRangeTime from 'lib/getStartEndRangeTime'; import { initialStepsData } from 'pages/TracesFunnelDetails/constants'; import { @@ -45,15 +47,15 @@ interface FunnelContextType { | undefined; isValidateStepsLoading: boolean; hasIncompleteStepFields: boolean; - setHasIncompleteStepFields: Dispatch>; hasAllEmptyStepFields: boolean; - setHasAllEmptyStepFields: Dispatch>; handleReplaceStep: ( index: number, serviceName: string, spanName: string, ) => void; handleRestoreSteps: (oldSteps: FunnelStepData[]) => void; + hasFunnelBeenExecuted: boolean; + setHasFunnelBeenExecuted: Dispatch>; } const FunnelContext = createContext(undefined); @@ -84,12 +86,27 @@ export function FunnelProvider({ const funnel = data?.payload; const initialSteps = funnel?.steps?.length ? funnel.steps : initialStepsData; const [steps, setSteps] = useState(initialSteps); - const [hasIncompleteStepFields, setHasIncompleteStepFields] = useState( - steps.some((step) => step.service_name === '' || step.span_name === ''), + const { hasIncompleteStepFields, hasAllEmptyStepFields } = useMemo( + () => ({ + hasAllEmptyStepFields: steps.every( + (step) => step.service_name === '' && step.span_name === '', + ), + hasIncompleteStepFields: steps.some( + (step) => step.service_name === '' || step.span_name === '', + ), + }), + [steps], ); - const [hasAllEmptyStepFields, setHasAllEmptyStepFields] = useState( - steps.every((step) => step.service_name === '' && step.span_name === ''), + + const [unexecutedFunnels, setUnexecutedFunnels] = useLocalStorage( + LOCALSTORAGE.UNEXECUTED_FUNNELS, + [], ); + + const [hasFunnelBeenExecuted, setHasFunnelBeenExecuted] = useState( + !unexecutedFunnels.includes(funnelId), + ); + const { data: validationResponse, isLoading: isValidationLoading, @@ -99,6 +116,7 @@ export function FunnelProvider({ selectedTime, startTime, endTime, + enabled: !!funnelId && !!selectedTime && !!startTime && !!endTime, }); const validTracesCount = useMemo( @@ -163,6 +181,11 @@ export function FunnelProvider({ const handleRunFunnel = useCallback(async (): Promise => { if (validTracesCount === 0) return; + if (!hasFunnelBeenExecuted) { + setUnexecutedFunnels(unexecutedFunnels.filter((id) => id !== funnelId)); + + setHasFunnelBeenExecuted(true); + } queryClient.refetchQueries([ REACT_QUERY_KEY.GET_FUNNEL_OVERVIEW, funnelId, @@ -183,7 +206,15 @@ export function FunnelProvider({ funnelId, selectedTime, ]); - }, [funnelId, queryClient, selectedTime, validTracesCount]); + }, [ + funnelId, + hasFunnelBeenExecuted, + unexecutedFunnels, + queryClient, + selectedTime, + setUnexecutedFunnels, + validTracesCount, + ]); const value = useMemo( () => ({ @@ -202,11 +233,11 @@ export function FunnelProvider({ validationResponse, isValidateStepsLoading: isValidationLoading || isValidationFetching, hasIncompleteStepFields, - setHasIncompleteStepFields, hasAllEmptyStepFields, - setHasAllEmptyStepFields, handleReplaceStep, handleRestoreSteps, + hasFunnelBeenExecuted, + setHasFunnelBeenExecuted, }), [ funnelId, @@ -224,11 +255,11 @@ export function FunnelProvider({ isValidationLoading, isValidationFetching, hasIncompleteStepFields, - setHasIncompleteStepFields, hasAllEmptyStepFields, - setHasAllEmptyStepFields, handleReplaceStep, handleRestoreSteps, + hasFunnelBeenExecuted, + setHasFunnelBeenExecuted, ], ); diff --git a/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx b/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx index f6f8b1fbd6..d5c5c4a261 100644 --- a/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx +++ b/frontend/src/pages/TracesFunnels/components/CreateFunnel/CreateFunnel.tsx @@ -4,9 +4,11 @@ import { Input } from 'antd'; import logEvent from 'api/common/logEvent'; import { AxiosError } from 'axios'; import SignozModal from 'components/SignozModal/SignozModal'; +import { LOCALSTORAGE } from 'constants/localStorage'; import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; import ROUTES from 'constants/routes'; import { useCreateFunnel } from 'hooks/TracesFunnels/useFunnels'; +import { useLocalStorage } from 'hooks/useLocalStorage'; import { useNotifications } from 'hooks/useNotifications'; import { useSafeNavigate } from 'hooks/useSafeNavigate'; import { Check, X } from 'lucide-react'; @@ -32,6 +34,11 @@ function CreateFunnel({ const { safeNavigate } = useSafeNavigate(); const { pathname } = useLocation(); + const [unexecutedFunnels, setUnexecutedFunnels] = useLocalStorage( + LOCALSTORAGE.UNEXECUTED_FUNNELS, + [], + ); + const handleCreate = (): void => { createFunnelMutation.mutate( { @@ -52,11 +59,17 @@ function CreateFunnel({ setFunnelName(''); queryClient.invalidateQueries([REACT_QUERY_KEY.GET_FUNNELS_LIST]); - onClose(data?.payload?.funnel_id); - if (data?.payload?.funnel_id && redirectToDetails) { + + const funnelId = data?.payload?.funnel_id; + if (funnelId) { + setUnexecutedFunnels([...unexecutedFunnels, funnelId]); + } + + onClose(funnelId); + if (funnelId && redirectToDetails) { safeNavigate( generatePath(ROUTES.TRACES_FUNNELS_DETAIL, { - funnelId: data.payload.funnel_id, + funnelId, }), ); } diff --git a/frontend/src/pages/TracesFunnels/components/FunnelsEmptyState/FunnelsEmptyState.tsx b/frontend/src/pages/TracesFunnels/components/FunnelsEmptyState/FunnelsEmptyState.tsx index e34e02bd1f..f6a50ab892 100644 --- a/frontend/src/pages/TracesFunnels/components/FunnelsEmptyState/FunnelsEmptyState.tsx +++ b/frontend/src/pages/TracesFunnels/components/FunnelsEmptyState/FunnelsEmptyState.tsx @@ -37,7 +37,7 @@ function FunnelsEmptyState({ > New funnel - +
diff --git a/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.tsx b/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.tsx index b069c79935..63bb4acebc 100644 --- a/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.tsx +++ b/frontend/src/pages/TracesFunnels/components/RenameFunnel/RenameFunnel.tsx @@ -40,6 +40,10 @@ function RenameFunnel({ message: 'Funnel renamed successfully', }); queryClient.invalidateQueries([REACT_QUERY_KEY.GET_FUNNELS_LIST]); + queryClient.invalidateQueries([ + REACT_QUERY_KEY.GET_FUNNEL_DETAILS, + funnelId, + ]); onClose(); }, onError: () => { diff --git a/frontend/src/types/api/traceFunnels/index.ts b/frontend/src/types/api/traceFunnels/index.ts index 2a713d6175..a422d9ff64 100644 --- a/frontend/src/types/api/traceFunnels/index.ts +++ b/frontend/src/types/api/traceFunnels/index.ts @@ -14,7 +14,7 @@ export interface FunnelStepData { span_name: string; filters: TagFilter; latency_pointer: 'start' | 'end'; - latency_type: LatencyOptionsType; + latency_type?: LatencyOptionsType; has_errors: boolean; name?: string; description?: string;