Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions static/app/components/charts/baseChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,11 @@ const getTooltipStyles = (p: {theme: Theme}) => css`
justify-content: flex-start;
align-items: baseline;
}
.tooltip-label-centered {
display: flex;
justify-content: center;
align-items: center;
}
.tooltip-code-no-margin {
padding-left: 0;
margin-left: 0;
Expand Down
29 changes: 24 additions & 5 deletions static/app/components/charts/components/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'echarts/lib/component/tooltip';
import type {Theme} from '@emotion/react';
import {useTheme} from '@emotion/react';
import type {TooltipComponentFormatterCallback} from 'echarts';
import type {CallbackDataParams} from 'echarts/types/dist/shared';
import moment from 'moment-timezone';

import type {BaseChart, BaseChartProps} from 'sentry/components/charts/baseChart';
Expand All @@ -11,6 +12,7 @@ import {t} from 'sentry/locale';
import type {DataPoint} from 'sentry/types/echarts';
import {toArray} from 'sentry/utils/array/toArray';
import {getFormattedDate, getTimeFormat} from 'sentry/utils/dates';
import {formatAbbreviatedNumber} from 'sentry/utils/formatters';

export const CHART_TOOLTIP_VIEWPORT_OFFSET = 20;

Expand Down Expand Up @@ -74,7 +76,7 @@ function defaultMarkerFormatter(value: string) {
return value;
}

function getSeriesValue(series: any, offset: number) {
export function getSeriesValue(series: any, offset: number) {
if (!series.data) {
return;
}
Expand Down Expand Up @@ -113,6 +115,10 @@ export type FormatterOptions = Pick<
* If true seconds will be added to the Axis label time format
*/
addSecondsToTimeFormat?: boolean;
/**
* Content that IS NOT part of the series data but is shown on the tooltip.
*/
extraContent?: (seriesParams: CallbackDataParams) => string | null;
/**
* Limit the number of series rendered in the tooltip and display "+X more".
*/
Expand All @@ -138,6 +144,7 @@ export function getFormatter({
valueFormatter = defaultValueFormatter,
nameFormatter = defaultNameFormatter,
markerFormatter = defaultMarkerFormatter,
extraContent,
subLabels = [],
addSecondsToTimeFormat = false,
limit,
Expand Down Expand Up @@ -244,16 +251,28 @@ export function getFormatter({
);

if (serie.seriesType === 'heatmap') {
Comment thread
nikkikapadia marked this conversation as resolved.
const zAxisCountValue = (getSeriesValue(serie, 2) ?? 0).toString();
const zAxisCountValue = formatAbbreviatedNumber(
getSeriesValue(serie, 2) ?? 0,
4,
false
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Heatmap tooltip shows log-transformed Z-axis counts

Medium Severity

When scale="log" (used by MetricsHeatMap), heatMap.tsx stores Z values as Math.log1p(zValue) in the series data. The tooltip reads this transformed value via getSeriesValue(serie, 2) and formats it with formatAbbreviatedNumber, displaying the log-transformed number instead of the actual event count (e.g., ~6.9 instead of 1,000). An inverse transform like Math.expm1 is needed before formatting.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3dac374. Configure here.

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.

going to deal with the log stuff in a different PR after i talk to Will and see how it's being calculated in the backend and how (or if) i'd need to transform it on the frontend.

const yAxisValue = valueFormatter(
getSeriesValue(serie, 1),
serie.seriesName,
serie
);

acc.series.push(
`<div><span class="tooltip-label"><strong>${yAxisValue}</strong></span> ${zAxisCountValue}</div>`
);
const additionalContent = extraContent ? extraContent(serie) : null;

if (additionalContent) {
acc.series.push(
`<div><span class="tooltip-label"><strong>${yAxisValue}</strong></span> ${zAxisCountValue}</div><div><span class="tooltip-label tooltip-label-centered">${additionalContent}</span></div>`
);
} else {
acc.series.push(
`<div><span class="tooltip-label"><strong>${yAxisValue}</strong></span> ${zAxisCountValue}</div>`
);
}

return acc;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,24 @@ import type {
TooltipFormatterCallback,
TopLevelFormatterParams,
} from 'echarts/types/dist/shared';
import type {LocationDescriptor} from 'history';

import {Flex} from '@sentry/scraps/layout';

import {BaseChart} from 'sentry/components/charts/baseChart';
import {getFormatter} from 'sentry/components/charts/components/tooltip';
import {getFormatter, getSeriesValue} from 'sentry/components/charts/components/tooltip';
import {isChartHovered, truncationFormatter} from 'sentry/components/charts/utils';
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
import {t} from 'sentry/locale';
import type {ReactEchartsRef} from 'sentry/types/echarts';
import {useOrganization} from 'sentry/utils/useOrganization';
import {NO_PLOTTABLE_VALUES} from 'sentry/views/dashboards/widgets/common/settings';
import {plottablesCanBeVisualized} from 'sentry/views/dashboards/widgets/plottablesCanBeVisualized';
import {formatTooltipValue} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatTooltipValue';
import {formatXAxisTimestamp} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatXAxisTimestamp';
import {formatYAxisValue} from 'sentry/views/dashboards/widgets/timeSeriesWidget/formatters/formatYAxisValue';
import {FALLBACK_TYPE} from 'sentry/views/dashboards/widgets/timeSeriesWidget/settings';
import {getExploreUrl, type GetExploreUrlArgs} from 'sentry/views/explore/utils';

import {HeatMap} from './plottables/heatMap';
import type {HeatMapPlottable} from './plottables/heatMapPlottable';
Expand All @@ -34,11 +38,16 @@ interface HeatMapWidgetVisualizationProps {
* Experimental! Specify the Z-axis scale type. Logarithmic scales can be much more useful for values with a high range.
*/
scale?: 'linear' | 'log';
/**
* getExploreUrl props that will be used to generate an explore link for the tooltip. Omitting this will not generate an explore link.
*/
tooltipExploreUrlArgs?: Omit<GetExploreUrlArgs, 'organization'>;
}

export function HeatMapWidgetVisualization(props: HeatMapWidgetVisualizationProps) {
const {plottables} = props;
const theme = useTheme();
const organization = useOrganization();

const pageFilters = usePageFilters();
const {start, end, period, utc} = pageFilters.selection.datetime;
Expand Down Expand Up @@ -94,6 +103,11 @@ export function HeatMapWidgetVisualization(props: HeatMapWidgetVisualizationProp
fieldType,
heatMapPlottable.yAxisValueUnit ?? undefined
);

if (bucketSize === 0) {
return yAxisMinValueFormatted;
}

const yAxisMaxValueFormatted = formatTooltipValue(
value + bucketSize,
fieldType,
Expand All @@ -102,6 +116,50 @@ export function HeatMapWidgetVisualization(props: HeatMapWidgetVisualizationProp

return `${yAxisMinValueFormatted} – ${yAxisMaxValueFormatted}`;
},
extraContent: props.tooltipExploreUrlArgs
? function (contentParams) {
// TODO(nikki): the y axis values might have to be changed if it's on a log scale :(
const yAxisBucketSize = heatMapPlottable.heatMapSeries.meta.yAxis.bucketSize;
const yAxisMinValue = getSeriesValue(contentParams, 1) as number;
const yAxisMaxValue = yAxisMinValue + yAxisBucketSize;

const xAxisBucketSize = heatMapPlottable.heatMapSeries.meta.xAxis.bucketSize;
const xAxisMinValue = getSeriesValue(contentParams, 0) as number;
const xAxisMaxValue = xAxisMinValue + xAxisBucketSize * 1000;

const exploreUrlProps: GetExploreUrlArgs = {
selection: {
...pageFilters.selection,
datetime: {
...pageFilters.selection.datetime,
start: new Date(xAxisMinValue),
end: new Date(xAxisMaxValue),
Comment thread
nikkikapadia marked this conversation as resolved.
Outdated
period: null,
},
},
organization,
// TODO(nikki): we're only handling metrics for now but if we're looking to support other explore
// surfaces then we'll need to add more logic here
...props.tooltipExploreUrlArgs,
crossEvents: props.tooltipExploreUrlArgs?.crossEvents?.map(crossEvent => {
if (crossEvent.type === 'metrics') {
return {
...crossEvent,
query:
yAxisBucketSize === 0
? `value:<=${yAxisMinValue}`
: `value:>=${yAxisMinValue} value:<${yAxisMaxValue}`,
};
}
return crossEvent;
}),
};

const tracesLink = getExploreUrl(exploreUrlProps);

return `<a href="${toHref(tracesLink)}">${t('View related traces')}</a>`;
}
: undefined,
truncate: false,
utc: utc ?? false,
})(params, asyncTicket);
Expand All @@ -118,6 +176,8 @@ export function HeatMapWidgetVisualization(props: HeatMapWidgetVisualizationProp
ref={chartRef}
tooltip={{
show: true,
enterable: true,
extraCssText: 'pointer-events: auto !important;',
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
axisPointer: {
show: false,
},
Expand Down Expand Up @@ -197,3 +257,11 @@ export function HeatMapWidgetVisualization(props: HeatMapWidgetVisualizationProp
</Flex>
);
}

function toHref(to: LocationDescriptor): string {
if (typeof to === 'string') {
return to;
}
const {pathname = '', search = '', hash = ''} = to;
return `${pathname}${search}${hash}`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class HeatMap implements HeatMapPlottable {

toSeries(plottingOptions: HeatMapPlottingOptions): SeriesOption[] {
const {heatMapSeries} = this;
const {scale = 'linear'} = plottingOptions;
const {scale = 'linear', theme} = plottingOptions;

return [
{
Expand All @@ -64,7 +64,10 @@ export class HeatMap implements HeatMapPlottable {
];
}),
emphasis: {
disabled: true,
itemStyle: {
borderColor: theme.tokens.content.primary,
borderWidth: 2,
Comment thread
nikkikapadia marked this conversation as resolved.
Outdated
},
},
},
];
Expand Down
18 changes: 17 additions & 1 deletion static/app/views/explore/metrics/metricsHeatMap.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {useMemo} from 'react';
import type {UseQueryResult} from '@tanstack/react-query';

import {t} from 'sentry/locale';
Expand All @@ -12,9 +13,10 @@ import {
useMetricName,
useMetricVisualize,
useMetricVisualizes,
useTraceMetric,
} from 'sentry/views/explore/metrics/metricsQueryParams';
import {STACKED_GRAPH_HEIGHT} from 'sentry/views/explore/metrics/settings';
import {prettifyAggregation} from 'sentry/views/explore/utils';
import {prettifyAggregation, type GetExploreUrlArgs} from 'sentry/views/explore/utils';

interface MetricsHeatMapProps {
actions: React.ReactNode;
Expand All @@ -27,6 +29,7 @@ export function MetricsHeatMap({heatmapResult, actions, title}: MetricsHeatMapPr
const visualizes = useMetricVisualizes();
const metricLabel = useMetricLabel();
const metricName = useMetricName();
const metric = useTraceMetric();

const {data: heatMapSeries, isPending, error} = heatmapResult;

Expand All @@ -36,6 +39,18 @@ export function MetricsHeatMap({heatmapResult, actions, title}: MetricsHeatMapPr
? metricName
: (title ?? metricLabel ?? prettifyAggregation(aggregate) ?? aggregate);

const tooltipExploreUrlArgs: Omit<GetExploreUrlArgs, 'organization'> = useMemo(() => {
return {
crossEvents: [
{
type: 'metrics',
query: '',
metric,
},
],
};
}, [metric]);

return (
<WidgetWrapper>
<Widget
Expand All @@ -52,6 +67,7 @@ export function MetricsHeatMap({heatmapResult, actions, title}: MetricsHeatMapPr
<HeatMapWidgetVisualization
plottables={[new HeatMap(heatMapSeries)]}
scale="log"
tooltipExploreUrlArgs={tooltipExploreUrlArgs}
/>
)
}
Expand Down
Loading