Skip to content

Commit cc2a3d4

Browse files
wrn14897wrn14897
authored
fix: no need to show field dropdown for all 'count' agg functions (#541)
<img width="696" alt="Screenshot 2024-12-23 at 3 12 25 PM" src="https://github.com/user-attachments/assets/b96c506a-8fc6-4451-a5d1-806aabe3035c" />
1 parent 8d72534 commit cc2a3d4

File tree

2 files changed

+15
-260
lines changed

2 files changed

+15
-260
lines changed

‎packages/app/src/ChartUtils.tsx

Lines changed: 13 additions & 259 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@ import Checkbox from './Checkbox';
1515
import FieldMultiSelect from './FieldMultiSelect';
1616
import MetricTagFilterInput from './MetricTagFilterInput';
1717
import SearchInput from './SearchInput';
18-
import { AggFn, ChartSeries, MetricsDataType, SourceTable } from './types';
19-
import { NumberFormat } from './types';
18+
import {
19+
AggFn,
20+
ChartSeries,
21+
MetricsDataType,
22+
NumberFormat,
23+
SourceTable,
24+
} from './types';
2025
import { legacyMetricNameToNameAndDataType } from './utils';
2126

2227
export const SORT_ORDER = [
@@ -50,6 +55,9 @@ export const AGG_FNS: {
5055
{ value: 'sum', label: 'Sum' },
5156
];
5257

58+
export const isCountAggFn = (aggFn: AggFn) =>
59+
aggFn.startsWith('count_per') || aggFn === 'count';
60+
5361
export const getMetricAggFns = (
5462
dataType: MetricsDataType,
5563
): { value: AggFn; label: string }[] => {
@@ -581,261 +589,6 @@ export function FieldSelect({
581589
);
582590
}
583591

584-
export function ChartSeriesForm({
585-
aggFn,
586-
field,
587-
groupBy,
588-
setAggFn,
589-
setField,
590-
setFieldAndAggFn,
591-
setTableAndAggFn,
592-
setGroupBy,
593-
setSortOrder,
594-
setWhere,
595-
sortOrder,
596-
table,
597-
where,
598-
numberFormat,
599-
setNumberFormat,
600-
}: {
601-
aggFn: AggFn;
602-
field: string | undefined;
603-
groupBy: string | undefined;
604-
setAggFn: (fn: AggFn) => void;
605-
setField: (field: string | undefined) => void;
606-
setFieldAndAggFn: (field: string | undefined, fn: AggFn) => void;
607-
setTableAndAggFn: (table: SourceTable, fn: AggFn) => void;
608-
setGroupBy: (groupBy: string | undefined) => void;
609-
setSortOrder?: (sortOrder: SortOrder) => void;
610-
setWhere: (where: string) => void;
611-
sortOrder?: string;
612-
table: string;
613-
where: string;
614-
numberFormat?: NumberFormat;
615-
setNumberFormat?: (format?: NumberFormat) => void;
616-
}) {
617-
const labelWidth = 350;
618-
const searchInputRef = useRef<HTMLInputElement>(null);
619-
620-
const isRate = useMemo(() => {
621-
return aggFn.includes('_rate');
622-
}, [aggFn]);
623-
const _setAggFn = (fn: AggFn, _isRate?: boolean) => {
624-
if (_isRate ?? isRate) {
625-
if (fn.includes('_rate')) {
626-
setAggFn(fn);
627-
} else {
628-
setAggFn(`${fn}_rate` as AggFn);
629-
}
630-
} else {
631-
if (fn.includes('_rate')) {
632-
setAggFn(fn.replace('_rate', '') as AggFn);
633-
} else {
634-
setAggFn(fn);
635-
}
636-
}
637-
};
638-
const metricAggFns = getMetricAggFns(
639-
legacyMetricNameToNameAndDataType(field)?.dataType,
640-
);
641-
642-
return (
643-
<div>
644-
<div className="d-flex align-items-center">
645-
<div style={{ width: labelWidth }}>
646-
<Select
647-
options={TABLES}
648-
className="ds-select"
649-
value={TABLES.find(v => v.value === table)}
650-
onChange={opt => {
651-
const val = opt?.value ?? 'logs';
652-
if (val === 'logs') {
653-
setTableAndAggFn('logs', 'count');
654-
} else if (val === 'metrics') {
655-
// TODO: This should set rate if metric field is a sum
656-
// or we should just reset the field if changing tables
657-
setTableAndAggFn('metrics', 'max');
658-
}
659-
}}
660-
classNamePrefix="ds-react-select"
661-
/>
662-
</div>
663-
<div className="ms-3" style={{ width: labelWidth }}>
664-
{table === 'logs' ? (
665-
<Select
666-
options={AGG_FNS}
667-
className="ds-select"
668-
value={AGG_FNS.find(v => v.value === aggFn)}
669-
onChange={opt => _setAggFn(opt?.value ?? 'count')}
670-
classNamePrefix="ds-react-select"
671-
/>
672-
) : (
673-
<Select
674-
options={metricAggFns}
675-
className="ds-select"
676-
value={metricAggFns.find(
677-
v => v.value === aggFn.replace('_rate', ''),
678-
)}
679-
onChange={opt => _setAggFn(opt?.value ?? 'sum')}
680-
classNamePrefix="ds-react-select"
681-
/>
682-
)}
683-
</div>
684-
{table === 'logs' && aggFn != 'count' && aggFn != 'count_distinct' ? (
685-
<div className="ms-3 flex-grow-1">
686-
<FieldSelect
687-
value={field}
688-
setValue={setField}
689-
types={['number']}
690-
autoFocus={!field}
691-
/>
692-
</div>
693-
) : null}
694-
{table === 'logs' && aggFn != 'count' && aggFn == 'count_distinct' ? (
695-
<div className="ms-3 flex-grow-1">
696-
<FieldSelect
697-
value={field}
698-
setValue={setField}
699-
types={['string', 'number', 'bool']}
700-
autoFocus={!field}
701-
/>
702-
</div>
703-
) : null}
704-
{table === 'metrics' ? (
705-
<div className="d-flex align-items-center align-middle flex-grow-1 ms-3">
706-
<MetricSelect
707-
metricName={field}
708-
setMetricName={setField}
709-
isRate={isRate}
710-
setAggFn={setAggFn}
711-
setFieldAndAggFn={setFieldAndAggFn}
712-
aggFn={aggFn}
713-
/>
714-
</div>
715-
) : null}
716-
</div>
717-
{table === 'logs' ? (
718-
<>
719-
<div className="d-flex mt-3 align-items-center">
720-
<div
721-
style={{ width: labelWidth }}
722-
className="text-muted fw-500 ps-2"
723-
>
724-
Where
725-
</div>
726-
<div className="ms-3 flex-grow-1">
727-
<SearchInput
728-
inputRef={searchInputRef}
729-
placeholder={'Filter results by a search query'}
730-
value={where}
731-
onChange={v => setWhere(v)}
732-
onSearch={() => {}}
733-
/>
734-
</div>
735-
</div>
736-
<div className="d-flex mt-3 align-items-center">
737-
<div
738-
style={{ width: labelWidth }}
739-
className="text-muted fw-500 ps-2"
740-
>
741-
Group By
742-
</div>
743-
<div className="ms-3 flex-grow-1" style={{ minWidth: 300 }}>
744-
<FieldSelect
745-
value={groupBy}
746-
setValue={setGroupBy}
747-
types={['number', 'bool', 'string']}
748-
/>
749-
</div>
750-
</div>
751-
</>
752-
) : (
753-
<>
754-
<div className="d-flex mt-3 align-items-center">
755-
<div
756-
style={{ width: labelWidth }}
757-
className="text-muted fw-500 ps-2"
758-
>
759-
Where
760-
</div>
761-
<div className="ms-3 flex-grow-1">
762-
<MetricTagFilterInput
763-
placeholder={
764-
field
765-
? 'Filter metric by tag...'
766-
: 'Select a metric above to start filtering by tag...'
767-
}
768-
inputRef={searchInputRef}
769-
value={where}
770-
onChange={v => setWhere(v)}
771-
metricName={field}
772-
/>
773-
</div>
774-
</div>
775-
<div className="d-flex mt-3 align-items-center">
776-
<div
777-
style={{ width: labelWidth }}
778-
className="text-muted fw-500 ps-2"
779-
>
780-
Group By
781-
</div>
782-
<div className="ms-3 flex-grow-1" style={{ minWidth: 300 }}>
783-
<MetricTagSelect
784-
value={groupBy}
785-
setValue={setGroupBy}
786-
metricNames={field != null ? [field] : []}
787-
/>
788-
</div>
789-
</div>
790-
</>
791-
)}
792-
{
793-
// TODO: support metrics
794-
sortOrder != null && setSortOrder != null && table === 'logs' && (
795-
<div className="d-flex mt-3 align-items-center">
796-
<div
797-
style={{ width: labelWidth }}
798-
className="text-muted fw-500 ps-2"
799-
>
800-
Sort Order
801-
</div>
802-
<div className="ms-3 flex-grow-1">
803-
<Select
804-
options={SORT_ORDER}
805-
className="ds-select"
806-
value={SORT_ORDER.find(v => v.value === sortOrder)}
807-
onChange={opt => setSortOrder(opt?.value ?? 'desc')}
808-
classNamePrefix="ds-react-select"
809-
/>
810-
</div>
811-
</div>
812-
)
813-
}
814-
{setNumberFormat && (
815-
<div className="ms-2 mt-2 mb-3">
816-
<Divider
817-
label={
818-
<>
819-
<i className="bi bi-gear me-1" />
820-
Chart Settings
821-
</>
822-
}
823-
c="dark.2"
824-
mb={8}
825-
/>
826-
<Group>
827-
<div className="fs-8 text-slate-300">Number Format</div>
828-
<NumberFormatInput
829-
value={numberFormat}
830-
onChange={setNumberFormat}
831-
/>
832-
</Group>
833-
</div>
834-
)}
835-
</div>
836-
);
837-
}
838-
839592
export function TableSelect({
840593
table,
841594
setTableAndAggFn,
@@ -984,6 +737,7 @@ export function ChartSeriesFormCompact({
984737
const metricAggFns = getMetricAggFns(
985738
legacyMetricNameToNameAndDataType(field)?.dataType,
986739
);
740+
const isAggFnCountDistinct = aggFn === 'count_distinct';
987741

988742
return (
989743
<div>
@@ -1018,7 +772,7 @@ export function ChartSeriesFormCompact({
1018772
/>
1019773
)}
1020774
</div>
1021-
{table === 'logs' && aggFn != 'count' && aggFn != 'count_distinct' ? (
775+
{table === 'logs' && !isCountAggFn(aggFn) && !isAggFnCountDistinct ? (
1022776
<div className="flex-grow-1">
1023777
<FieldSelect
1024778
className="w-auto text-nowrap"
@@ -1029,7 +783,7 @@ export function ChartSeriesFormCompact({
1029783
/>
1030784
</div>
1031785
) : null}
1032-
{table === 'logs' && aggFn != 'count' && aggFn == 'count_distinct' ? (
786+
{table === 'logs' && isAggFnCountDistinct ? (
1033787
<div className="flex-grow-1">
1034788
<FieldSelect
1035789
className="w-auto text-nowrap"

‎packages/app/src/api.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import ky from 'ky-universal';
55
import type { UseQueryOptions } from 'react-query';
66
import { useInfiniteQuery, useMutation, useQuery } from 'react-query';
77

8+
import { isCountAggFn } from './ChartUtils';
89
import { SERVER_URL } from './config';
910
import type {
1011
AlertChannel,
@@ -73,7 +74,7 @@ const getEnrichedSeries = (series: ChartSeries[]) =>
7374
if (s.table === 'metrics' && !s.field) {
7475
return false;
7576
}
76-
if (s.table === 'logs' && !s.field && s.aggFn !== 'count') {
77+
if (s.table === 'logs' && !s.field && !isCountAggFn(s.aggFn)) {
7778
return false;
7879
}
7980
}

0 commit comments

Comments
 (0)