From eb53f83265de97ee0559b257847b1b62e592f674 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Wed, 3 Jul 2024 17:54:30 +0200 Subject: [PATCH 1/5] feat(stats): Display subseries in tooltip --- .../views/organizationStats/usageStatsOrg.tsx | 85 +++++++++++++++---- 1 file changed, 70 insertions(+), 15 deletions(-) diff --git a/static/app/views/organizationStats/usageStatsOrg.tsx b/static/app/views/organizationStats/usageStatsOrg.tsx index 23d4dd25a316d3..4fa0bdcff6c87b 100644 --- a/static/app/views/organizationStats/usageStatsOrg.tsx +++ b/static/app/views/organizationStats/usageStatsOrg.tsx @@ -4,9 +4,11 @@ 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'; @@ -115,7 +117,7 @@ class UsageStatsOrganization< return { ...queryDatetime, interval: getSeriesApiInterval(dataDatetime), - groupBy: ['category', 'outcome'], + groupBy: ['category', 'outcome', 'reason'], project: projectIds, field: ['sum(quantity)'], }; @@ -136,6 +138,7 @@ class UsageStatsOrganization< chartDateTimezoneDisplay: string; chartDateUtc: boolean; chartStats: ChartStats; + chartSubLabels: TooltipSubLabel[]; chartTransform: ChartDataTransform; dataError?: Error; } { @@ -232,6 +235,7 @@ class UsageStatsOrganization< chartDateEnd, chartDateUtc, chartTransform, + chartSubLabels, } = this.chartData; const hasError = error || !!dataError; @@ -249,6 +253,9 @@ class UsageStatsOrganization< usageDateShowUtc: chartDateUtc, usageDateInterval: chartDateInterval, usageStats: chartStats, + chartTooltip: { + subLabels: chartSubLabels, + }, } as UsageChartProps; return chartProps; @@ -318,6 +325,7 @@ class UsageStatsOrganization< total?: string; }; chartStats: ChartStats; + chartSubLabels: TooltipSubLabel[]; dataError?: Error; } { const cardStats = { @@ -332,9 +340,10 @@ class UsageStatsOrganization< projected: [], filtered: [], }; + const chartSubLabels: TooltipSubLabel[] = []; if (!orgStats) { - return {cardStats, chartStats}; + return {cardStats, chartStats, chartSubLabels}; } try { @@ -383,20 +392,64 @@ class UsageStatsOrganization< count[outcome] += group.totals['sum(quantity)']; group.series['sum(quantity)'].forEach((stat, i) => { - switch (outcome) { - case Outcome.ACCEPTED: - case Outcome.FILTERED: - usageStats[i][outcome] += stat; - return; - 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: + 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 (group.by.outcome === Outcome.FILTERED) { + if (existingSubLabel) { + existingSubLabel.data.push({ + ...dataObject, + value: dataObject.value, + }); + return; + } + + chartSubLabels.push({ + parentLabel: t('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, + }); return; + } + + chartSubLabels.push({ + parentLabel: t('Dropped'), + label, + data: [dataObject], + }); } }); }); @@ -441,6 +494,7 @@ class UsageStatsOrganization< ), }, chartStats, + chartSubLabels, }; } catch (err) { Sentry.withScope(scope => { @@ -452,6 +506,7 @@ class UsageStatsOrganization< return { cardStats, chartStats, + chartSubLabels, dataError: new Error('Failed to parse stats data'), }; } From 5063cdc921388dec754a907335298e19b5e8d42e Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Wed, 3 Jul 2024 23:52:53 +0200 Subject: [PATCH 2/5] remove not needed if --- .../views/organizationStats/usageStatsOrg.tsx | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/static/app/views/organizationStats/usageStatsOrg.tsx b/static/app/views/organizationStats/usageStatsOrg.tsx index 4fa0bdcff6c87b..cfc88926c01dc4 100644 --- a/static/app/views/organizationStats/usageStatsOrg.tsx +++ b/static/app/views/organizationStats/usageStatsOrg.tsx @@ -406,21 +406,20 @@ class UsageStatsOrganization< if (outcome === Outcome.FILTERED) { usageStats[i][outcome] += stat; - if (group.by.outcome === Outcome.FILTERED) { - if (existingSubLabel) { - existingSubLabel.data.push({ - ...dataObject, - value: dataObject.value, - }); - return; - } - - chartSubLabels.push({ - parentLabel: t('Filtered'), - label, - data: [dataObject], + if (existingSubLabel) { + existingSubLabel.data.push({ + ...dataObject, + value: dataObject.value, }); + return; } + + chartSubLabels.push({ + parentLabel: t('Filtered'), + label, + data: [dataObject], + }); + return; } From 92a73918ceb9012f4a99cc74e106a676b38f4ed2 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Thu, 4 Jul 2024 00:06:56 +0200 Subject: [PATCH 3/5] remove translations --- .../app/views/organizationStats/usageChart/index.tsx | 2 +- static/app/views/organizationStats/usageStatsOrg.tsx | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/static/app/views/organizationStats/usageChart/index.tsx b/static/app/views/organizationStats/usageChart/index.tsx index 178c38eead9b9a..e2ee2109f82286 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[] = [ }, ]; -const enum SeriesTypes { +export 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 cfc88926c01dc4..593ba4b531c4d0 100644 --- a/static/app/views/organizationStats/usageStatsOrg.tsx +++ b/static/app/views/organizationStats/usageStatsOrg.tsx @@ -32,7 +32,11 @@ import { } from './usageChart/utils'; import type {UsageSeries, UsageStat} from './types'; import type {ChartStats, UsageChartProps} from './usageChart'; -import UsageChart, {CHART_OPTIONS_DATA_TRANSFORM, ChartDataTransform} from './usageChart'; +import UsageChart, { + CHART_OPTIONS_DATA_TRANSFORM, + ChartDataTransform, + SeriesTypes, +} from './usageChart'; import UsageStatsPerMin from './usageStatsPerMin'; import {formatUsageWithUnits, getFormatUsageOptions, isDisplayUtc} from './utils'; @@ -415,7 +419,7 @@ class UsageStatsOrganization< } chartSubLabels.push({ - parentLabel: t('Filtered'), + parentLabel: SeriesTypes.FILTERED, label, data: [dataObject], }); @@ -445,7 +449,7 @@ class UsageStatsOrganization< } chartSubLabels.push({ - parentLabel: t('Dropped'), + parentLabel: SeriesTypes.DROPPED, label, data: [dataObject], }); From 712fee6fc324f378d3776ca07ea538f4b7edf1e9 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Thu, 4 Jul 2024 17:07:36 +0200 Subject: [PATCH 4/5] feedback --- static/app/components/charts/baseChart.tsx | 4 ++++ static/app/components/charts/components/tooltip.tsx | 13 ++++++++++++- .../app/views/organizationStats/usageStatsOrg.tsx | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/static/app/components/charts/baseChart.tsx b/static/app/components/charts/baseChart.tsx index 080476692b56ff..16f8732ed1c86b 100644 --- a/static/app/components/charts/baseChart.tsx +++ b/static/app/components/charts/baseChart.tsx @@ -111,6 +111,10 @@ 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 d80becccada85d..66e65beb91fc78 100644 --- a/static/app/components/charts/components/tooltip.tsx +++ b/static/app/components/charts/components/tooltip.tsx @@ -118,6 +118,10 @@ 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. */ @@ -138,6 +142,7 @@ 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 @@ -255,13 +260,17 @@ 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 + subLabel.data[serie.dataIndex].value; + acc.total = acc.total + serieValue; } acc.series.push(labelWithSubLabels.join('')); @@ -336,6 +345,7 @@ export function computeChartTooltip( hideDelay, subLabels, chartId, + skipZeroValuedSubLabels, ...props }: Props, theme: Theme @@ -355,6 +365,7 @@ export function computeChartTooltip( nameFormatter, markerFormatter, subLabels, + skipZeroValuedSubLabels, }); return { diff --git a/static/app/views/organizationStats/usageStatsOrg.tsx b/static/app/views/organizationStats/usageStatsOrg.tsx index 593ba4b531c4d0..e4a0716f2ee7c6 100644 --- a/static/app/views/organizationStats/usageStatsOrg.tsx +++ b/static/app/views/organizationStats/usageStatsOrg.tsx @@ -259,6 +259,7 @@ class UsageStatsOrganization< usageStats: chartStats, chartTooltip: { subLabels: chartSubLabels, + skipZeroValuedSubLabels: true, }, } as UsageChartProps; From dd7d70626e03017e7283c9ff267b5528bec8d476 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Fri, 5 Jul 2024 08:36:38 +0200 Subject: [PATCH 5/5] fix test --- static/app/views/organizationStats/index.spec.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/static/app/views/organizationStats/index.spec.tsx b/static/app/views/organizationStats/index.spec.tsx index dee37c3a0229b5..8c67e74f033771 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'], + groupBy: ['category', 'outcome', 'reason'], project: [-1], field: ['sum(quantity)'], }, @@ -314,7 +314,7 @@ describe('OrganizationStats', function () { query: { statsPeriod: DEFAULT_STATS_PERIOD, interval: '1h', - groupBy: ['category', 'outcome'], + groupBy: ['category', 'outcome', 'reason'], project: selectedProjects, field: ['sum(quantity)'], }, @@ -355,7 +355,7 @@ describe('OrganizationStats', function () { query: { statsPeriod: DEFAULT_STATS_PERIOD, interval: '1h', - groupBy: ['category', 'outcome'], + groupBy: ['category', 'outcome', 'reason'], project: selectedProject, field: ['sum(quantity)'], },