Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion static/app/views/explore/hooks/useMetricOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import usePageFilters from 'sentry/utils/usePageFilters';
interface EventsMetricResult {
data: Array<{
['metric.name']: string;
['metric.type']: string;
['metric.type']: 'counter' | 'distribution' | 'gauge';
}>;
meta?: {
fields?: Record<string, string>;
Expand Down
15 changes: 14 additions & 1 deletion static/app/views/explore/metrics/metricQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export function decodeMetricsQueryParams(value: string): BaseMetricQuery | null
return null;
}

const aggregateFields = json.aggregateFields;
if (!Array.isArray(aggregateFields)) {
return null;
}

return {
metric: {name: metric},
queryParams: new ReadableQueryParams({
Expand All @@ -49,7 +54,14 @@ export function decodeMetricsQueryParams(value: string): BaseMetricQuery | null
sortBys: defaultSortBys(),

aggregateCursor: '',
aggregateFields: defaultAggregateFields(),
aggregateFields: aggregateFields
.filter(field => field && typeof field.yAxis === 'string')
.map(
aggregateField =>
new VisualizeFunction(aggregateField.yAxis, {
chartType: aggregateField.chartType,
})
),
aggregateSortBys: defaultAggregateSortBys(),
}),
};
Expand All @@ -59,6 +71,7 @@ export function encodeMetricQueryParams(metricQuery: BaseMetricQuery): string {
return JSON.stringify({
metric: metricQuery.metric.name,
query: metricQuery.queryParams.query,
aggregateFields: metricQuery.queryParams.aggregateFields,
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import {CompactSelect} from 'sentry/components/core/compactSelect';
import {defined} from 'sentry/utils';
import {
useMetricVisualize,
useSetMetricVisualize,
} from 'sentry/views/explore/metrics/metricsQueryParams';

const OPTIONS_BY_TYPE = {
counter: [
{
label: 'sum',
value: 'sum',
},
],
distribution: [
{
label: 'p50',
value: 'p50',
},
{
label: 'p75',
value: 'p75',
},
{
label: 'p90',
value: 'p90',
},
{
label: 'p95',
value: 'p95',
},
{
label: 'p99',
value: 'p99',
},
{
label: 'avg',
value: 'avg',
},
{
label: 'min',
value: 'min',
},
{
label: 'max',
value: 'max',
},
{
label: 'sum',
value: 'sum',
},
{
label: 'count',
value: 'count',
},
],
gauge: [
{
label: 'min',
value: 'min',
},
{
label: 'max',
value: 'max',
},
{
label: 'avg',
value: 'avg',
},
{
label: 'last',
value: 'last',
},
],
};

export function AggregateDropdown({
type,
}: {
type: 'counter' | 'distribution' | 'gauge' | undefined;
}) {
const visualize = useMetricVisualize();
const setVisualize = useSetMetricVisualize();

return (
<CompactSelect
options={defined(type) ? OPTIONS_BY_TYPE[type] : []}
value={visualize.parsedFunction?.name ?? ''}
onChange={option => {
setVisualize(
visualize.replace({
yAxis: `${option.value}(${visualize.parsedFunction?.arguments?.[0] ?? ''})`,
})
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Malformed Y-Axis Strings in Aggregate Dropdown

The AggregateDropdown can generate malformed yAxis strings, such as sum(), when visualize.parsedFunction is null or its arguments are missing. This results in invalid query syntax and potential API errors.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignoring this for now

}}
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from 'sentry/views/explore/components/traceItemSearchQueryBuilder';
import {useMetricOptions} from 'sentry/views/explore/hooks/useMetricOptions';
import {type TraceMetric} from 'sentry/views/explore/metrics/metricQuery';
import {AggregateDropdown} from 'sentry/views/explore/metrics/metricRow/aggregateDropdown';
import {
useMetricVisualize,
useSetMetricName,
Expand All @@ -20,7 +21,6 @@ import {
useQueryParamsQuery,
useSetQueryParamsQuery,
} from 'sentry/views/explore/queryParams/context';
import type {VisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
import {TraceItemDataset} from 'sentry/views/explore/types';

interface MetricRowProps {
Expand Down Expand Up @@ -67,7 +67,7 @@ function MetricToolbar({
tracesItemSearchQueryBuilderProps,
traceMetric,
}: MetricToolbarProps) {
const visualize = useMetricVisualize() as VisualizeFunction;
const visualize = useMetricVisualize();
const groupBys = useQueryParamsGroupBys();
const query = useQueryParamsQuery();
const {data: metricOptionsData} = useMetricOptions();
Expand All @@ -78,20 +78,29 @@ function MetricToolbar({
...(metricOptionsData?.data?.map(option => ({
label: `${option['metric.name']} (${option['metric.type']})`,
value: option['metric.name'],
type: option['metric.type'],
})) ?? []),
// TODO(nar): Remove these when we actually have metrics served
// This is only used for providing an option to test current selection behavior
{
label: 'test-metric',
value: 'test-metric',
label: 'test-distribution',
value: 'test-distribution',
type: 'distribution' as const,
},
{
label: 'mock-metric',
value: 'mock-metric',
label: 'test-gauge',
value: 'test-gauge',
type: 'gauge' as const,
},
];
}, [metricOptionsData]);

// TODO(nar): This should come from the metric data context
// so we can display different types with conflicting names
const currentMetricType = useMemo(() => {
return metricOptions?.find(metricData => metricData.value === traceMetric.name)?.type;
}, [metricOptions, traceMetric.name]);

return (
<div style={{width: '100%'}}>
{traceMetric.name}/{visualize.yAxis}/ by {groupBys.join(',')}/ where {query}
Expand All @@ -104,15 +113,7 @@ function MetricToolbar({
setMetricName(option.value);
}}
/>
<CompactSelect
options={[
{
label: 'count',
value: 'count',
},
]}
value={visualize.parsedFunction?.name}
/>
<AggregateDropdown type={currentMetricType} />
{t('by')}
<CompactSelect options={[]} value={groupBys[0] ?? ''} />
{t('where')}
Expand Down
22 changes: 19 additions & 3 deletions static/app/views/explore/metrics/metricsQueryParams.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ import type {AggregateField} from 'sentry/views/explore/queryParams/aggregateFie
import {
QueryParamsContextProvider,
useQueryParamsVisualizes,
useSetQueryParamsVisualizes,
} from 'sentry/views/explore/queryParams/context';
import {isGroupBy} from 'sentry/views/explore/queryParams/groupBy';
import {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
import {parseVisualize} from 'sentry/views/explore/queryParams/visualize';
import {
parseVisualize,
VisualizeFunction,
} from 'sentry/views/explore/queryParams/visualize';
import type {WritableQueryParams} from 'sentry/views/explore/queryParams/writableQueryParams';

interface MetricNameContextValue {
Expand Down Expand Up @@ -40,6 +44,7 @@ export function MetricsQueryParamsProvider({
(writableQueryParams: WritableQueryParams) => {
const newQueryParams = updateQueryParams(queryParams, {
query: getUpdatedValue(writableQueryParams.query, defaultQuery),
aggregateFields: writableQueryParams.aggregateFields,
});

setQueryParams(newQueryParams);
Expand Down Expand Up @@ -83,10 +88,10 @@ function getUpdatedValue<T>(
return undefined;
}

export function useMetricVisualize() {
export function useMetricVisualize(): VisualizeFunction {
const visualizes = useQueryParamsVisualizes();
if (visualizes.length === 1) {
return visualizes[0]!;
return visualizes[0] as VisualizeFunction;
}
throw new Error('Only 1 visualize per metric allowed');
}
Expand All @@ -96,6 +101,17 @@ export function useSetMetricName() {
return setMetricName;
}

export function useSetMetricVisualize() {
const setVisualizes = useSetQueryParamsVisualizes();
const setVisualize = useCallback(
(newVisualize: VisualizeFunction) => {
setVisualizes([newVisualize.serialize()]);
},
[setVisualizes]
);
return setVisualize;
}

function updateQueryParams(
readableQueryParams: ReadableQueryParams,
writableQueryParams: WritableQueryParams
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type {ReactNode} from 'react';
import {useMemo} from 'react';
import {useMemo, type ReactNode} from 'react';
import type {Location} from 'history';

import {defined} from 'sentry/utils';
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/explore/queryParams/visualize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class VisualizeFunction extends Visualize {
this.parsedFunction = parseFunction(yAxis);
}

clone(): Visualize {
clone(): VisualizeFunction {
return new VisualizeFunction(this.yAxis, {
chartType: this.selectedChartType,
visible: this.visible,
Expand All @@ -100,7 +100,7 @@ export class VisualizeFunction extends Visualize {
chartType?: ChartType;
visible?: boolean;
yAxis?: string;
}): Visualize {
}): VisualizeFunction {
return new VisualizeFunction(yAxis ?? this.yAxis, {
chartType: chartType ?? this.selectedChartType,
visible: visible ?? this.visible,
Expand Down
Loading