diff --git a/.changeset/cost-insights-wild-cars-wait.md b/.changeset/cost-insights-wild-cars-wait.md new file mode 100644 index 0000000000..24f84dc56b --- /dev/null +++ b/.changeset/cost-insights-wild-cars-wait.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-cost-insights': patch +--- + +Fix savings/excess display calculation diff --git a/plugins/cost-insights/src/client.ts b/plugins/cost-insights/src/client.ts index 740a44fcbd..2180385424 100644 --- a/plugins/cost-insights/src/client.ts +++ b/plugins/cost-insights/src/client.ts @@ -16,11 +16,9 @@ /* eslint-disable no-restricted-imports */ import dayjs from 'dayjs'; -import regression, { DataPoint } from 'regression'; import { CostInsightsApi, ProductInsightsOptions } from '../src/api'; import { Alert, - ChangeStatistic, Cost, DateAggregation, DEFAULT_DATE_FORMAT, @@ -30,17 +28,14 @@ import { MetricData, Project, ProjectGrowthData, - Trendline, UnlabeledDataflowData, } from '../src/types'; import { ProjectGrowthAlert, UnlabeledDataflowAlert, } from '../src/utils/alerts'; -import { - exclusiveEndDateOf, - inclusiveStartDateOf, -} from '../src/utils/duration'; +import { inclusiveStartDateOf } from '../src/utils/duration'; +import { trendlineOf, changeOf } from './utils/mockData'; type IntervalFields = { duration: Duration; @@ -66,7 +61,7 @@ function aggregationFor( baseline: number, ): DateAggregation[] { const { duration, endDate } = parseIntervals(intervals); - const days = dayjs(exclusiveEndDateOf(duration, endDate)).diff( + const days = dayjs(endDate).diff( inclusiveStartDateOf(duration, endDate), 'day', ); @@ -86,32 +81,6 @@ function aggregationFor( ); } -function trendlineOf(aggregation: DateAggregation[]): Trendline { - const data: ReadonlyArray = aggregation.map(a => [ - Date.parse(a.date) / 1000, - a.amount, - ]); - const result = regression.linear(data, { precision: 5 }); - return { - slope: result.equation[0], - intercept: result.equation[1], - }; -} - -function changeOf(aggregation: DateAggregation[]): ChangeStatistic { - const half = Math.ceil(aggregation.length / 2); - const before = aggregation - .slice(0, half) - .reduce((sum, a) => sum + a.amount, 0); - const after = aggregation - .slice(half, aggregation.length) - .reduce((sum, a) => sum + a.amount, 0); - return { - ratio: (after - before) / before, - amount: after - before, - }; -} - export class ExampleCostInsightsClient implements CostInsightsApi { private request(_: any, res: any): Promise { return new Promise(resolve => setTimeout(resolve, 0, res)); diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx index 8f52a0070b..81dc118ea9 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx @@ -22,7 +22,12 @@ import { CostOverviewHeader } from './CostOverviewHeader'; import { LegendItem } from '../LegendItem'; import { MetricSelect } from '../MetricSelect'; import { PeriodSelect } from '../PeriodSelect'; -import { useScroll, useFilters, useConfig } from '../../hooks'; +import { + useScroll, + useFilters, + useConfig, + useLastCompleteBillingDate, +} from '../../hooks'; import { mapFiltersToProps } from './selector'; import { DefaultNavigation } from '../../utils/navigation'; import { formatPercent } from '../../utils/formatters'; @@ -41,6 +46,7 @@ export const CostOverviewCard = ({ }: CostOverviewCardProps) => { const theme = useTheme(); const config = useConfig(); + const lastCompleteBillingDate = useLastCompleteBillingDate(); const { ScrollAnchor } = useScroll(DefaultNavigation.CostOverviewCard); const { setDuration, setProject, setMetric, ...filters } = useFilters( mapFiltersToProps, @@ -50,7 +56,12 @@ export const CostOverviewCard = ({ ? findAlways(config.metrics, m => m.kind === filters.metric) : null; const comparedChange = metricData - ? getComparedChange(dailyCostData, metricData) + ? getComparedChange( + dailyCostData, + metricData, + filters.duration, + lastCompleteBillingDate, + ) : null; return ( diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewChart.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewChart.tsx index 6ed72923ff..1910013581 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewChart.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewChart.tsx @@ -14,7 +14,8 @@ * limitations under the License. */ import React from 'react'; -import moment from 'moment'; +import dayjs from 'dayjs'; +import utc from 'dayjs/plugin/utc'; import { useTheme } from '@material-ui/core'; import { ComposedChart, @@ -50,6 +51,8 @@ import { useCostOverviewStyles as useStyles } from '../../utils/styles'; import { groupByDate, toDataMax, trendFrom } from '../../utils/charts'; import { aggregationSort } from '../../utils/sort'; +dayjs.extend(utc); + type CostOverviewChartProps = { metric: Maybe; metricData: Maybe; @@ -104,7 +107,7 @@ export const CostOverviewChart = ({ if (isInvalid({ label, payload })) return null; const dataKeys = [data.dailyCost.dataKey, data.metric.dataKey]; - const title = moment(label).format(DEFAULT_DATE_FORMAT); + const title = dayjs(label).utc().format(DEFAULT_DATE_FORMAT); const items = payload .filter(p => dataKeys.includes(p.dataKey as string)) .map(p => ({ diff --git a/plugins/cost-insights/src/utils/change.test.ts b/plugins/cost-insights/src/utils/change.test.ts index daf4538f2c..6a0a8e793b 100644 --- a/plugins/cost-insights/src/utils/change.test.ts +++ b/plugins/cost-insights/src/utils/change.test.ts @@ -14,8 +14,15 @@ * limitations under the License. */ -import { growthOf } from './change'; -import { GrowthType, ChangeThreshold, EngineerThreshold } from '../types'; +import { growthOf, getPreviousPeriodTotalCost } from './change'; +import { + GrowthType, + ChangeThreshold, + EngineerThreshold, + Duration, + Cost, +} from '../types'; +import { MockAggregatedDailyCosts, trendlineOf, changeOf } from './mockData'; const GrowthMap = { [GrowthType.Negligible]: 'negligible growth', @@ -60,3 +67,22 @@ describe.each` }); }, ); + +describe('getPreviousPeriodTotalCost', () => { + it('Correctly returns the total cost for the previous period given daily costs', () => { + const mockGroupDailyCost: Cost = { + id: 'test-group', + aggregation: MockAggregatedDailyCosts, + change: changeOf(MockAggregatedDailyCosts), + trendline: trendlineOf(MockAggregatedDailyCosts), + }; + const exclusiveEndDate = '2020-09-30'; + expect( + getPreviousPeriodTotalCost( + mockGroupDailyCost, + Duration.P1M, + exclusiveEndDate, + ), + ).toEqual(100_000); + }); +}); diff --git a/plugins/cost-insights/src/utils/change.ts b/plugins/cost-insights/src/utils/change.ts index cc797bb3b0..1718462948 100644 --- a/plugins/cost-insights/src/utils/change.ts +++ b/plugins/cost-insights/src/utils/change.ts @@ -21,8 +21,14 @@ import { EngineerThreshold, GrowthType, MetricData, + Duration, + DEFAULT_DATE_FORMAT, } from '../types'; -import { aggregationSort } from '../utils/sort'; +import dayjs, { OpUnitType } from 'dayjs'; +import duration from 'dayjs/plugin/duration'; +import { inclusiveStartDateOf } from './duration'; + +dayjs.extend(duration); // Used for displaying status colors export function growthOf(ratio: number, amount?: number) { @@ -45,11 +51,41 @@ export function growthOf(ratio: number, amount?: number) { export function getComparedChange( dailyCost: Cost, metricData: MetricData, + duration: Duration, + lastCompleteBillingDate: string, // YYYY-MM-DD, ): ChangeStatistic { const ratio = dailyCost.change.ratio - metricData.change.ratio; - const amount = dailyCost.aggregation.slice().sort(aggregationSort)[0].amount; + const previousPeriodTotal = getPreviousPeriodTotalCost( + dailyCost, + duration, + lastCompleteBillingDate, + ); return { ratio: ratio, - amount: amount * ratio, + amount: previousPeriodTotal * ratio, }; } + +export function getPreviousPeriodTotalCost( + dailyCost: Cost, + duration: Duration, + inclusiveEndDate: string, +): number { + const dayjsDuration = dayjs.duration(duration); + const startDate = inclusiveStartDateOf( + duration, + dayjs(inclusiveEndDate).add(1, 'day').format(DEFAULT_DATE_FORMAT), + ); + // dayjs doesn't allow adding an ISO 8601 period to dates. + const [amount, type]: [number, OpUnitType] = dayjsDuration.days() + ? [dayjsDuration.days(), 'day'] + : [dayjsDuration.months(), 'month']; + const nextPeriodStart = dayjs(startDate).add(amount, type); + + // Add up costs that incurred before the start of the next period. + return dailyCost.aggregation.reduce((acc, costByDate) => { + return dayjs(costByDate.date).isBefore(nextPeriodStart) + ? acc + costByDate.amount + : acc; + }, 0); +} diff --git a/plugins/cost-insights/src/utils/duration.ts b/plugins/cost-insights/src/utils/duration.ts index 6a880b382c..ae8c255b91 100644 --- a/plugins/cost-insights/src/utils/duration.ts +++ b/plugins/cost-insights/src/utils/duration.ts @@ -22,27 +22,27 @@ import { assertNever } from './assert'; * Derive the start date of a given period, assuming two repeating intervals. * * @param duration see comment on Duration enum - * @param endDate from CostInsightsApi.getLastCompleteBillingDate + * @param endDate from CostInsightsApi.getLastCompleteBillingDate + 1 day */ export function inclusiveStartDateOf( duration: Duration, - endDate: string, + exclusiveEndDate: string, ): string { switch (duration) { case Duration.P30D: case Duration.P90D: - return moment(endDate) + return moment(exclusiveEndDate) .utc() .subtract(moment.duration(duration).add(moment.duration(duration))) .format(DEFAULT_DATE_FORMAT); case Duration.P1M: - return moment(endDate) + return moment(exclusiveEndDate) .utc() .startOf('month') .subtract(moment.duration(duration).add(moment.duration(duration))) .format(DEFAULT_DATE_FORMAT); case Duration.P3M: - return moment(endDate) + return moment(exclusiveEndDate) .utc() .startOf('quarter') .subtract(moment.duration(duration).add(moment.duration(duration))) @@ -54,16 +54,22 @@ export function inclusiveStartDateOf( export function exclusiveEndDateOf( duration: Duration, - endDate: string, + inclusiveEndDate: string, ): string { switch (duration) { case Duration.P30D: case Duration.P90D: - return moment(endDate).utc().add(1, 'day').format(DEFAULT_DATE_FORMAT); + return moment(inclusiveEndDate) + .utc() + .add(1, 'day') + .format(DEFAULT_DATE_FORMAT); case Duration.P1M: - return moment(endDate).utc().startOf('month').format(DEFAULT_DATE_FORMAT); + return moment(inclusiveEndDate) + .utc() + .startOf('month') + .format(DEFAULT_DATE_FORMAT); case Duration.P3M: - return moment(endDate) + return moment(inclusiveEndDate) .utc() .startOf('quarter') .format(DEFAULT_DATE_FORMAT); @@ -74,15 +80,15 @@ export function exclusiveEndDateOf( export function inclusiveEndDateOf( duration: Duration, - endDate: string, + inclusiveEndDate: string, ): string { - return moment(exclusiveEndDateOf(duration, endDate)) + return moment(exclusiveEndDateOf(duration, inclusiveEndDate)) .utc() .subtract(1, 'day') .format(DEFAULT_DATE_FORMAT); } // https://en.wikipedia.org/wiki/ISO_8601#Repeating_intervals -export function intervalsOf(duration: Duration, endDate: string) { - return `R2/${duration}/${exclusiveEndDateOf(duration, endDate)}`; +export function intervalsOf(duration: Duration, inclusiveEndDate: string) { + return `R2/${duration}/${exclusiveEndDateOf(duration, inclusiveEndDate)}`; } diff --git a/plugins/cost-insights/src/utils/formatters.ts b/plugins/cost-insights/src/utils/formatters.ts index bf8e26ef5f..e81fc1f7c9 100644 --- a/plugins/cost-insights/src/utils/formatters.ts +++ b/plugins/cost-insights/src/utils/formatters.ts @@ -15,7 +15,7 @@ */ import moment from 'moment'; -import { Duration } from '../types'; +import { Duration, DEFAULT_DATE_FORMAT } from '../types'; import { inclusiveEndDateOf, inclusiveStartDateOf } from '../utils/duration'; import { pluralOf } from '../utils/grammar'; @@ -84,21 +84,27 @@ export function formatPercent(n: number): string { return `${(n * 100).toFixed(0)}%`; } -export function formatLastTwoLookaheadQuarters(endDate: string) { - const start = moment(inclusiveStartDateOf(Duration.P3M, endDate)).format( - '[Q]Q YYYY', - ); - const end = moment(inclusiveEndDateOf(Duration.P3M, endDate)).format( +export function formatLastTwoLookaheadQuarters(inclusiveEndDate: string) { + const exclusiveEndDate = moment(inclusiveEndDate) + .add(1, 'day') + .format(DEFAULT_DATE_FORMAT); + const start = moment( + inclusiveStartDateOf(Duration.P3M, exclusiveEndDate), + ).format('[Q]Q YYYY'); + const end = moment(inclusiveEndDateOf(Duration.P3M, inclusiveEndDate)).format( '[Q]Q YYYY', ); return `${start} vs ${end}`; } -export function formatLastTwoMonths(endDate: string) { - const start = moment(inclusiveStartDateOf(Duration.P1M, endDate)) +export function formatLastTwoMonths(inclusiveEndDate: string) { + const exclusiveEndDate = moment(inclusiveEndDate) + .add(1, 'day') + .format(DEFAULT_DATE_FORMAT); + const start = moment(inclusiveStartDateOf(Duration.P1M, exclusiveEndDate)) .utc() .format('MMMM'); - const end = moment(inclusiveEndDateOf(Duration.P1M, endDate)) + const end = moment(inclusiveEndDateOf(Duration.P1M, inclusiveEndDate)) .utc() .format('MMMM'); return `${start} vs ${end}`; diff --git a/plugins/cost-insights/src/utils/mockData.ts b/plugins/cost-insights/src/utils/mockData.ts index ddeb4636b9..708ce733dd 100644 --- a/plugins/cost-insights/src/utils/mockData.ts +++ b/plugins/cost-insights/src/utils/mockData.ts @@ -14,16 +14,20 @@ * limitations under the License. */ +import regression, { DataPoint } from 'regression'; import { Config } from '@backstage/config'; import { ConfigApi } from '@backstage/core'; import { + ChangeStatistic, Duration, Entity, Product, ProductFilters, ProjectGrowthData, + Trendline, UnlabeledDataflowAlertProject, UnlabeledDataflowData, + DateAggregation, } from '../types'; import { DefaultLoadingAction, @@ -185,3 +189,276 @@ export const MockCostInsightsConfig: Partial = { getConfig: () => MockProductsConfig as Config, getOptionalConfig: () => MockMetricsConfig as Config, }; + +export function trendlineOf(aggregation: DateAggregation[]): Trendline { + const data: ReadonlyArray = aggregation.map(a => [ + Date.parse(a.date) / 1000, + a.amount, + ]); + const result = regression.linear(data, { precision: 5 }); + return { + slope: result.equation[0], + intercept: result.equation[1], + }; +} + +export function changeOf(aggregation: DateAggregation[]): ChangeStatistic { + const half = Math.ceil(aggregation.length / 2); + const before = aggregation + .slice(0, half) + .reduce((sum, a) => sum + a.amount, 0); + const after = aggregation + .slice(half, aggregation.length) + .reduce((sum, a) => sum + a.amount, 0); + return { + ratio: (after - before) / before, + amount: after - before, + }; +} + +export const MockAggregatedDailyCosts: DateAggregation[] = [ + { + date: '2020-08-07', + amount: 3500, + }, + { + date: '2020-08-06', + amount: 2500, + }, + { + date: '2020-08-05', + amount: 1400, + }, + { + date: '2020-08-04', + amount: 3800, + }, + { + date: '2020-08-09', + amount: 1900, + }, + { + date: '2020-08-08', + amount: 2400, + }, + { + date: '2020-08-03', + amount: 4000, + }, + { + date: '2020-08-02', + amount: 3700, + }, + { + date: '2020-08-01', + amount: 2500, + }, + { + date: '2020-08-18', + amount: 4300, + }, + { + date: '2020-08-17', + amount: 1500, + }, + { + date: '2020-08-16', + amount: 3600, + }, + { + date: '2020-08-15', + amount: 2200, + }, + { + date: '2020-08-19', + amount: 3900, + }, + { + date: '2020-08-10', + amount: 4100, + }, + { + date: '2020-08-14', + amount: 3600, + }, + { + date: '2020-08-13', + amount: 2900, + }, + { + date: '2020-08-12', + amount: 2700, + }, + { + date: '2020-08-11', + amount: 5100, + }, + { + date: '2020-09-19', + amount: 1200, + }, + { + date: '2020-09-18', + amount: 6500, + }, + { + date: '2020-09-17', + amount: 2500, + }, + { + date: '2020-09-16', + amount: 1400, + }, + { + date: '2020-09-11', + amount: 2300, + }, + { + date: '2020-09-10', + amount: 1900, + }, + { + date: '2020-09-15', + amount: 3100, + }, + { + date: '2020-09-14', + amount: 4500, + }, + { + date: '2020-09-13', + amount: 3300, + }, + { + date: '2020-09-12', + amount: 2800, + }, + { + date: '2020-09-29', + amount: 2600, + }, + { + date: '2020-09-28', + amount: 4100, + }, + { + date: '2020-09-27', + amount: 3800, + }, + { + date: '2020-09-22', + amount: 3700, + }, + { + date: '2020-09-21', + amount: 2700, + }, + { + date: '2020-09-20', + amount: 2200, + }, + { + date: '2020-09-26', + amount: 3300, + }, + { + date: '2020-09-25', + amount: 4000, + }, + { + date: '2020-09-24', + amount: 3800, + }, + { + date: '2020-09-23', + amount: 4100, + }, + { + date: '2020-08-29', + amount: 4400, + }, + { + date: '2020-08-28', + amount: 5000, + }, + { + date: '2020-08-27', + amount: 4900, + }, + { + date: '2020-08-26', + amount: 4100, + }, + { + date: '2020-08-21', + amount: 3700, + }, + { + date: '2020-08-20', + amount: 2200, + }, + { + date: '2020-08-25', + amount: 1700, + }, + { + date: '2020-08-24', + amount: 2100, + }, + { + date: '2020-08-23', + amount: 3100, + }, + { + date: '2020-08-22', + amount: 1500, + }, + { + date: '2020-09-08', + amount: 2900, + }, + { + date: '2020-09-07', + amount: 4100, + }, + { + date: '2020-09-06', + amount: 3600, + }, + { + date: '2020-09-05', + amount: 3300, + }, + { + date: '2020-09-09', + amount: 2800, + }, + { + date: '2020-08-31', + amount: 3400, + }, + { + date: '2020-08-30', + amount: 4300, + }, + { + date: '2020-09-04', + amount: 6100, + }, + { + date: '2020-09-03', + amount: 2500, + }, + { + date: '2020-09-02', + amount: 4900, + }, + { + date: '2020-09-01', + amount: 6100, + }, + { + date: '2020-09-30', + amount: 5500, + }, +];