From fb18959eb11e4f1168f08b30a45ab15cf21fc770 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Sun, 15 Dec 2024 18:46:33 -0500 Subject: [PATCH 1/2] feat(explore): Update insufficient samples warning Simplify the designs a little by using the chart footer only instead of an alert on the top of the page. --- static/app/views/explore/charts/index.tsx | 54 ++++++++++++----------- static/app/views/explore/content.tsx | 22 +-------- static/app/views/explore/tables/index.tsx | 2 - 3 files changed, 30 insertions(+), 48 deletions(-) diff --git a/static/app/views/explore/charts/index.tsx b/static/app/views/explore/charts/index.tsx index a529ca74ca590e..ad645f1e7e5155 100644 --- a/static/app/views/explore/charts/index.tsx +++ b/static/app/views/explore/charts/index.tsx @@ -10,7 +10,7 @@ import {CHART_PALETTE} from 'sentry/constants/chartPalette'; import {IconClock, IconGraph} from 'sentry/icons'; import {t, tct} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import type {Confidence, NewQuery} from 'sentry/types/organization'; +import type {NewQuery} from 'sentry/types/organization'; import {defined} from 'sentry/utils'; import {dedupeArray} from 'sentry/utils/dedupeArray'; import EventView from 'sentry/utils/discover/eventView'; @@ -20,7 +20,6 @@ import { prettifyParsedFunction, } from 'sentry/utils/discover/fields'; import {DiscoverDatasets} from 'sentry/utils/discover/types'; -import {formatPercentage} from 'sentry/utils/number/formatPercentage'; import {MutableSearch} from 'sentry/utils/tokenizeSearch'; import usePageFilters from 'sentry/utils/usePageFilters'; import usePrevious from 'sentry/utils/usePrevious'; @@ -50,7 +49,6 @@ import {TOP_EVENTS_LIMIT, useTopEvents} from '../hooks/useTopEvents'; interface ExploreChartsProps { query: string; - setConfidence: Dispatch>; setError: Dispatch>; } @@ -71,7 +69,7 @@ const exploreChartTypeOptions = [ export const EXPLORE_CHART_GROUP = 'explore-charts_group'; -export function ExploreCharts({query, setConfidence, setError}: ExploreChartsProps) { +export function ExploreCharts({query, setError}: ExploreChartsProps) { const dataset = useExploreDataset(); const visualizes = useExploreVisualizes(); const setVisualizes = useSetExploreVisualizes(); @@ -192,13 +190,6 @@ export function ExploreCharts({query, setConfidence, setError}: ExploreChartsPro return 'high'; }, [dataset, timeSeriesResult.data]); - useEffect(() => { - // only update the confidence once the result has loaded - if (!timeSeriesResult.isPending) { - setConfidence(resultConfidence); - } - }, [setConfidence, resultConfidence, timeSeriesResult.isPending]); - useEffect(() => { setError(timeSeriesResult.error?.message ?? ''); }, [setError, timeSeriesResult.error?.message]); @@ -353,24 +344,31 @@ export function ExploreCharts({query, setConfidence, setError}: ExploreChartsPro /> {dataset === DiscoverDatasets.SPANS_EAP_RPC && ( - {defined(extrapolationMetaResults.data?.[0]?.['count_sample()']) && - defined( - extrapolationMetaResults.data?.[0]?.['avg_sample(sampling_rate)'] - ) - ? tct( - '*[sampleCount] samples extrapolated with an average sampling rate of [sampleRate]', - { + {!defined(extrapolationMetaResults.data?.[0]?.['count_sample()']) + ? t('* Extrapolated from \u2026') + : resultConfidence === 'low' + ? tct('* Extrapolated from [insufficientSamples]', { + insufficientSamples: ( + + + {t('insufficient samples')} + + + ), + }) + : tct('* Extrapolated from [sampleCount] samples', { sampleCount: ( ), - sampleRate: formatPercentage( - extrapolationMetaResults.data[0]['avg_sample(sampling_rate)'] - ), - } - ) - : t('foo')} + })} )} @@ -401,7 +399,7 @@ function useExtrapolationMeta({ const discoverQuery: NewQuery = { id: undefined, name: 'Explore - Extrapolation Meta', - fields: ['count_sample()', 'avg_sample(sampling_rate)', 'min(sampling_rate)'], + fields: ['count_sample()', 'min(sampling_rate)'], query: search.formatString(), version: 2, dataset, @@ -455,3 +453,7 @@ const ChartFooter = styled('div')` margin-top: ${space(1.5)}; margin-bottom: 0; `; + +const InsufficientSamples = styled('span')` + text-decoration: underline dotted ${p => p.theme.gray300}; +`; diff --git a/static/app/views/explore/content.tsx b/static/app/views/explore/content.tsx index 347af2582f4f81..5e16a4578fb003 100644 --- a/static/app/views/explore/content.tsx +++ b/static/app/views/explore/content.tsx @@ -21,7 +21,6 @@ import { import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import type {Confidence} from 'sentry/types/organization'; import {DiscoverDatasets} from 'sentry/utils/discover/types'; import { type AggregationKey, @@ -79,7 +78,6 @@ function ExploreContentImpl({}: ExploreContentProps) { }); }, [location, navigate]); - const [confidence, setConfidence] = useState(null); const [chartError, setChartError] = useState(''); const [tableError, setTableError] = useState(''); @@ -114,13 +112,6 @@ function ExploreContentImpl({}: ExploreContentProps) { - {confidence === 'low' && ( - - {t( - 'Your low sample count may impact the accuracy of this extrapolation. Edit your query or increase your sample rate.' - )} - - )} @@ -178,12 +169,8 @@ function ExploreContentImpl({}: ExploreContentProps) { {tableError || chartError} )} - - + + @@ -225,11 +212,6 @@ const Body = styled(Layout.Body)` } `; -const ConfidenceAlert = styled(Alert)` - grid-column: 1/3; - margin: 0; -`; - const TopSection = styled('div')` display: grid; gap: ${space(2)}; diff --git a/static/app/views/explore/tables/index.tsx b/static/app/views/explore/tables/index.tsx index f9bc28a5e178df..6dea9af4487ccf 100644 --- a/static/app/views/explore/tables/index.tsx +++ b/static/app/views/explore/tables/index.tsx @@ -8,7 +8,6 @@ import {TabList, Tabs} from 'sentry/components/tabs'; import {IconTable} from 'sentry/icons/iconTable'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import type {Confidence} from 'sentry/types/organization'; import { useExploreFields, useExploreMode, @@ -23,7 +22,6 @@ import {SpansTable} from 'sentry/views/explore/tables/spansTable'; import {TracesTable} from 'sentry/views/explore/tables/tracesTable/index'; interface ExploreTablesProps { - confidence: Confidence; setError: Dispatch>; } From df7fae5a9206bd2b08e151a0e6108355c10d6df9 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 16 Dec 2024 00:38:47 -0500 Subject: [PATCH 2/2] put back confidence --- static/app/views/explore/charts/index.tsx | 12 ++++++++++-- static/app/views/explore/content.tsx | 10 ++++++++-- static/app/views/explore/tables/index.tsx | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/static/app/views/explore/charts/index.tsx b/static/app/views/explore/charts/index.tsx index ad645f1e7e5155..09af7ff11cb6a2 100644 --- a/static/app/views/explore/charts/index.tsx +++ b/static/app/views/explore/charts/index.tsx @@ -10,7 +10,7 @@ import {CHART_PALETTE} from 'sentry/constants/chartPalette'; import {IconClock, IconGraph} from 'sentry/icons'; import {t, tct} from 'sentry/locale'; import {space} from 'sentry/styles/space'; -import type {NewQuery} from 'sentry/types/organization'; +import type {Confidence, NewQuery} from 'sentry/types/organization'; import {defined} from 'sentry/utils'; import {dedupeArray} from 'sentry/utils/dedupeArray'; import EventView from 'sentry/utils/discover/eventView'; @@ -49,6 +49,7 @@ import {TOP_EVENTS_LIMIT, useTopEvents} from '../hooks/useTopEvents'; interface ExploreChartsProps { query: string; + setConfidence: Dispatch>; setError: Dispatch>; } @@ -69,7 +70,7 @@ const exploreChartTypeOptions = [ export const EXPLORE_CHART_GROUP = 'explore-charts_group'; -export function ExploreCharts({query, setError}: ExploreChartsProps) { +export function ExploreCharts({query, setConfidence, setError}: ExploreChartsProps) { const dataset = useExploreDataset(); const visualizes = useExploreVisualizes(); const setVisualizes = useSetExploreVisualizes(); @@ -190,6 +191,13 @@ export function ExploreCharts({query, setError}: ExploreChartsProps) { return 'high'; }, [dataset, timeSeriesResult.data]); + useEffect(() => { + // only update the confidence once the result has loaded + if (!timeSeriesResult.isPending) { + setConfidence(resultConfidence); + } + }, [setConfidence, resultConfidence, timeSeriesResult.isPending]); + useEffect(() => { setError(timeSeriesResult.error?.message ?? ''); }, [setError, timeSeriesResult.error?.message]); diff --git a/static/app/views/explore/content.tsx b/static/app/views/explore/content.tsx index 5e16a4578fb003..d574f59eff9ccc 100644 --- a/static/app/views/explore/content.tsx +++ b/static/app/views/explore/content.tsx @@ -21,6 +21,7 @@ import { import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; +import type {Confidence} from 'sentry/types/organization'; import {DiscoverDatasets} from 'sentry/utils/discover/types'; import { type AggregationKey, @@ -78,6 +79,7 @@ function ExploreContentImpl({}: ExploreContentProps) { }); }, [location, navigate]); + const [confidence, setConfidence] = useState(null); const [chartError, setChartError] = useState(''); const [tableError, setTableError] = useState(''); @@ -169,8 +171,12 @@ function ExploreContentImpl({}: ExploreContentProps) { {tableError || chartError} )} - - + + diff --git a/static/app/views/explore/tables/index.tsx b/static/app/views/explore/tables/index.tsx index 6dea9af4487ccf..f9bc28a5e178df 100644 --- a/static/app/views/explore/tables/index.tsx +++ b/static/app/views/explore/tables/index.tsx @@ -8,6 +8,7 @@ import {TabList, Tabs} from 'sentry/components/tabs'; import {IconTable} from 'sentry/icons/iconTable'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; +import type {Confidence} from 'sentry/types/organization'; import { useExploreFields, useExploreMode, @@ -22,6 +23,7 @@ import {SpansTable} from 'sentry/views/explore/tables/spansTable'; import {TracesTable} from 'sentry/views/explore/tables/tracesTable/index'; interface ExploreTablesProps { + confidence: Confidence; setError: Dispatch>; }