Skip to content

Commit b58c52e

Browse files
pulpdrewpulpdrew
authored
fix: Fix Services Dashboard bugs (#1484)
Closes HDX-3033 # Summary This PR fixes three bugs in the Services Dashboard 1. When using CTEs in chart configs, as we do on the HTTP and Databases tabs, there were frequent console errors as we tried to `DESCRIBE` the CTE names, to support the materialized columns optimization. With this PR, we no longer try to DESCRIBE CTEs, by skipping the materialized column optimization for configs without a `from.databaseName`. 2. Previously, the Request Throughput chart would reload whenever switching the Request Error Rate chart from `Overall` to `By Endpoint`. This was because the `displayType` in the Request Throughput chart was based on the toggle state, despite being unrelated. Now, the displayType of the Request Throughput chart is constant, eliminating the extra refetch. 3. Previously, when switching to the Services dashboard with a non-Trace Source ID in the URL params, the Services dashboard would initially be empty, then after toggling to a Trace Source, queries would briefly be issued against the non-Trace source (they would fail and/or be cancelled a moment later). Now, non-Trace sources are filtered out so that a Trace source is chosen as the default, and non-Trace sources are not queried. 4. Previously, we were spreading the entirety of `...source` into each config, which resulted in `metricTables` being in the config under particular circumstances (HDX-3035), which in turn caused errors from renderChartConfig. This has been fixed by `pick`ing only the fields we need from source.
1 parent 8241ffe commit b58c52e

File tree

8 files changed

+169
-68
lines changed

8 files changed

+169
-68
lines changed

‎.changeset/popular-coins-pretend.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
"@hyperdx/api": patch
4+
"@hyperdx/app": patch
5+
---
6+
7+
fix: Fix bugs in the Services dashboard

‎packages/api/src/clickhouse/__tests__/renderChartConfig.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ describe('renderChartConfig', () => {
145145
{ aggFn: 'sum', valueExpression: 'strVal' },
146146
],
147147
from: {
148-
databaseName: '',
148+
databaseName: DEFAULT_DATABASE,
149149
tableName: `agg_fn_str_test`,
150150
},
151151
where: '',
@@ -183,7 +183,7 @@ describe('renderChartConfig', () => {
183183
{ aggFn: 'sum', valueExpression: 'strVal' },
184184
],
185185
from: {
186-
databaseName: '',
186+
databaseName: DEFAULT_DATABASE,
187187
tableName: `agg_fn_default_test`,
188188
},
189189
where: '',

‎packages/app/src/ServicesDashboardPage.tsx‎

Lines changed: 109 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
useQueryState,
88
useQueryStates,
99
} from 'nuqs';
10-
import { UseControllerProps, useForm } from 'react-hook-form';
10+
import { UseControllerProps, useForm, useWatch } from 'react-hook-form';
1111
import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata';
1212
import { DEFAULT_AUTO_GRANULARITY_MAX_BUCKETS } from '@hyperdx/common-utils/dist/core/renderChartConfig';
1313
import {
@@ -128,7 +128,7 @@ function ServiceSelectControlled({
128128
const { expressions } = useServiceDashboardExpressions({ source });
129129

130130
const queriedConfig = {
131-
...source,
131+
timestampValueExpression: source?.timestampValueExpression || '',
132132
from: {
133133
databaseName: source?.from.databaseName || '',
134134
tableName: source?.from.tableName || '',
@@ -240,7 +240,11 @@ export function EndpointLatencyChart({
240240
'avg_duration_ns',
241241
]}
242242
config={{
243-
...source,
243+
...pick(source, [
244+
'timestampValueExpression',
245+
'connection',
246+
'from',
247+
]),
244248
where: appliedConfig.where || '',
245249
whereLanguage: appliedConfig.whereLanguage || 'sql',
246250
select: [
@@ -289,7 +293,11 @@ export function EndpointLatencyChart({
289293
) : (
290294
<DBHistogramChart
291295
config={{
292-
...source,
296+
...pick(source, [
297+
'timestampValueExpression',
298+
'connection',
299+
'from',
300+
]),
293301
where: appliedConfig.where || '',
294302
whereLanguage: appliedConfig.whereLanguage || 'sql',
295303
select: [
@@ -343,7 +351,7 @@ function HttpTab({
343351
if (!source || !expressions) return null;
344352
if (reqChartType === 'overall') {
345353
return {
346-
...source,
354+
...pick(source, ['timestampValueExpression', 'connection', 'from']),
347355
where: appliedConfig.where || '',
348356
whereLanguage: appliedConfig.whereLanguage || 'sql',
349357
displayType: DisplayType.Line,
@@ -539,13 +547,14 @@ function HttpTab({
539547
<DBTimeChart
540548
sourceId={source.id}
541549
config={{
542-
...source,
550+
...pick(source, [
551+
'timestampValueExpression',
552+
'connection',
553+
'from',
554+
]),
543555
where: appliedConfig.where || '',
544556
whereLanguage: appliedConfig.whereLanguage || 'sql',
545-
displayType:
546-
reqChartType === 'overall'
547-
? DisplayType.Line
548-
: DisplayType.StackedBar,
557+
displayType: DisplayType.Line,
549558
select: [
550559
{
551560
aggFn: 'count' as const,
@@ -582,7 +591,11 @@ function HttpTab({
582591
'error_requests',
583592
]}
584593
config={{
585-
...source,
594+
...pick(source, [
595+
'timestampValueExpression',
596+
'connection',
597+
'from',
598+
]),
586599
where: appliedConfig.where || '',
587600
whereLanguage: appliedConfig.whereLanguage || 'sql',
588601
select: [
@@ -703,7 +716,11 @@ function HttpTab({
703716
'error_count',
704717
]}
705718
config={{
706-
...source,
719+
...pick(source, [
720+
'timestampValueExpression',
721+
'connection',
722+
'from',
723+
]),
707724
where: appliedConfig.where || '',
708725
whereLanguage: appliedConfig.whereLanguage || 'sql',
709726
select: [
@@ -1122,7 +1139,11 @@ function DatabaseTab({
11221139
'p50_duration_ns',
11231140
]}
11241141
config={{
1125-
...source,
1142+
...pick(source, [
1143+
'timestampValueExpression',
1144+
'connection',
1145+
'from',
1146+
]),
11261147
where: appliedConfig.where || '',
11271148
whereLanguage: appliedConfig.whereLanguage || 'sql',
11281149
dateRange: searchedTimeRange,
@@ -1198,7 +1219,11 @@ function DatabaseTab({
11981219
'p50_duration_ns',
11991220
]}
12001221
config={{
1201-
...source,
1222+
...pick(source, [
1223+
'timestampValueExpression',
1224+
'connection',
1225+
'from',
1226+
]),
12021227
where: appliedConfig.where || '',
12031228
whereLanguage: appliedConfig.whereLanguage || 'sql',
12041229
dateRange: searchedTimeRange,
@@ -1292,7 +1317,11 @@ function ErrorsTab({
12921317
<DBTimeChart
12931318
sourceId={source.id}
12941319
config={{
1295-
...source,
1320+
...pick(source, [
1321+
'timestampValueExpression',
1322+
'connection',
1323+
'from',
1324+
]),
12961325
where: appliedConfig.where || '',
12971326
whereLanguage: appliedConfig.whereLanguage || 'sql',
12981327
displayType: DisplayType.StackedBar,
@@ -1341,13 +1370,34 @@ function ServicesDashboardPage() {
13411370

13421371
const { data: sources } = useSources();
13431372

1344-
const [appliedConfig, setAppliedConfig] = useQueryStates(appliedConfigMap);
1373+
const [appliedConfigParams, setAppliedConfigParams] =
1374+
useQueryStates(appliedConfigMap);
1375+
1376+
// Only use the source from the URL params if it is a trace source
1377+
const appliedConfig = useMemo(() => {
1378+
if (!sources?.length) return appliedConfigParams;
1379+
1380+
const traceSources = sources?.filter(s => s.kind === SourceKind.Trace);
1381+
const paramsSourceIdIsTraceSource = traceSources?.find(
1382+
s => s.id === appliedConfigParams.source,
1383+
);
1384+
1385+
const effectiveSourceId = paramsSourceIdIsTraceSource
1386+
? appliedConfigParams.source
1387+
: traceSources?.[0]?.id || '';
1388+
1389+
return {
1390+
...appliedConfigParams,
1391+
source: effectiveSourceId,
1392+
};
1393+
}, [appliedConfigParams, sources]);
1394+
13451395
const { control, watch, setValue, handleSubmit } = useForm({
1346-
values: {
1396+
defaultValues: {
13471397
where: '',
13481398
whereLanguage: 'sql' as 'sql' | 'lucene',
13491399
service: appliedConfig?.service || '',
1350-
source: appliedConfig?.source || sources?.[0]?.id,
1400+
source: appliedConfig?.source ?? '',
13511401
},
13521402
});
13531403

@@ -1357,11 +1407,19 @@ function ServicesDashboardPage() {
13571407
id: watch('source'),
13581408
});
13591409

1410+
// Update the `source` query parameter if the appliedConfig source changes
13601411
useEffect(() => {
1361-
if (sourceId && !appliedConfig.source) {
1362-
setAppliedConfig({ source: sourceId });
1412+
if (
1413+
appliedConfig.source &&
1414+
appliedConfig.source !== appliedConfigParams.source
1415+
) {
1416+
setAppliedConfigParams({ source: appliedConfig.source });
13631417
}
1364-
}, [appliedConfig.source, setAppliedConfig, sourceId]);
1418+
}, [
1419+
appliedConfig.source,
1420+
appliedConfigParams.source,
1421+
setAppliedConfigParams,
1422+
]);
13651423

13661424
const DEFAULT_INTERVAL = 'Past 1h';
13671425
const [displayedTimeInputValue, setDisplayedTimeInputValue] =
@@ -1374,7 +1432,7 @@ function ServicesDashboardPage() {
13741432
});
13751433

13761434
// For future use if Live button is added
1377-
const [isLive, setIsLive] = useState(false);
1435+
const [isLive, _setIsLive] = useState(false);
13781436

13791437
const { manualRefreshCooloff, refresh } = useDashboardRefresh({
13801438
searchedTimeRange,
@@ -1385,30 +1443,38 @@ function ServicesDashboardPage() {
13851443
const onSubmit = useCallback(() => {
13861444
onSearch(displayedTimeInputValue);
13871445
handleSubmit(values => {
1388-
setAppliedConfig(values);
1446+
setAppliedConfigParams(values);
13891447
})();
1390-
}, [handleSubmit, setAppliedConfig, onSearch, displayedTimeInputValue]);
1448+
}, [handleSubmit, setAppliedConfigParams, onSearch, displayedTimeInputValue]);
13911449

1392-
// Auto submit when service or source changes
1450+
// Auto-submit when source changes
13931451
useEffect(() => {
1394-
const normalizedService = service ?? '';
1395-
const appliedService = appliedConfig.service ?? '';
1396-
const normalizedSource = sourceId ?? '';
1397-
const appliedSource = appliedConfig.source ?? '';
1452+
const { unsubscribe } = watch((data, { name, type }) => {
1453+
if (
1454+
name === 'source' &&
1455+
type === 'change' &&
1456+
data.source &&
1457+
data.source !== appliedConfig.source
1458+
) {
1459+
onSubmit();
1460+
}
1461+
});
1462+
return () => unsubscribe();
1463+
}, [appliedConfig.source, onSubmit, watch]);
13981464

1399-
if (
1400-
normalizedService !== appliedService ||
1401-
(normalizedSource && normalizedSource !== appliedSource)
1402-
) {
1403-
onSubmit();
1404-
}
1405-
}, [
1406-
service,
1407-
sourceId,
1408-
appliedConfig.service,
1409-
appliedConfig.source,
1410-
onSubmit,
1411-
]);
1465+
// Auto-submit when service changes
1466+
useEffect(() => {
1467+
const { unsubscribe } = watch((data, { name, type }) => {
1468+
if (
1469+
name === 'service' &&
1470+
type === 'change' &&
1471+
data.service !== appliedConfig.service
1472+
) {
1473+
onSubmit();
1474+
}
1475+
});
1476+
return () => unsubscribe();
1477+
}, [appliedConfig.service, onSubmit, watch]);
14121478

14131479
return (
14141480
<Box p="sm">
@@ -1554,7 +1620,7 @@ const ServicesDashboardPageDynamic = dynamic(
15541620
},
15551621
);
15561622

1557-
// @ts-ignore
1623+
// @ts-expect-error Next.js layout typing
15581624
ServicesDashboardPageDynamic.getLayout = withAppNav;
15591625

15601626
export default ServicesDashboardPageDynamic;

‎packages/app/src/components/ServiceDashboardDbQuerySidePanel.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useCallback, useMemo } from 'react';
2+
import { pick } from 'lodash';
23
import { parseAsString, useQueryState } from 'nuqs';
34
import type { Filter } from '@hyperdx/common-utils/dist/types';
45
import { Drawer, Grid, Group, Text } from '@mantine/core';
@@ -97,7 +98,11 @@ export default function ServiceDashboardDbQuerySidePanel({
9798
sourceId={sourceId}
9899
hiddenSeries={['total_duration_ns']}
99100
config={{
100-
...source,
101+
...pick(source, [
102+
'timestampValueExpression',
103+
'connection',
104+
'from',
105+
]),
101106
where: '',
102107
whereLanguage: 'sql',
103108
select: [
@@ -130,7 +135,11 @@ export default function ServiceDashboardDbQuerySidePanel({
130135
<DBTimeChart
131136
sourceId={sourceId}
132137
config={{
133-
...source,
138+
...pick(source, [
139+
'timestampValueExpression',
140+
'connection',
141+
'from',
142+
]),
134143
where: '',
135144
whereLanguage: 'sql',
136145
select: [

‎packages/app/src/components/ServiceDashboardEndpointPerformanceChart.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { pick } from 'lodash';
12
import { TSource } from '@hyperdx/common-utils/dist/types';
23
import { Group, Text } from '@mantine/core';
34

@@ -95,7 +96,7 @@ export default function ServiceDashboardEndpointPerformanceChart({
9596
groupColumn="group"
9697
valueColumn="Total Time Spent"
9798
config={{
98-
...source,
99+
...pick(source, ['timestampValueExpression', 'connection', 'from']),
99100
where: '',
100101
whereLanguage: 'sql',
101102
select: [

‎packages/app/src/components/ServiceDashboardEndpointSidePanel.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useCallback, useMemo } from 'react';
2+
import { pick } from 'lodash';
23
import { parseAsString, useQueryState } from 'nuqs';
34
import type { Filter } from '@hyperdx/common-utils/dist/types';
45
import { Drawer, Grid, Group, Text } from '@mantine/core';
@@ -104,7 +105,11 @@ export default function ServiceDashboardEndpointSidePanel({
104105
sourceId={source.id}
105106
hiddenSeries={['total_count', 'error_count']}
106107
config={{
107-
...source,
108+
...pick(source, [
109+
'timestampValueExpression',
110+
'connection',
111+
'from',
112+
]),
108113
where: '',
109114
whereLanguage: 'sql',
110115
select: [
@@ -144,7 +149,11 @@ export default function ServiceDashboardEndpointSidePanel({
144149
<DBTimeChart
145150
sourceId={source.id}
146151
config={{
147-
...source,
152+
...pick(source, [
153+
'timestampValueExpression',
154+
'connection',
155+
'from',
156+
]),
148157
where: '',
149158
whereLanguage: 'sql',
150159
select: [

0 commit comments

Comments
 (0)