Skip to content

feat(insights): Refactor chart widgets to pass down prop overrides #89455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 15, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import type {
LegendSelection,
Release,
} from 'sentry/views/dashboards/widgets/common/types';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useReleaseBubbles} from 'sentry/views/releases/releaseBubbles/useReleaseBubbles';
import {makeReleasesPathname} from 'sentry/views/releases/utils/pathnames';

Expand All @@ -55,7 +56,8 @@ import {TimeSeriesWidgetYAxis} from './timeSeriesWidgetYAxis';

const {error, warn, info} = Sentry.logger;

export interface TimeSeriesWidgetVisualizationProps {
export interface TimeSeriesWidgetVisualizationProps
extends Partial<LoadableChartWidgetProps> {
/**
* An array of `Plottable` objects. This can be any object that implements the `Plottable` interface.
*/
Expand Down Expand Up @@ -95,10 +97,6 @@ export interface TimeSeriesWidgetVisualizationProps {
* Default: `auto`
*/
showLegend?: 'auto' | 'never';
/**
* Show releases as either lines per release or a bubble for a group of releases.
*/
showReleaseAs?: 'bubble' | 'line';
}

export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizationProps) {
Expand All @@ -114,7 +112,8 @@ export function TimeSeriesWidgetVisualization(props: TimeSeriesWidgetVisualizati
const {register: registerWithWidgetSyncContext} = useWidgetSyncContext();

const pageFilters = usePageFilters();
const {start, end, period, utc} = pageFilters.selection.datetime;
const {start, end, period, utc} =
props.pageFilters?.datetime || pageFilters.selection.datetime;

const theme = useTheme();
const organization = useOrganization();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,21 @@ import {
HTTP_RESPONSE_5XX_COLOR,
THROUGHPUT_COLOR,
} from 'sentry/views/insights/colors';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import type {DiscoverSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {convertSeriesToTimeseries} from 'sentry/views/insights/common/utils/convertSeriesToTimeseries';
import {INGESTION_DELAY} from 'sentry/views/insights/settings';

export interface InsightsTimeSeriesWidgetProps extends WidgetTitleProps {
export interface InsightsTimeSeriesWidgetProps
extends WidgetTitleProps,
LoadableChartWidgetProps {
error: Error | null;
isLoading: boolean;
series: DiscoverSeries[];
visualizationType: 'line' | 'area' | 'bar';
aliases?: Record<string, string>;
description?: React.ReactNode;
height?: string | number;
id?: string;
interactiveTitle?: () => React.ReactNode;
legendSelection?: LegendSelection | undefined;
onLegendSelectionChange?: ((selection: LegendSelection) => void) | undefined;
Expand All @@ -51,7 +53,8 @@ export function InsightsTimeSeriesWidget(props: InsightsTimeSeriesWidgetProps) {
const theme = useTheme();
const organization = useOrganization();
const pageFilters = usePageFilters();
const {releases: releasesWithDate} = useReleaseStats(pageFilters.selection);
const pageFiltersSelection = props.pageFilters || pageFilters.selection;
const {releases: releasesWithDate} = useReleaseStats(pageFiltersSelection);
const releases =
releasesWithDate?.map(({date, version}) => ({
timestamp: date,
Expand Down Expand Up @@ -118,7 +121,7 @@ export function InsightsTimeSeriesWidget(props: InsightsTimeSeriesWidgetProps) {
}

const enableReleaseBubblesProps = organization.features.includes('release-bubbles-ui')
? ({releases, showReleaseAs: 'bubble'} as const)
? ({releases, showReleaseAs: props.showReleaseAs || 'bubble'} as const)
: {};

return (
Expand All @@ -127,6 +130,8 @@ export function InsightsTimeSeriesWidget(props: InsightsTimeSeriesWidgetProps) {
Title={Title}
Visualization={
<TimeSeriesWidgetVisualization
id={props.id}
pageFilters={props.pageFilters}
{...enableReleaseBubblesProps}
legendSelection={props.legendSelection}
onLegendSelectionChange={props.onLegendSelectionChange}
Expand All @@ -149,6 +154,7 @@ export function InsightsTimeSeriesWidget(props: InsightsTimeSeriesWidgetProps) {
children: (
<ModalChartContainer>
<TimeSeriesWidgetVisualization
id={props.id}
{...visualizationProps}
{...enableReleaseBubblesProps}
onZoom={() => {}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useHttpDomainSummaryChartFilter} from 'sentry/views/insights/common/components/widgets/hooks/useHttpDomainSummaryChartFilter';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {getDurationChartTitle} from 'sentry/views/insights/common/views/spans/types';
import {Referrer} from 'sentry/views/insights/http/referrers';
import {SpanMetricsField} from 'sentry/views/insights/types';

export default function HttpDomainSummaryDurationChartWidget() {
export default function HttpDomainSummaryDurationChartWidget(
props: LoadableChartWidgetProps
) {
const chartFilters = useHttpDomainSummaryChartFilter();
const {
isPending: isDurationDataLoading,
Expand All @@ -18,11 +21,13 @@ export default function HttpDomainSummaryDurationChartWidget() {
yAxis: [`avg(${SpanMetricsField.SPAN_SELF_TIME})`],
transformAliasToInputFormat: true,
},
Referrer.DOMAIN_SUMMARY_DURATION_CHART
Referrer.DOMAIN_SUMMARY_DURATION_CHART,
props.pageFilters
);

return (
<InsightsLineChartWidget
{...props}
id="httpDomainSummaryDurationChartWidget"
title={getDurationChartTitle('http')}
series={[durationData[`avg(${SpanMetricsField.SPAN_SELF_TIME})`]]}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useHttpDomainSummaryChartFilter} from 'sentry/views/insights/common/components/widgets/hooks/useHttpDomainSummaryChartFilter';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {DataTitles} from 'sentry/views/insights/common/views/spans/types';
import {Referrer} from 'sentry/views/insights/http/referrers';
import {FIELD_ALIASES} from 'sentry/views/insights/http/settings';

export default function HttpDomainSummaryResponseCodesWidget() {
export default function HttpDomainSummaryResponseCodesChartWidget(
props: LoadableChartWidgetProps
) {
const chartFilters = useHttpDomainSummaryChartFilter();
const {
isPending: isResponseCodeDataLoading,
Expand All @@ -18,12 +21,14 @@ export default function HttpDomainSummaryResponseCodesWidget() {
yAxis: ['http_response_rate(3)', 'http_response_rate(4)', 'http_response_rate(5)'],
transformAliasToInputFormat: true,
},
Referrer.DOMAIN_SUMMARY_RESPONSE_CODE_CHART
Referrer.DOMAIN_SUMMARY_RESPONSE_CODE_CHART,
props.pageFilters
);

return (
<InsightsLineChartWidget
id="httpDomainSummaryResponseCodesWidget"
{...props}
id="httpDomainSummaryResponseCodesChartWidget"
Copy link
Member Author

Choose a reason for hiding this comment

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

Gj cursor on autofixing this I guess

title={DataTitles.unsuccessfulHTTPCodes}
series={[
responseCodeData[`http_response_rate(3)`],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useHttpDomainSummaryChartFilter} from 'sentry/views/insights/common/components/widgets/hooks/useHttpDomainSummaryChartFilter';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {getThroughputChartTitle} from 'sentry/views/insights/common/views/spans/types';
import {Referrer} from 'sentry/views/insights/http/referrers';

export default function HttpDomainSummaryThroughputChartWidget() {
export default function HttpDomainSummaryThroughputChartWidget(
props: LoadableChartWidgetProps
) {
const chartFilters = useHttpDomainSummaryChartFilter();
const {
isPending: isThroughputDataLoading,
Expand All @@ -17,11 +20,13 @@ export default function HttpDomainSummaryThroughputChartWidget() {
yAxis: ['epm()'],
transformAliasToInputFormat: true,
},
Referrer.DOMAIN_SUMMARY_THROUGHPUT_CHART
Referrer.DOMAIN_SUMMARY_THROUGHPUT_CHART,
props.pageFilters
);

return (
<InsightsLineChartWidget
{...props}
id="httpDomainSummaryThroughputChartWidget"
title={getThroughputChartTitle('http')}
series={[throughputData['epm()']]}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useHttpLandingChartFilter} from 'sentry/views/insights/common/components/widgets/hooks/useHttpLandingChartFilter';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {getDurationChartTitle} from 'sentry/views/insights/common/views/spans/types';
import {Referrer} from 'sentry/views/insights/http/referrers';

export default function HttpDurationChartWidget() {
export default function HttpDurationChartWidget(props: LoadableChartWidgetProps) {
const chartFilters = useHttpLandingChartFilter();
const {
isPending: isDurationDataLoading,
Expand All @@ -17,11 +18,13 @@ export default function HttpDurationChartWidget() {
yAxis: ['avg(span.self_time)'],
transformAliasToInputFormat: true,
},
Referrer.LANDING_DURATION_CHART
Referrer.LANDING_DURATION_CHART,
props.pageFilters
);

return (
<InsightsLineChartWidget
{...props}
id="httpDurationChartWidget"
title={getDurationChartTitle('http')}
series={[durationData['avg(span.self_time)']]}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useHttpLandingChartFilter} from 'sentry/views/insights/common/components/widgets/hooks/useHttpLandingChartFilter';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {DataTitles} from 'sentry/views/insights/common/views/spans/types';
import {Referrer} from 'sentry/views/insights/http/referrers';
import {FIELD_ALIASES} from 'sentry/views/insights/http/settings';

export default function HttpResponseCodesChartWidget() {
export default function HttpResponseCodesChartWidget(props: LoadableChartWidgetProps) {
const chartFilters = useHttpLandingChartFilter();
const {
isPending: isResponseCodeDataLoading,
Expand All @@ -18,11 +19,13 @@ export default function HttpResponseCodesChartWidget() {
yAxis: ['http_response_rate(3)', 'http_response_rate(4)', 'http_response_rate(5)'],
transformAliasToInputFormat: true,
},
Referrer.LANDING_RESPONSE_CODE_CHART
Referrer.LANDING_RESPONSE_CODE_CHART,
props.pageFilters
);

return (
<InsightsLineChartWidget
{...props}
id="httpResponseCodesChartWidget"
title={DataTitles.unsuccessfulHTTPCodes}
series={[
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {InsightsLineChartWidget} from 'sentry/views/insights/common/components/insightsLineChartWidget';
import {useHttpLandingChartFilter} from 'sentry/views/insights/common/components/widgets/hooks/useHttpLandingChartFilter';
import type {LoadableChartWidgetProps} from 'sentry/views/insights/common/components/widgets/types';
import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
import {getThroughputChartTitle} from 'sentry/views/insights/common/views/spans/types';
import {Referrer} from 'sentry/views/insights/http/referrers';

export default function HttpThroughputChartWidget() {
export default function HttpThroughputChartWidget(props: LoadableChartWidgetProps) {
const chartFilters = useHttpLandingChartFilter();
const {
isPending: isThroughputDataLoading,
Expand All @@ -17,11 +18,13 @@ export default function HttpThroughputChartWidget() {
yAxis: ['epm()'],
transformAliasToInputFormat: true,
},
Referrer.LANDING_THROUGHPUT_CHART
Referrer.LANDING_THROUGHPUT_CHART,
props.pageFilters
);

return (
<InsightsLineChartWidget
{...props}
id="httpThroughputChartWidget"
title={getThroughputChartTitle('http')}
series={[throughputData['epm()']]}
Expand Down
25 changes: 25 additions & 0 deletions static/app/views/insights/common/components/widgets/types.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import type {PageFilters} from 'sentry/types/core';

/**
* These props are common across components that are required to dynamically
* render an Insight Chart Widget
*/
export interface LoadableChartWidgetProps {
// TODO(billy): This should be required when all chart widgets are converted
/**
* Unique ID for the widget
*/
Comment on lines +8 to +11
Copy link
Member

Choose a reason for hiding this comment

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

That makes sense, but if that's the case, TimeSeriesWidgetVisualization should use Partial<LoadableChartWidgetProps>. ID shouldn't be required there, just for the Insights chart

id?: string;

/**
* PageFilters-like object that will override the main PageFilters e.g. in
* Releases Drawer, we have a smaller timeframe to show a smaller amount of
* releases.
*/
pageFilters?: PageFilters;

/**
* Show releases as either lines per release or a bubble for a group of releases.
*/
showReleaseAs?: 'bubble' | 'line';
}
16 changes: 10 additions & 6 deletions static/app/views/insights/common/queries/useDiscoverSeries.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import moment from 'moment-timezone';

import type {PageFilters} from 'sentry/types/core';
import type {Series} from 'sentry/types/echarts';
import {encodeSort, type EventsMetaType} from 'sentry/utils/discover/eventView';
import {
Expand Down Expand Up @@ -51,13 +52,15 @@ interface UseMetricsSeriesOptions<Fields> {

export const useSpanMetricsSeries = <Fields extends SpanMetricsProperty[]>(
options: UseMetricsSeriesOptions<Fields> = {},
referrer: string
referrer: string,
pageFilters?: PageFilters
) => {
const useEap = useInsightsEap();
return useDiscoverSeries<Fields>(
options,
useEap ? DiscoverDatasets.SPANS_EAP_RPC : DiscoverDatasets.SPANS_METRICS,
referrer
referrer,
pageFilters
);
};

Expand Down Expand Up @@ -108,7 +111,8 @@ export const useSpanIndexedSeries = <
const useDiscoverSeries = <T extends string[]>(
options: UseMetricsSeriesOptions<T> = {},
dataset: DiscoverDatasets,
referrer: string
referrer: string,
pageFilters?: PageFilters
) => {
const {
search = undefined,
Expand All @@ -117,7 +121,7 @@ const useDiscoverSeries = <T extends string[]>(
samplingMode = DEFAULT_SAMPLING_MODE,
} = options;

const pageFilters = usePageFilters();
const defaultPageFilters = usePageFilters();
const location = useLocation();
const organization = useOrganization();

Expand All @@ -127,7 +131,7 @@ const useDiscoverSeries = <T extends string[]>(
const eventView = getSeriesEventView(
search,
undefined,
pageFilters.selection,
pageFilters || defaultPageFilters.selection,
yAxis,
undefined,
dataset
Expand Down Expand Up @@ -161,7 +165,7 @@ const useDiscoverSeries = <T extends string[]>(
samplingMode === 'NONE' || !shouldSetSamplingMode ? undefined : samplingMode,
}),
options: {
enabled: options.enabled && pageFilters.isReady,
enabled: options.enabled && defaultPageFilters.isReady,
refetchOnWindowFocus: false,
retry: shouldRetryHandler,
retryDelay: getRetryDelay,
Expand Down
Loading