sloth icon indicating copy to clipboard operation
sloth copied to clipboard

Issues when generating SLOs with custom period window

Open Haegi opened this issue 3 years ago • 0 comments

Issue

When specifying a custom slo period window, the sloth cli returns some technical errors and not the expected behaviour. Even when following the guide on the website, it doesn't work properly. Especially without altering enabled, the page & ticket windows are still required in the AlertWindows.

Expected behaviour

It should be easy to generate SLOs with custom period windows (e.g. 7d or 14d)

Steps to reproduce

Scenario 1: Just specifying the default-slo-period

sloth generate --default-slo-period="14d" -i ./examples/getting-started.yml
...
error: "generate" command failed: invalid default slo period: window period 336h0m0s missing%

Very technical error

Scenario 2: Specifying the default-slo-period and slo-period-windows-path

sloth generate --default-slo-period="7d" --slo-period-windows-path=./examples/windows -i ./examples/getting-started.yml

Works

Scenario 3: Specifying the default-slo-period and slo-period-windows-path without alerting/paging

Change 7d.yaml to not include alerting/paging

apiVersion: sloth.slok.dev/v1
kind: AlertWindows
spec:
  sloPeriod: 7d
sloth generate --default-slo-period="7d" --slo-period-windows-path=./examples/windows -i ./examples/no-alerts.yml
(error: "generate" command failed: could not load SLO period windows repository: could not initialize custom windows: could not discover period windows: could not load "7d.yaml" alert windows: invalid alerting window: invalid page quick: long window is required%

It seems like the page & ticket section in the AlertWindows is needed even though it is not used in the SLO spec.

Ideas

Improve error messages

  • Throw a proper error message when custom window can't be found
  • Throw a proper error message when the page/ticket section in the AlertWindows is missing (make it required)

Don't throw an error if altering is not needed

  • When the alerting is disabled in the SLO spec, it should not be required in the AlertWindows section

Haegi avatar Dec 22 '22 08:12 Haegi