From 5e2457d3a76b203b0b02596379230182e54baf56 Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Fri, 5 Jul 2024 07:55:31 +0000 Subject: [PATCH] Revert "feat(stats): Display subseries in tooltip (#73774)" This reverts commit 479ea5a73109cd24609a80f7785828df8e1b9182. Co-authored-by: priscilawebdev <29228205+priscilawebdev@users.noreply.github.com> --- static/app/components/charts/baseChart.tsx | 4 - .../components/charts/components/tooltip.tsx | 13 +-- .../views/organizationStats/index.spec.tsx | 6 +- .../organizationStats/usageChart/index.tsx | 2 +- .../views/organizationStats/usageStatsOrg.tsx | 89 ++++--------------- 5 files changed, 20 insertions(+), 94 deletions(-) diff --git a/static/app/components/charts/baseChart.tsx b/static/app/components/charts/baseChart.tsx index 16f8732ed1c86b..080476692b56ff 100644 --- a/static/app/components/charts/baseChart.tsx +++ b/static/app/components/charts/baseChart.tsx @@ -111,10 +111,6 @@ interface TooltipOption name: string, seriesParams?: TooltipComponentFormatterCallback ) => string; - /** - * If true does not display sublabels with a value of 0. - */ - skipZeroValuedSubLabels?: boolean; /** * Array containing data that is used to display indented sublabels. */ diff --git a/static/app/components/charts/components/tooltip.tsx b/static/app/components/charts/components/tooltip.tsx index 66e65beb91fc78..d80becccada85d 100644 --- a/static/app/components/charts/components/tooltip.tsx +++ b/static/app/components/charts/components/tooltip.tsx @@ -118,10 +118,6 @@ export type FormatterOptions = Pick< * Limit the number of series rendered in the tooltip and display "+X more". */ limit?: number; - /** - * If true does not display sublabels with a value of 0. - */ - skipZeroValuedSubLabels?: boolean; /** * Array containing data that is used to display indented sublabels. */ @@ -142,7 +138,6 @@ export function getFormatter({ subLabels = [], addSecondsToTimeFormat = false, limit, - skipZeroValuedSubLabels, }: FormatterOptions): TooltipComponentFormatterCallback { const getFilter = (seriesParam: any) => { // Series do not necessarily have `data` defined, e.g. releases don't have `data`, but rather @@ -260,17 +255,13 @@ export function getFormatter({ for (const subLabel of filteredSubLabels) { const serieValue = subLabel.data[serie.dataIndex].value; - if (skipZeroValuedSubLabels && serieValue === 0) { - continue; - } - labelWithSubLabels.push( `
${ subLabel.label } ${valueFormatter(serieValue)}
` ); - acc.total = acc.total + serieValue; + acc.total = acc.total + subLabel.data[serie.dataIndex].value; } acc.series.push(labelWithSubLabels.join('')); @@ -345,7 +336,6 @@ export function computeChartTooltip( hideDelay, subLabels, chartId, - skipZeroValuedSubLabels, ...props }: Props, theme: Theme @@ -365,7 +355,6 @@ export function computeChartTooltip( nameFormatter, markerFormatter, subLabels, - skipZeroValuedSubLabels, }); return { diff --git a/static/app/views/organizationStats/index.spec.tsx b/static/app/views/organizationStats/index.spec.tsx index 8c67e74f033771..dee37c3a0229b5 100644 --- a/static/app/views/organizationStats/index.spec.tsx +++ b/static/app/views/organizationStats/index.spec.tsx @@ -115,7 +115,7 @@ describe('OrganizationStats', function () { UsageStatsOrg: { statsPeriod: DEFAULT_STATS_PERIOD, interval: '1h', - groupBy: ['category', 'outcome', 'reason'], + groupBy: ['category', 'outcome'], project: [-1], field: ['sum(quantity)'], }, @@ -314,7 +314,7 @@ describe('OrganizationStats', function () { query: { statsPeriod: DEFAULT_STATS_PERIOD, interval: '1h', - groupBy: ['category', 'outcome', 'reason'], + groupBy: ['category', 'outcome'], project: selectedProjects, field: ['sum(quantity)'], }, @@ -355,7 +355,7 @@ describe('OrganizationStats', function () { query: { statsPeriod: DEFAULT_STATS_PERIOD, interval: '1h', - groupBy: ['category', 'outcome', 'reason'], + groupBy: ['category', 'outcome'], project: selectedProject, field: ['sum(quantity)'], }, diff --git a/static/app/views/organizationStats/usageChart/index.tsx b/static/app/views/organizationStats/usageChart/index.tsx index e2ee2109f82286..178c38eead9b9a 100644 --- a/static/app/views/organizationStats/usageChart/index.tsx +++ b/static/app/views/organizationStats/usageChart/index.tsx @@ -118,7 +118,7 @@ export const CHART_OPTIONS_DATA_TRANSFORM: SelectValue[] = [ }, ]; -export const enum SeriesTypes { +const enum SeriesTypes { ACCEPTED = 'Accepted', DROPPED = 'Dropped', PROJECTED = 'Projected', diff --git a/static/app/views/organizationStats/usageStatsOrg.tsx b/static/app/views/organizationStats/usageStatsOrg.tsx index e4a0716f2ee7c6..23d4dd25a316d3 100644 --- a/static/app/views/organizationStats/usageStatsOrg.tsx +++ b/static/app/views/organizationStats/usageStatsOrg.tsx @@ -4,11 +4,9 @@ import type {WithRouterProps} from 'react-router'; import styled from '@emotion/styled'; import * as Sentry from '@sentry/react'; import isEqual from 'lodash/isEqual'; -import startCase from 'lodash/startCase'; import moment from 'moment'; import {navigateTo} from 'sentry/actionCreators/navigation'; -import type {TooltipSubLabel} from 'sentry/components/charts/components/tooltip'; import OptionSelector from 'sentry/components/charts/optionSelector'; import {InlineContainer, SectionHeading} from 'sentry/components/charts/styles'; import type {DateTimeObject} from 'sentry/components/charts/utils'; @@ -32,11 +30,7 @@ import { } from './usageChart/utils'; import type {UsageSeries, UsageStat} from './types'; import type {ChartStats, UsageChartProps} from './usageChart'; -import UsageChart, { - CHART_OPTIONS_DATA_TRANSFORM, - ChartDataTransform, - SeriesTypes, -} from './usageChart'; +import UsageChart, {CHART_OPTIONS_DATA_TRANSFORM, ChartDataTransform} from './usageChart'; import UsageStatsPerMin from './usageStatsPerMin'; import {formatUsageWithUnits, getFormatUsageOptions, isDisplayUtc} from './utils'; @@ -121,7 +115,7 @@ class UsageStatsOrganization< return { ...queryDatetime, interval: getSeriesApiInterval(dataDatetime), - groupBy: ['category', 'outcome', 'reason'], + groupBy: ['category', 'outcome'], project: projectIds, field: ['sum(quantity)'], }; @@ -142,7 +136,6 @@ class UsageStatsOrganization< chartDateTimezoneDisplay: string; chartDateUtc: boolean; chartStats: ChartStats; - chartSubLabels: TooltipSubLabel[]; chartTransform: ChartDataTransform; dataError?: Error; } { @@ -239,7 +232,6 @@ class UsageStatsOrganization< chartDateEnd, chartDateUtc, chartTransform, - chartSubLabels, } = this.chartData; const hasError = error || !!dataError; @@ -257,10 +249,6 @@ class UsageStatsOrganization< usageDateShowUtc: chartDateUtc, usageDateInterval: chartDateInterval, usageStats: chartStats, - chartTooltip: { - subLabels: chartSubLabels, - skipZeroValuedSubLabels: true, - }, } as UsageChartProps; return chartProps; @@ -330,7 +318,6 @@ class UsageStatsOrganization< total?: string; }; chartStats: ChartStats; - chartSubLabels: TooltipSubLabel[]; dataError?: Error; } { const cardStats = { @@ -345,10 +332,9 @@ class UsageStatsOrganization< projected: [], filtered: [], }; - const chartSubLabels: TooltipSubLabel[] = []; if (!orgStats) { - return {cardStats, chartStats, chartSubLabels}; + return {cardStats, chartStats}; } try { @@ -397,63 +383,20 @@ class UsageStatsOrganization< count[outcome] += group.totals['sum(quantity)']; group.series['sum(quantity)'].forEach((stat, i) => { - const dataObject = { - name: orgStats.intervals[i], - value: stat, - }; - - const reason = String(group.by.reason ?? ''); - const label = startCase(reason.replace(/-|_/g, ' ')); - const existingSubLabel = chartSubLabels.find( - subLabel => subLabel.label === label - ); - - if (outcome === Outcome.FILTERED) { - usageStats[i][outcome] += stat; - - if (existingSubLabel) { - existingSubLabel.data.push({ - ...dataObject, - value: dataObject.value, - }); + switch (outcome) { + case Outcome.ACCEPTED: + case Outcome.FILTERED: + usageStats[i][outcome] += stat; return; - } - - chartSubLabels.push({ - parentLabel: SeriesTypes.FILTERED, - label, - data: [dataObject], - }); - - return; - } - - if (outcome === Outcome.ACCEPTED) { - usageStats[i][outcome] += stat; - return; - } - - if ( - outcome === Outcome.DROPPED || - outcome === Outcome.RATE_LIMITED || - outcome === Outcome.CARDINALITY_LIMITED || - outcome === Outcome.INVALID - ) { - usageStats[i].dropped.total += stat; - - if (existingSubLabel) { - existingSubLabel.data.push({ - ...dataObject, - value: dataObject.value, - }); + case Outcome.DROPPED: + case Outcome.RATE_LIMITED: + case Outcome.CARDINALITY_LIMITED: + case Outcome.INVALID: + usageStats[i].dropped.total += stat; + // TODO: add client discards to dropped? + return; + default: return; - } - - chartSubLabels.push({ - parentLabel: SeriesTypes.DROPPED, - label, - data: [dataObject], - }); } }); }); @@ -498,7 +441,6 @@ class UsageStatsOrganization< ), }, chartStats, - chartSubLabels, }; } catch (err) { Sentry.withScope(scope => { @@ -510,7 +452,6 @@ class UsageStatsOrganization< return { cardStats, chartStats, - chartSubLabels, dataError: new Error('Failed to parse stats data'), }; }