Skip to content

Commit

Permalink
feat(slo): Add optional settings (#146470)
Browse files Browse the repository at this point in the history
  • Loading branch information
kdelemme authored Nov 29, 2022
1 parent e1227a3 commit fdcefb7
Show file tree
Hide file tree
Showing 24 changed files with 209 additions and 22 deletions.
5 changes: 5 additions & 0 deletions x-pack/plugins/observability/dev_docs/slo.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ For example, defining a **timeslices** budgeting method with a `95%` slice thres
The target objective is the value the SLO needs to meet during the time window.
If a **timeslices** budgeting method is used, we also need to define the **timeslice_target** which can be different than the overall SLO target.

### Optional settings

The default settings should be sufficient for most users, but if needed, the following properties can be overwritten:
- timestamp_field: The date time field to use from the source index
- sync_delay: The ingest delay in the source data
- frequency: How often do we query the source data

## Example

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,34 @@ import {
TransformPivot,
TransformPutTransformRequest,
TransformSource,
TransformTimeSync,
} from '@elastic/elasticsearch/lib/api/types';

export interface TransformSettings {
frequency: TransformPutTransformRequest['frequency'];
sync_field: TransformTimeSync['field'];
sync_delay: TransformTimeSync['delay'];
}

export const getSLOTransformTemplate = (
transformId: string,
source: TransformSource,
destination: TransformDestination,
groupBy: TransformPivot['group_by'] = {},
aggregations: TransformPivot['aggregations'] = {}
aggregations: TransformPivot['aggregations'] = {},
settings: TransformSettings
): TransformPutTransformRequest => ({
transform_id: transformId,
source,
frequency: '1m',
frequency: settings.frequency,
dest: destination,
settings: {
deduce_mappings: false,
},
sync: {
time: {
field: '@timestamp',
delay: '60s',
field: settings.sync_field,
delay: settings.sync_delay,
},
},
pivot: {
Expand Down
23 changes: 23 additions & 0 deletions x-pack/plugins/observability/server/domain/models/duration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,27 @@ describe('Duration', () => {
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);
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', () => {
const short = new Duration(1, DurationUnit.Minute);
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Minute))).toBe(true);
expect(short.isLongerOrEqualThan(new Duration(1, DurationUnit.Hour))).toBe(false);
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);
});
});
});
4 changes: 4 additions & 0 deletions x-pack/plugins/observability/server/domain/models/duration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ class Duration {
return currentDurationMoment.asSeconds() < otherDurationMoment.asSeconds();
}

isLongerOrEqualThan(other: Duration): boolean {
return !this.isShorterThan(other);
}

format(): string {
return `${this.value}${this.unit}`;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { validateSLO } from '.';
import { oneMinute, sixHours } from '../../services/slo/fixtures/duration';
import { createSLO } from '../../services/slo/fixtures/slo';
import { Duration, DurationUnit } from '../models/duration';

Expand Down Expand Up @@ -34,6 +35,30 @@ describe('validateSLO', () => {
});
expect(() => validateSLO(slo)).toThrowError('Invalid time_window.duration');
});

describe('settings', () => {
it("throws when frequency is longer or equal than '1h'", () => {
const slo = createSLO({
settings: {
frequency: sixHours(),
timestamp_field: '@timestamp',
sync_delay: oneMinute(),
},
});
expect(() => validateSLO(slo)).toThrowError('Invalid settings.frequency');
});

it("throws when sync_delay is longer or equal than '6h'", () => {
const slo = createSLO({
settings: {
frequency: oneMinute(),
timestamp_field: '@timestamp',
sync_delay: sixHours(),
},
});
expect(() => validateSLO(slo)).toThrowError('Invalid settings.sync_delay');
});
});
});

describe('slo with timeslices budgeting method', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ export function validateSLO(slo: SLO) {
throw new IllegalArgumentError('Invalid objective.timeslice_window');
}
}

validateSettings(slo);
}

function validateSettings(slo: SLO) {
if (!isValidFrequencySettings(slo.settings.frequency)) {
throw new IllegalArgumentError('Invalid settings.frequency');
}

if (!isValidSyncDelaySettings(slo.settings.sync_delay)) {
throw new IllegalArgumentError('Invalid settings.sync_delay');
}
}

function isValidTargetNumber(value: number): boolean {
Expand All @@ -63,3 +75,23 @@ function isValidTimesliceWindowDuration(timesliceWindow: Duration, timeWindow: D
timesliceWindow.isShorterThan(timeWindow)
);
}

/**
* validate that 1 minute <= frequency < 1 hour
*/
function isValidFrequencySettings(frequency: Duration): boolean {
return (
frequency.isLongerOrEqualThan(new Duration(1, DurationUnit.Minute)) &&
frequency.isShorterThan(new Duration(1, DurationUnit.Hour))
);
}

/**
* validate that 1 minute <= sync_delay < 6 hour
*/
function isValidSyncDelaySettings(syncDelay: Duration): boolean {
return (
syncDelay.isLongerOrEqualThan(new Duration(1, DurationUnit.Minute)) &&
syncDelay.isShorterThan(new Duration(6, DurationUnit.Hour))
);
}
7 changes: 7 additions & 0 deletions x-pack/plugins/observability/server/saved_objects/slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ export const slo: SavedObjectsType = {
timeslice_window: { type: 'keyword' },
},
},
settings: {
properties: {
timestamp_field: { type: 'keyword' },
sync_delay: { type: 'keyword' },
frequency: { type: 'keyword' },
},
},
revision: { type: 'short' },
created_at: { type: 'date' },
updated_at: { type: 'date' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import uuid from 'uuid';

import { SLO } from '../../domain/models';
import { Duration, DurationUnit, SLO } from '../../domain/models';
import { ResourceInstaller } from './resource_installer';
import { SLORepository } from './slo_repository';
import { TransformManager } from './transform_manager';
Expand Down Expand Up @@ -55,6 +55,11 @@ export class CreateSLO {
return {
...params,
id: uuid.v1(),
settings: {
timestamp_field: params.settings?.timestamp_field ?? '@timestamp',
sync_delay: params.settings?.sync_delay ?? new Duration(1, DurationUnit.Minute),
frequency: params.settings?.frequency ?? new Duration(1, DurationUnit.Minute),
},
revision: 1,
created_at: now,
updated_at: now,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ describe('FindSLO', () => {
duration: '7d',
is_rolling: true,
},
settings: {
timestamp_field: '@timestamp',
sync_delay: '1m',
frequency: '1m',
},
summary: {
sli_value: 0.9999,
error_budget: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { sloSchema } from '../../../types/schema';
import {
APMTransactionDurationIndicator,
APMTransactionErrorRateIndicator,
Duration,
DurationUnit,
Indicator,
KQLCustomIndicator,
SLO,
Expand Down Expand Up @@ -74,6 +76,11 @@ const defaultSLO: Omit<SLO, 'id' | 'revision' | 'created_at' | 'updated_at'> = {
target: 0.999,
},
indicator: createAPMTransactionDurationIndicator(),
settings: {
timestamp_field: '@timestamp',
sync_delay: new Duration(1, DurationUnit.Minute),
frequency: new Duration(1, DurationUnit.Minute),
},
};

export const createSLOParams = (params: Partial<CreateSLOParams> = {}): CreateSLOParams => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ describe('GetSLO', () => {
duration: '7d',
is_rolling: true,
},

settings: {
timestamp_field: '@timestamp',
sync_delay: '1m',
frequency: '1m',
},
summary: {
sli_value: 0.9999,
error_budget: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export class GetSLO {
time_window: slo.time_window,
budgeting_method: slo.budgeting_method,
objective: slo.objective,
settings: slo.settings,
summary: slo.summary,
revision: slo.revision,
created_at: slo.created_at,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,15 @@ export class DefaultSLIClient implements SLIClient {
generateSearchQuery(slo, dateRangeBySlo[slo.id]),
]);

const indicatorDataBySlo: Record<SLOId, IndicatorData> = {};
if (searches.length === 0) {
return indicatorDataBySlo;
}

const result = await this.esClient.msearch<unknown, Record<AggKey, AggregationsSumAggregate>>({
searches,
});

const indicatorDataBySlo: Record<SLOId, IndicatorData> = {};
for (let i = 0; i < result.responses.length; i++) {
const slo = sloList[i];
if ('error' in result.responses[i]) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export class ApmTransactionDurationTransformGenerator extends TransformGenerator
this.buildSource(slo, slo.indicator),
this.buildDestination(),
this.buildCommonGroupBy(slo),
this.buildAggregations(slo.indicator)
this.buildAggregations(slo.indicator),
this.buildSettings(slo)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export class ApmTransactionErrorRateTransformGenerator extends TransformGenerato
this.buildSource(slo, slo.indicator),
this.buildDestination(),
this.buildCommonGroupBy(slo),
this.buildAggregations(slo, slo.indicator)
this.buildAggregations(slo, slo.indicator),
this.buildSettings(slo)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export class KQLCustomTransformGenerator extends TransformGenerator {
this.buildSource(slo, slo.indicator),
this.buildDestination(),
this.buildCommonGroupBy(slo),
this.buildAggregations(slo, slo.indicator)
this.buildAggregations(slo, slo.indicator),
this.buildSettings(slo)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
AggregationsCalendarInterval,
TransformPutTransformRequest,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { TransformSettings } from '../../../assets/transform_templates/slo_transform_template';
import {
calendarAlignedTimeWindowSchema,
rollingTimeWindowSchema,
Expand Down Expand Up @@ -140,12 +141,21 @@ export abstract class TransformGenerator {
},
},
}),
// Field used in the destination index, using @timestamp as per mapping definition
'@timestamp': {
date_histogram: {
field: '@timestamp',
field: slo.settings.timestamp_field,
calendar_interval: '1m' as AggregationsCalendarInterval,
},
},
};
}

public buildSettings(slo: SLO): TransformSettings {
return {
frequency: slo.settings.frequency.format(),
sync_field: slo.settings.timestamp_field,
sync_delay: slo.settings.sync_delay.format(),
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,25 @@ describe('UpdateSLO', () => {
});

describe('with breaking changes', () => {
it('consideres settings as a breaking change', async () => {
const slo = createSLO();
mockRepository.findById.mockResolvedValueOnce(slo);

const newSettings = { ...slo.settings, timestamp_field: 'newField' };
await updateSLO.execute(slo.id, { settings: newSettings });

expectDeletionOfObsoleteSLOData(slo);
expect(mockRepository.save).toBeCalledWith(
expect.objectContaining({
...slo,
settings: newSettings,
revision: 2,
updated_at: expect.anything(),
})
);
expectInstallationOfNewSLOTransform();
});

it('removes the obsolete data from the SLO previous revision', async () => {
const slo = createSLO({
indicator: createAPMTransactionErrorRateIndicator({ environment: 'development' }),
Expand Down
Loading

0 comments on commit fdcefb7

Please sign in to comment.