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
7 changes: 6 additions & 1 deletion static/app/types/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ type OrganizationHeaderProps = {

type ProductSelectionAvailabilityProps = Omit<ProductSelectionProps, 'disabledProducts'>;

type DateRangeQueryLimitFooterProps = {
description: string;
source: string;
};

type FirstPartyIntegrationAlertProps = {
integrations: Integration[];
hideCTA?: boolean;
Expand Down Expand Up @@ -190,9 +195,9 @@ type ComponentHooks = {
'component:disabled-member': () => React.ComponentType;
'component:disabled-member-tooltip': () => React.ComponentType<DisabledMemberTooltipProps>;
'component:enhanced-org-stats': () => React.ComponentType<OrganizationStatsProps>;
'component:explore-date-range-query-limit-footer': () => React.ComponentType;
'component:first-party-integration-additional-cta': () => React.ComponentType<FirstPartyIntegrationAdditionalCTAProps>;
'component:first-party-integration-alert': () => React.ComponentType<FirstPartyIntegrationAlertProps>;
'component:header-date-page-filter-upsell-footer': () => React.ComponentType<DateRangeQueryLimitFooterProps>;
'component:header-date-range': () => React.ComponentType<DateRangeProps>;
'component:header-selector-items': () => React.ComponentType<SelectorItemsProps>;
'component:insights-date-range-query-limit-footer': () => React.ComponentType;
Expand Down
64 changes: 64 additions & 0 deletions static/app/utils/useDatePageFilterProps.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import {useMemo, type ReactNode} from 'react';

import type {SelectOptionWithKey} from 'sentry/components/core/compactSelect/types';
import type {DatePageFilterProps} from 'sentry/components/organizations/datePageFilter';
import {t} from 'sentry/locale';
import {isEmptyObject} from 'sentry/utils/object/isEmptyObject';
import type {MaxPickableDaysOptions} from 'sentry/utils/useMaxPickableDays';

interface UseDatePageFilterPropsProps extends MaxPickableDaysOptions {}

export function useDatePageFilterProps({
defaultPeriod,
maxPickableDays,
maxUpgradableDays,
upsellFooter,
}: UseDatePageFilterPropsProps): DatePageFilterProps {
return useMemo(() => {
// ensure the available relative options are always sorted
const availableRelativeOptions: Array<[number, string, ReactNode]> = [
[1 / 24, '1h', t('Last hour')],
[1, '24h', t('Last 24 hours')],
[7, '7d', t('Last 7 days')],
[14, '14d', t('Last 14 days')],
[30, '30d', t('Last 30 days')],
[90, '90d', t('Last 90 days')],
];

// find the relative options that should be enabled based on the maxPickableDays
const pickableIndex =
availableRelativeOptions.findLastIndex(([days]) => days <= maxPickableDays) + 1;
const enabledOptions = Object.fromEntries(
availableRelativeOptions
.slice(0, pickableIndex)
.map(([_, period, label]) => [period, label])
);

// find the relative options that should be disabled based on the maxUpgradableDays
const upgradableIndex =
availableRelativeOptions.findLastIndex(([days]) => days <= maxUpgradableDays) + 1;
const disabledOptions = Object.fromEntries(
availableRelativeOptions
.slice(pickableIndex, upgradableIndex)
.map(([_, period, label]) => [period, label])
);

const isOptionDisabled = (option: SelectOptionWithKey<string>): boolean => {
return disabledOptions.hasOwnProperty(option.value);
};

const menuFooter = isEmptyObject(disabledOptions) ? null : (upsellFooter ?? null);

return {
defaultPeriod,
isOptionDisabled,
maxPickableDays,
menuFooter,
relativeOptions: ({arbitraryOptions}) => ({
...arbitraryOptions,
...enabledOptions,
...disabledOptions,
}),
};
}, [defaultPeriod, maxPickableDays, maxUpgradableDays, upsellFooter]);
}
90 changes: 90 additions & 0 deletions static/app/utils/useMaxPickableDays.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import {OrganizationFixture} from 'sentry-fixture/organization';

import {renderHook} from 'sentry-test/reactTestingLibrary';

import {DataCategory} from 'sentry/types/core';

import {useMaxPickableDays} from './useMaxPickableDays';

describe('useMaxPickableDays', () => {
it('returns 30/90 for spans without flag', () => {
const {result} = renderHook(() =>
useMaxPickableDays({
dataCategories: [DataCategory.SPANS],
organization: OrganizationFixture({features: []}),
})
);

expect(result.current).toEqual({
maxPickableDays: 30,
maxUpgradableDays: 90,
upsellFooter: expect.any(Object),
});
});

it('returns 90/90 for spans with flag', () => {
const {result} = renderHook(() =>
useMaxPickableDays({
dataCategories: [DataCategory.SPANS],
organization: OrganizationFixture({features: ['visibility-explore-range-high']}),
})
);

expect(result.current).toEqual({
maxPickableDays: 90,
maxUpgradableDays: 90,
upsellFooter: expect.any(Object),
});
});

it('returns 30/30 days for tracemetrics', () => {
const {result} = renderHook(() =>
useMaxPickableDays({
dataCategories: [DataCategory.TRACE_METRICS],
organization: OrganizationFixture(),
})
);

expect(result.current).toEqual({
defaultPeriod: '24h',
maxPickableDays: 30,
maxUpgradableDays: 30,
});
});

it('returns 30/30 days for logs', () => {
const {result} = renderHook(() =>
useMaxPickableDays({
dataCategories: [DataCategory.LOG_BYTE, DataCategory.LOG_ITEM],
organization: OrganizationFixture(),
})
);

expect(result.current).toEqual({
defaultPeriod: '24h',
maxPickableDays: 30,
maxUpgradableDays: 30,
});
});

it('returns 30/90 for many without flag', () => {
const {result} = renderHook(() =>
useMaxPickableDays({
dataCategories: [
DataCategory.SPANS,
DataCategory.SPANS_INDEXED,
DataCategory.TRACE_METRICS,
DataCategory.LOG_BYTE,
DataCategory.LOG_ITEM,
],
organization: OrganizationFixture(),
})
);

expect(result.current).toEqual({
maxPickableDays: 30,
maxUpgradableDays: 90,
upsellFooter: expect.any(Object),
});
});
});
108 changes: 108 additions & 0 deletions static/app/utils/useMaxPickableDays.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import {useMemo, type ReactNode} from 'react';

import HookOrDefault from 'sentry/components/hookOrDefault';
import type {DatePageFilterProps} from 'sentry/components/organizations/datePageFilter';
import {t} from 'sentry/locale';
import {DataCategory} from 'sentry/types/core';
import type {Organization} from 'sentry/types/organization';

export interface MaxPickableDaysOptions {
/**
* The maximum number of days the user is allowed to pick on the date page filter
*/
maxPickableDays: NonNullable<DatePageFilterProps['maxPickableDays']>;
/**
* The maximum number of days the user can upgrade to on the date page filter
*/
maxUpgradableDays: NonNullable<DatePageFilterProps['maxPickableDays']>;
defaultPeriod?: DatePageFilterProps['defaultPeriod'];
upsellFooter?: ReactNode;
}

interface UseMaxPickableDaysProps {
dataCategories: readonly [DataCategory, ...DataCategory[]];
organization: Organization;
}

export function useMaxPickableDays({
dataCategories,
organization,
}: UseMaxPickableDaysProps): MaxPickableDaysOptions {
Comment on lines +27 to +30
Copy link
Member Author

Choose a reason for hiding this comment

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

This hook will be replaced by an implementation that looks at the org's subscription.

return useMemo(() => {
function getMaxPickableDaysFor(dataCategory: DataCategory) {
return getMaxPickableDays(dataCategory, organization);
}

return getBestMaxPickableDays(dataCategories, getMaxPickableDaysFor);
}, [dataCategories, organization]);
}

function getBestMaxPickableDays(
dataCategories: readonly [DataCategory, ...DataCategory[]],
getMaxPickableDaysFor: (dataCategory: DataCategory) => MaxPickableDaysOptions
) {
let maxPickableDays: MaxPickableDaysOptions = getMaxPickableDaysFor(dataCategories[0]);

for (let i = 1; i < dataCategories.length; i++) {
const dataCategory = dataCategories[i]!;
const maxPickableDaysForDataCategory = getMaxPickableDaysFor(dataCategory);
maxPickableDays = max(maxPickableDays, maxPickableDaysForDataCategory);
}

return maxPickableDays;
}

function max(
a: MaxPickableDaysOptions,
b: MaxPickableDaysOptions
): MaxPickableDaysOptions {
if (a.maxPickableDays < b.maxPickableDays) {
return b;
}

if (a.maxUpgradableDays < b.maxUpgradableDays) {
return b;
}

return a;
}

const DESCRIPTION = t('To query over longer time ranges, upgrade to Business');

function getMaxPickableDays(
dataCategory: DataCategory,
organization: Organization
): MaxPickableDaysOptions {
switch (dataCategory) {
case DataCategory.SPANS:
case DataCategory.SPANS_INDEXED: {
const maxPickableDays = organization.features.includes(
'visibility-explore-range-high'
)
? 90
: 30;
return {
maxPickableDays,
maxUpgradableDays: 90,
upsellFooter: <UpsellFooterHook description={DESCRIPTION} source="spans" />,
};
}
case DataCategory.TRACE_METRICS:
case DataCategory.LOG_BYTE:
case DataCategory.LOG_ITEM:
return {
maxPickableDays: 30,
maxUpgradableDays: 30,
defaultPeriod: '24h',
};
Comment on lines +90 to +97
Copy link

Choose a reason for hiding this comment

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

Bug: The max() function in useMaxPickableDays.tsx can incorrectly drop the defaultPeriod property when comparing MaxPickableDaysOptions objects.
Severity: HIGH | Confidence: High

🔍 Detailed Analysis

The max() function in useMaxPickableDays.tsx (lines 55-68) incorrectly handles MaxPickableDaysOptions objects when one contains a defaultPeriod and another does not. If useMaxPickableDays is called with multiple data categories, and one category provides a defaultPeriod while another does not, the max() function may return an object missing the defaultPeriod. This can lead to datePageFilterProps.defaultPeriod being undefined in content.tsx, causing defaultSelection to be undefined when a default period should have been applied.

💡 Suggested Fix

Modify the max() function in useMaxPickableDays.tsx to ensure defaultPeriod is preserved if present in any of the compared MaxPickableDaysOptions objects, prioritizing the defaultPeriod from the object that "wins" the maxPickableDays and maxUpgradableDays comparison, or merging it if appropriate.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/utils/useMaxPickableDays.tsx#L90-L97

Potential issue: The `max()` function in `useMaxPickableDays.tsx` (lines 55-68)
incorrectly handles `MaxPickableDaysOptions` objects when one contains a `defaultPeriod`
and another does not. If `useMaxPickableDays` is called with multiple data categories,
and one category provides a `defaultPeriod` while another does not, the `max()` function
may return an object missing the `defaultPeriod`. This can lead to
`datePageFilterProps.defaultPeriod` being `undefined` in `content.tsx`, causing
`defaultSelection` to be `undefined` when a default period should have been applied.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3289398

default:
throw new Error(
`Unsupported data category: ${dataCategory} for getMaxPickableDays`
);
}
}

const UpsellFooterHook = HookOrDefault({
hookName: 'component:header-date-page-filter-upsell-footer',
defaultComponent: () => null,
});
42 changes: 23 additions & 19 deletions static/app/views/explore/logs/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import {withoutLoggingSupport} from 'sentry/data/platformCategories';
import {platforms} from 'sentry/data/platforms';
import {IconMegaphone, IconOpen} from 'sentry/icons';
import {t} from 'sentry/locale';
import {DataCategory} from 'sentry/types/core';
import {defined} from 'sentry/utils';
import {trackAnalytics} from 'sentry/utils/analytics';
import {LogsAnalyticsPageSource} from 'sentry/utils/analytics/logsAnalyticsEvent';
import {useDatePageFilterProps} from 'sentry/utils/useDatePageFilterProps';
import {useFeedbackForm} from 'sentry/utils/useFeedbackForm';
import {useMaxPickableDays} from 'sentry/utils/useMaxPickableDays';
import useOrganization from 'sentry/utils/useOrganization';
import usePageFilters from 'sentry/utils/usePageFilters';
import useProjects from 'sentry/utils/useProjects';
Expand All @@ -22,7 +25,6 @@ import {useGetSavedQuery} from 'sentry/views/explore/hooks/useGetSavedQueries';
import {LogsTabOnboarding} from 'sentry/views/explore/logs/logsOnboarding';
import {LogsQueryParamsProvider} from 'sentry/views/explore/logs/logsQueryParamsProvider';
import {LogsTabContent} from 'sentry/views/explore/logs/logsTab';
import {logsPickableDays} from 'sentry/views/explore/logs/utils';
import {
useQueryParamsId,
useQueryParamsTitle,
Expand Down Expand Up @@ -58,22 +60,30 @@ function FeedbackButton() {

export default function LogsContent() {
const organization = useOrganization();
const {defaultPeriod, maxPickableDays, relativeOptions} = logsPickableDays();
const maxPickableDays = useMaxPickableDays({
dataCategories: [DataCategory.LOG_BYTE],
organization,
});
const datePageFilterProps = useDatePageFilterProps(maxPickableDays);

const onboardingProject = useOnboardingProject({property: 'hasLogs'});

return (
<SentryDocumentTitle title={t('Logs')} orgSlug={organization?.slug}>
<PageFiltersContainer
maxPickableDays={maxPickableDays}
defaultSelection={{
datetime: {
period: defaultPeriod,
start: null,
end: null,
utc: null,
},
}}
maxPickableDays={datePageFilterProps.maxPickableDays}
defaultSelection={
datePageFilterProps.defaultPeriod
? {
datetime: {
period: datePageFilterProps.defaultPeriod,
start: null,
end: null,
utc: null,
},
}
: undefined
}
>
<LogsQueryParamsProvider
analyticsPageSource={LogsAnalyticsPageSource.EXPLORE_LOGS}
Expand All @@ -87,16 +97,10 @@ export default function LogsContent() {
<LogsTabOnboarding
organization={organization}
project={onboardingProject}
defaultPeriod={defaultPeriod}
maxPickableDays={maxPickableDays}
relativeOptions={relativeOptions}
datePageFilterProps={datePageFilterProps}
/>
) : (
<LogsTabContent
defaultPeriod={defaultPeriod}
maxPickableDays={maxPickableDays}
relativeOptions={relativeOptions}
/>
<LogsTabContent datePageFilterProps={datePageFilterProps} />
)}
</LogsPageDataProvider>
</TraceItemAttributeProvider>
Expand Down
Loading
Loading