Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions static/app/views/explore/hooks/useChartInterval.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('useChartInterval', () => {
{value: '12h', label: '12 hours'},
{value: '1d', label: '1 day'},
]);
expect(chartInterval).toBe('12h'); // default
expect(chartInterval).toBe('1h'); // default

act(() => setChartInterval('3h'));
expect(chartInterval).toBe('3h');
Expand All @@ -50,9 +50,9 @@ describe('useChartInterval', () => {
{value: '15m', label: '15 minutes'},
]);
act(() => {
setChartInterval('1m');
setChartInterval('15m');
});
expect(chartInterval).toBe('1m');
expect(chartInterval).toBe('15m');
});
});

Expand Down
4 changes: 2 additions & 2 deletions static/app/views/explore/hooks/useChartInterval.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {useLocation} from 'sentry/utils/useLocation';
import {useNavigate} from 'sentry/utils/useNavigate';
import usePageFilters from 'sentry/utils/usePageFilters';

export enum ChartIntervalUnspecifiedStrategy {
enum ChartIntervalUnspecifiedStrategy {
/** Use the second biggest possible interval (e.g., pretty big buckets) */
USE_SECOND_BIGGEST = 'use_second_biggest',
/** Use the smallest possible interval (e.g., the smallest possible buckets) */
Expand All @@ -35,7 +35,7 @@ interface Options {
}

export function useChartInterval({
unspecifiedStrategy,
unspecifiedStrategy = ChartIntervalUnspecifiedStrategy.USE_SMALLEST,
}: {unspecifiedStrategy?: ChartIntervalUnspecifiedStrategy} = {}): [
string,
(interval: string) => void,
Expand Down
9 changes: 2 additions & 7 deletions static/app/views/explore/logs/logsGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ import {ChartVisualization} from 'sentry/views/explore/components/chart/chartVis
import type {ChartInfo} from 'sentry/views/explore/components/chart/types';
import {useLogsPageDataQueryResult} from 'sentry/views/explore/contexts/logs/logsPageData';
import {formatSort} from 'sentry/views/explore/contexts/pageParamsContext/sortBys';
import {
ChartIntervalUnspecifiedStrategy,
useChartInterval,
} from 'sentry/views/explore/hooks/useChartInterval';
import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
import {TOP_EVENTS_LIMIT} from 'sentry/views/explore/hooks/useTopEvents';
import {ConfidenceFooter} from 'sentry/views/explore/logs/confidenceFooter';
import {
Expand Down Expand Up @@ -127,9 +124,7 @@ function Graph({
const userQuery = useQueryParamsQuery();
const topEventsLimit = useQueryParamsTopEventsLimit();

const [interval, setInterval, intervalOptions] = useChartInterval({
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SMALLEST,
});
const [interval, setInterval, intervalOptions] = useChartInterval();

const chartInfo: ChartInfo = useMemo(() => {
// If the table is empty or pending, we want to withhold the chart data.
Expand Down
9 changes: 2 additions & 7 deletions static/app/views/explore/logs/logsTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ import {usePersistedLogsPageParams} from 'sentry/views/explore/contexts/logs/log
import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
import {useTraceItemAttributes} from 'sentry/views/explore/contexts/traceItemAttributeContext';
import {useLogAnalytics} from 'sentry/views/explore/hooks/useAnalytics';
import {
ChartIntervalUnspecifiedStrategy,
useChartInterval,
} from 'sentry/views/explore/hooks/useChartInterval';
import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
import {
HiddenColumnEditorLogFields,
HiddenLogSearchFields,
Expand Down Expand Up @@ -116,9 +113,7 @@ export function LogsTabContent({datePageFilterProps}: LogsTabProps) {

const columnEditorButtonRef = useRef<HTMLButtonElement>(null);
// always use the smallest interval possible (the most bars)
const [interval] = useChartInterval({
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SMALLEST,
});
const [interval] = useChartInterval();
const visualizes = useQueryParamsVisualizes();

const [sidebarOpen, setSidebarOpen] = useState(mode === Mode.AGGREGATE);
Expand Down
9 changes: 2 additions & 7 deletions static/app/views/explore/logs/useLogsTimeseries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ import {DiscoverDatasets} from 'sentry/utils/discover/types';
import {determineSeriesSampleCountAndIsSampled} from 'sentry/views/alerts/rules/metric/utils/determineSeriesSampleCount';
import type {useLogsPageDataQueryResult} from 'sentry/views/explore/contexts/logs/logsPageData';
import {formatSort} from 'sentry/views/explore/contexts/pageParamsContext/sortBys';
import {
ChartIntervalUnspecifiedStrategy,
useChartInterval,
} from 'sentry/views/explore/hooks/useChartInterval';
import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
import {
useProgressiveQuery,
type RPCQueryExtras,
Expand Down Expand Up @@ -96,9 +93,7 @@ function useLogsTimeseriesImpl({
const topEventsLimit = useQueryParamsTopEventsLimit();
const [caseInsensitive] = useCaseInsensitivity();

const [interval] = useChartInterval({
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SMALLEST,
});
const [interval] = useChartInterval();

const orderby: string | string[] | undefined = useMemo(() => {
if (!aggregateSortBys.length) {
Expand Down
4 changes: 3 additions & 1 deletion static/app/views/explore/logs/useSaveAsItems.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ describe('useSaveAsItems', () => {
useSaveAsItems({
visualizes: [new VisualizeFunction('count()')],
groupBys: ['message.template'],
// Note: useSaveQuery uses the value returned by useChartInterval()
// not the interval passed in as options.
interval: '5m',
mode: Mode.AGGREGATE,
search: new MutableSearch('message:"test error"'),
Expand Down Expand Up @@ -257,7 +259,7 @@ describe('useSaveAsItems', () => {
end: '2024-01-01T01:00:00.000Z',
range: '1h',
environment: ['production'],
interval: '5m',
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Given that logs were already setting ChartIntervalUnspecifiedStrategy.USE_SMALLEST I'm confused as to why I needed to update this test.

Would like to look a little closer before landing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

useSaveAsItems, when invoking useSaveQuery uses the interval returned by userChartInterval() rather than the interval passed in as options: https://github.com/getsentry/sentry/blob/master/static/app/views/explore/hooks/useSaveQuery.tsx#L59

I believe this to be intentional so that when you save you're saving whatever is currently set in the chart on the page.

Testing locally saving queries seemed to work as expected.

interval: '1m',
query: [
{
fields: ['timestamp', 'message', 'user.email'],
Expand Down
12 changes: 6 additions & 6 deletions static/app/views/explore/multiQueryMode/content.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ describe('MultiQueryModeContent', () => {
environment: [],
excludeOther: 0,
groupBy: [],
interval: '3h',
interval: '30m',
partial: 1,
project: [2],
query: '',
Expand Down Expand Up @@ -1009,7 +1009,7 @@ describe('MultiQueryModeContent', () => {
environment: [],
excludeOther: 0,
groupBy: ['span.op'],
interval: '3h',
interval: '30m',
partial: 1,
project: [2],
query: '',
Expand Down Expand Up @@ -1143,14 +1143,14 @@ describe('MultiQueryModeContent', () => {

const section = screen.getByTestId('section-visualization-0');
expect(
await within(section).findByRole('button', {name: '3 hours'})
await within(section).findByRole('button', {name: '30 minutes'})
).toBeInTheDocument();
await userEvent.click(within(section).getByRole('button', {name: '3 hours'}));
await userEvent.click(within(section).getByRole('option', {name: '30 minutes'}));
await userEvent.click(within(section).getByRole('button', {name: '30 minutes'}));
await userEvent.click(within(section).getByRole('option', {name: '3 hours'}));
expect(router.push).toHaveBeenCalledWith({
pathname: '/traces/compare',
query: expect.objectContaining({
interval: '30m',
interval: '3h',
queries: [
'{"groupBys":[],"query":"","sortBys":["-timestamp"],"yAxes":["avg(span.duration)"]}',
],
Expand Down
Loading