Skip to content
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

Improve time window handling and validation #161978

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
14 changes: 2 additions & 12 deletions x-pack/packages/kbn-slo-schema/src/models/duration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ describe('Duration', () => {
expect(new Duration(1, DurationUnit.Day).format()).toBe('1d');
expect(new Duration(1, DurationUnit.Week).format()).toBe('1w');
expect(new Duration(1, DurationUnit.Month).format()).toBe('1M');
expect(new Duration(1, DurationUnit.Quarter).format()).toBe('1Q');
expect(new Duration(1, DurationUnit.Year).format()).toBe('1Y');
});
});

Expand All @@ -39,31 +37,25 @@ describe('Duration', () => {
expect(short.isShorterThan(new Duration(1, DurationUnit.Day))).toBe(true);
expect(short.isShorterThan(new Duration(1, DurationUnit.Week))).toBe(true);
expect(short.isShorterThan(new Duration(1, DurationUnit.Month))).toBe(true);
expect(short.isShorterThan(new Duration(1, DurationUnit.Quarter))).toBe(true);
expect(short.isShorterThan(new Duration(1, DurationUnit.Year))).toBe(true);
});

it('returns false when the current duration is longer (or equal) than the other duration', () => {
const long = new Duration(1, DurationUnit.Year);
const long = new Duration(1, DurationUnit.Month);
expect(long.isShorterThan(new Duration(1, DurationUnit.Minute))).toBe(false);
expect(long.isShorterThan(new Duration(1, DurationUnit.Hour))).toBe(false);
expect(long.isShorterThan(new Duration(1, DurationUnit.Day))).toBe(false);
expect(long.isShorterThan(new Duration(1, DurationUnit.Week))).toBe(false);
expect(long.isShorterThan(new Duration(1, DurationUnit.Month))).toBe(false);
expect(long.isShorterThan(new Duration(1, DurationUnit.Quarter))).toBe(false);
expect(long.isShorterThan(new Duration(1, DurationUnit.Year))).toBe(false);
});
});

describe('isLongerOrEqualThan', () => {
it('returns true when the current duration is longer or equal than the other duration', () => {
const long = new Duration(2, DurationUnit.Year);
const long = new Duration(2, DurationUnit.Month);
expect(long.isLongerOrEqualThan(new Duration(1, DurationUnit.Hour))).toBe(true);
expect(long.isLongerOrEqualThan(new Duration(1, DurationUnit.Day))).toBe(true);
expect(long.isLongerOrEqualThan(new Duration(1, DurationUnit.Week))).toBe(true);
expect(long.isLongerOrEqualThan(new Duration(1, DurationUnit.Month))).toBe(true);
expect(long.isLongerOrEqualThan(new Duration(1, DurationUnit.Quarter))).toBe(true);
expect(long.isLongerOrEqualThan(new Duration(1, DurationUnit.Year))).toBe(true);
});

it('returns false when the current duration is shorter than the other duration', () => {
Expand All @@ -73,8 +65,6 @@ describe('Duration', () => {
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Day))).toBe(false);
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Week))).toBe(false);
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Month))).toBe(false);
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Quarter))).toBe(false);
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Year))).toBe(false);
});
});

Expand Down
10 changes: 0 additions & 10 deletions x-pack/packages/kbn-slo-schema/src/models/duration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ enum DurationUnit {
'Day' = 'd',
'Week' = 'w',
'Month' = 'M',
'Quarter' = 'Q',
'Year' = 'Y',
}

class Duration {
Expand Down Expand Up @@ -73,10 +71,6 @@ const toDurationUnit = (unit: string): DurationUnit => {
return DurationUnit.Week;
case 'M':
return DurationUnit.Month;
case 'Q':
return DurationUnit.Quarter;
case 'y':
return DurationUnit.Year;
default:
throw new Error('invalid duration unit');
}
Expand All @@ -94,10 +88,6 @@ const toMomentUnitOfTime = (unit: DurationUnit): moment.unitOfTime.Diff => {
return 'weeks';
case DurationUnit.Month:
return 'months';
case DurationUnit.Quarter:
return 'quarters';
case DurationUnit.Year:
return 'years';
default:
assertNever(unit);
}
Expand Down
4 changes: 0 additions & 4 deletions x-pack/packages/kbn-slo-schema/src/schema/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ const summarySchema = t.type({
errorBudget: errorBudgetSchema,
});

type SummarySchema = t.TypeOf<typeof summarySchema>;

const historicalSummarySchema = t.intersection([
t.type({
date: dateType,
Expand All @@ -59,8 +57,6 @@ const previewDataSchema = t.type({

const dateRangeSchema = t.type({ from: dateType, to: dateType });

export type { SummarySchema };

export {
ALL_VALUE,
allOrAnyString,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { rollingTimeWindowTypeSchema, SLOWithSummaryResponse } from '@kbn/slo-sc
import { euiLightVars } from '@kbn/ui-theme';
import moment from 'moment';
import React from 'react';
import { toMomentUnitOfTime } from '../../../../utils/slo/duration';
import { toCalendarAlignedMomentUnitOfTime } from '../../../../utils/slo/duration';
import { toDurationLabel } from '../../../../utils/slo/labels';

export interface Props {
Expand All @@ -34,11 +34,11 @@ export function SloTimeWindowBadge({ slo }: Props) {
);
}

const unitMoment = toMomentUnitOfTime(unit);
const unitMoment = toCalendarAlignedMomentUnitOfTime(unit);
const now = moment.utc();

const periodStart = now.clone().startOf(unitMoment!).add(1, 'day');
const periodEnd = now.clone().endOf(unitMoment!).add(1, 'day');
const periodStart = now.clone().startOf(unitMoment);
const periodEnd = now.clone().endOf(unitMoment);

const totalDurationInDays = periodEnd.diff(periodStart, 'days') + 1;
const elapsedDurationInDays = now.diff(periodStart, 'days') + 1;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/observability/public/typings/slo/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { RuleTypeParams } from '@kbn/alerting-plugin/common';

type DurationUnit = 'm' | 'h' | 'd' | 'w' | 'M' | 'Y';
type DurationUnit = 'm' | 'h' | 'd' | 'w' | 'M';

interface Duration {
value: number;
Expand Down
17 changes: 5 additions & 12 deletions x-pack/plugins/observability/public/utils/slo/duration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,17 @@ export function toMinutes(duration: Duration) {
return duration.value * 7 * 24 * 60;
case 'M':
return duration.value * 30 * 24 * 60;
case 'Y':
return duration.value * 365 * 24 * 60;
default:
assertNever(duration.unit);
}

assertNever(duration.unit);
}

export function toMomentUnitOfTime(unit: string): moment.unitOfTime.Diff | undefined {
export function toCalendarAlignedMomentUnitOfTime(unit: string): moment.unitOfTime.StartOf {
switch (unit) {
case 'd':
return 'days';
default:
case 'w':
return 'weeks';
return 'isoWeek';
Copy link
Member

Choose a reason for hiding this comment

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

👍

case 'M':
return 'months';
case 'Q':
return 'quarters';
case 'Y':
return 'years';
}
}
11 changes: 0 additions & 11 deletions x-pack/plugins/observability/public/utils/slo/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,6 @@ export function toDurationLabel(durationStr: string): string {
duration: duration.value,
},
});
case 'Y':
return i18n.translate('xpack.observability.slo.duration.year', {
defaultMessage: '{duration, plural, one {1 year} other {# years}}',
values: {
duration: duration.value,
},
});
}
}

Expand Down Expand Up @@ -146,9 +139,5 @@ export function toDurationAdverbLabel(durationStr: string): string {
return i18n.translate('xpack.observability.slo.duration.monthly', {
defaultMessage: 'Monthly',
});
case 'Y':
return i18n.translate('xpack.observability.slo.duration.yearly', {
defaultMessage: 'Yearly',
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,42 @@
* 2.0.
*/

import {
calendarAlignedTimeWindowSchema,
rollingTimeWindowSchema,
timeWindowSchema,
} from '@kbn/slo-schema';
import moment from 'moment';
import * as t from 'io-ts';
import { rollingTimeWindowSchema, timeWindowSchema } from '@kbn/slo-schema';

type TimeWindow = t.TypeOf<typeof timeWindowSchema>;
type RollingTimeWindow = t.TypeOf<typeof rollingTimeWindowSchema>;
type CalendarAlignedTimeWindow = t.TypeOf<typeof calendarAlignedTimeWindowSchema>;

export type { RollingTimeWindow, TimeWindow };
export type { RollingTimeWindow, TimeWindow, CalendarAlignedTimeWindow };

export function toCalendarAlignedTimeWindowMomentUnit(
timeWindow: CalendarAlignedTimeWindow
): moment.unitOfTime.StartOf {
const unit = timeWindow.duration.unit;
switch (unit) {
case 'w':
return 'isoWeeks';
case 'M':
return 'months';
default:
throw new Error(`Invalid calendar aligned time window duration unit: ${unit}`);
}
}

export function toRollingTimeWindowMomentUnit(
timeWindow: RollingTimeWindow
): moment.unitOfTime.Diff {
const unit = timeWindow.duration.unit;
switch (unit) {
case 'd':
return 'days';
default:
throw new Error(`Invalid rolling time window duration unit: ${unit}`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
import { computeBurnRate } from './compute_burn_rate';
import { toDateRange } from './date_range';
import { createSLO } from '../../services/slo/fixtures/slo';
import { sixHoursRolling } from '../../services/slo/fixtures/time_window';
import { ninetyDaysRolling } from '../../services/slo/fixtures/time_window';

describe('computeBurnRate', () => {
it('computes 0 when total is 0', () => {
expect(
computeBurnRate(createSLO(), {
good: 10,
total: 0,
dateRange: toDateRange(sixHoursRolling()),
dateRange: toDateRange(ninetyDaysRolling()),
})
).toEqual(0);
});
Expand All @@ -26,7 +26,7 @@ describe('computeBurnRate', () => {
computeBurnRate(createSLO(), {
good: 9999,
total: 1,
dateRange: toDateRange(sixHoursRolling()),
dateRange: toDateRange(ninetyDaysRolling()),
})
).toEqual(0);
});
Expand All @@ -36,7 +36,7 @@ describe('computeBurnRate', () => {
computeBurnRate(createSLO({ objective: { target: 0.9 } }), {
good: 90,
total: 100,
dateRange: toDateRange(sixHoursRolling()),
dateRange: toDateRange(ninetyDaysRolling()),
})
).toEqual(1);
});
Expand All @@ -46,7 +46,7 @@ describe('computeBurnRate', () => {
computeBurnRate(createSLO({ objective: { target: 0.99 } }), {
good: 90,
total: 100,
dateRange: toDateRange(sixHoursRolling()),
dateRange: toDateRange(ninetyDaysRolling()),
})
).toEqual(10);
});
Expand All @@ -56,7 +56,7 @@ describe('computeBurnRate', () => {
computeBurnRate(createSLO({ objective: { target: 0.8 } }), {
good: 90,
total: 100,
dateRange: toDateRange(sixHoursRolling()),
dateRange: toDateRange(ninetyDaysRolling()),
})
).toEqual(0.5);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,29 @@
* 2.0.
*/

import { TimeWindow } from '../models/time_window';
import { Duration } from '../models';
import {
monthlyCalendarAligned,
ninetyDaysRolling,
sevenDaysRolling,
thirtyDaysRolling,
weeklyCalendarAligned,
} from '../../services/slo/fixtures/time_window';
import { toDateRange } from './date_range';
import { oneMonth, oneQuarter, oneWeek, thirtyDays } from '../../services/slo/fixtures/duration';

const NOW = new Date('2022-08-11T08:31:00.000Z');

describe('toDateRange', () => {
describe('for calendar aligned time window', () => {
it('computes the date range for weekly calendar', () => {
const timeWindow = aCalendarTimeWindow(oneWeek());
const timeWindow = weeklyCalendarAligned();
expect(toDateRange(timeWindow, NOW)).toEqual({
from: new Date('2022-08-08T00:00:00.000Z'),
to: new Date('2022-08-14T23:59:59.999Z'),
});
});

it('computes the date range for monthly calendar', () => {
const timeWindow = aCalendarTimeWindow(oneMonth());
const timeWindow = monthlyCalendarAligned();
expect(toDateRange(timeWindow, NOW)).toEqual({
from: new Date('2022-08-01T00:00:00.000Z'),
to: new Date('2022-08-31T23:59:59.999Z'),
Expand All @@ -33,42 +37,24 @@ describe('toDateRange', () => {

describe('for rolling time window', () => {
it("computes the date range using a '30days' rolling window", () => {
expect(toDateRange(aRollingTimeWindow(thirtyDays()), NOW)).toEqual({
expect(toDateRange(thirtyDaysRolling(), NOW)).toEqual({
from: new Date('2022-07-12T08:31:00.000Z'),
to: new Date('2022-08-11T08:31:00.000Z'),
});
});

it("computes the date range using a 'weekly' rolling window", () => {
expect(toDateRange(aRollingTimeWindow(oneWeek()), NOW)).toEqual({
it("computes the date range using a '7days' rolling window", () => {
expect(toDateRange(sevenDaysRolling(), NOW)).toEqual({
from: new Date('2022-08-04T08:31:00.000Z'),
to: new Date('2022-08-11T08:31:00.000Z'),
});
});

it("computes the date range using a 'monthly' rolling window", () => {
expect(toDateRange(aRollingTimeWindow(oneMonth()), NOW)).toEqual({
from: new Date('2022-07-11T08:31:00.000Z'),
to: new Date('2022-08-11T08:31:00.000Z'),
});
});

it("computes the date range using a 'quarterly' rolling window", () => {
expect(toDateRange(aRollingTimeWindow(oneQuarter()), NOW)).toEqual({
from: new Date('2022-05-11T08:31:00.000Z'),
it("computes the date range using a '90days' rolling window", () => {
expect(toDateRange(ninetyDaysRolling(), NOW)).toEqual({
from: new Date('2022-05-13T08:31:00.000Z'),
to: new Date('2022-08-11T08:31:00.000Z'),
});
});
});
});

function aCalendarTimeWindow(duration: Duration): TimeWindow {
return {
duration,
type: 'calendarAligned',
};
}

function aRollingTimeWindow(duration: Duration): TimeWindow {
return { duration, type: 'rolling' };
}
Loading