Skip to content

Commit 16bb8d8

Browse files
authored
chore(timeseries): Loosen series comparison conditions (#101249)
A few improvements based on recent findings. 1. Do not compare numeric values of incomplete buckets. This is generally pointless, those buckets don't have all the data, so they change a lot, and we don't really care about differences there 2. Do not re-run comparison on `data` change. This causes huge log spikes in the Logs view, which re-loads the data a lot on live refresh. Instead, just react to loading state changes
1 parent d6959a4 commit 16bb8d8

File tree

1 file changed

+23
-12
lines changed

1 file changed

+23
-12
lines changed

static/app/views/insights/common/queries/useSortedTimeSeries.tsx

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {useEffect, useMemo, useState} from 'react';
2+
import {useEffectEvent} from '@react-aria/utils';
23
import * as Sentry from '@sentry/react';
34
import isEmpty from 'lodash/isEmpty';
45
import isEqualWith from 'lodash/isEqualWith';
@@ -186,26 +187,32 @@ export const useSortedTimeSeries = <
186187
: {};
187188
}, [timeSeriesResult.data]);
188189

189-
useEffect(() => {
190-
if (
191-
isTimeSeriesEndpointComparisonEnabled &&
192-
!result.isFetching &&
193-
!isEmpty(data) &&
194-
!timeSeriesResult.isFetching &&
195-
!isEmpty(otherData)
196-
) {
190+
// We don't want to re-run the comparison every time the `data` changes, since
191+
// `data` might be changed or mutated by the live reload functionality.
192+
// Instead, extract that into an effect event so it doesn't have a dependency
193+
// on `data`.
194+
const compareResponses = useEffectEvent(() => {
195+
if (!isEmpty(data) && !isEmpty(otherData)) {
197196
if (!isEqualWith(data, otherData, comparator)) {
198197
warn(`\`useDiscoverSeries\` found a data difference in responses`, {
199198
statsData: JSON.stringify(data),
200199
timeSeriesData: JSON.stringify(otherData),
201200
});
202201
}
203202
}
203+
});
204+
205+
useEffect(() => {
206+
if (
207+
isTimeSeriesEndpointComparisonEnabled &&
208+
!result.isFetching &&
209+
!timeSeriesResult.isFetching
210+
) {
211+
compareResponses();
212+
}
204213
}, [
205214
isTimeSeriesEndpointComparisonEnabled,
206-
data,
207215
result.isFetching,
208-
otherData,
209216
timeSeriesResult.isFetching,
210217
]);
211218

@@ -370,14 +377,18 @@ const NUMERIC_KEYS: Array<symbol | string | number> = [
370377
function comparator(
371378
valueA: unknown,
372379
valueB: unknown,
373-
key: symbol | string | number | undefined
380+
key: symbol | string | number | undefined,
381+
objA: Record<PropertyKey, unknown>,
382+
objB: Record<PropertyKey, unknown>
374383
) {
375384
// Compare numbers by near equality, which makes the comparison less sensitive to small natural variations in value caused by request sequencing
376385
if (
377386
key &&
378387
NUMERIC_KEYS.includes(key) &&
379388
typeof valueA === 'number' &&
380-
typeof valueB === 'number'
389+
typeof valueB === 'number' &&
390+
!objA?.incomplete &&
391+
!objB?.incomplete
381392
) {
382393
return areNumbersAlmostEqual(valueA, valueB, 5);
383394
}

0 commit comments

Comments
 (0)