[SLO] Add timeslice metric indicator #168539

simianhacker · 2023-10-10T23:12:18Z

Summary

This PR adds the new Timeslice Metric indicator for SLOs. Due to the nature of these statistical aggregations, this indicator requires the budgeting method to be set to timeslices; we ignore the timeslice threshold in favor of the threshold set in the metric definition.

Users can create SLOs based on the following aggregations:

Average
Min
Max
Sum
Cardinality
Percentile
Document count
Std. Deviation
Last Value

Other notable feature include:

The ability to define an equation which supports basic math and logic
Users can define a threshold based on the equation for good slices vs bad slices

Counter Metric Example

CC: @lucasmoore

apmmachine · 2023-10-10T23:12:35Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…etric-indicator

elasticmachine · 2023-10-12T19:51:15Z

Pinging @elastic/actionable-observability (Team: Actionable Observability)

…etric-indicator

kdelemme · 2023-10-16T20:38:12Z

I'm testing it manually at the moment, I have a few suggestions for the UX:

Can we make the FormRow labels invalid when the nested Input is invalid? Like we do for the index selection for example
In the screenshot below, Aggregation A should be red as well.

Aggregation field	Index Field

Should we also disable the "add metric" and 🗑️ icon when the index is not yet selected?

Screenshot

kdelemme

I've done a first pass on the UI, looking promising. I haven't looked at the backend yet

...observability/docs/openapi/slo/components/schemas/indicator_properties_timeslice_metric.yaml

x-pack/plugins/observability/docs/openapi/slo/bundled.yaml

x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.ts

…as/indicator_properties_timeslice_metric.yaml Co-authored-by: Kevin Delemme <[email protected]>

…etric-indicator

…abel

…etric-indicator

kdelemme

A few comments but otherwise LGTM.
I've tested locally with different aggregation and all seems to work. I have not verified the math, but I trust the ES aggregation queries.

I think we can reuse the threshold in the data preview chart, for the other indicator and use the objective.target 😏 But that can be another PR

...ck/plugins/observability/public/pages/slo_edit/components/custom_metric/metric_indicator.tsx

x-pack/plugins/observability/public/pages/slo_edit/components/timeslice_metric/metric_input.tsx

...servability/public/pages/slo_edit/components/timeslice_metric/timeslice_metric_indicator.tsx

...ns/observability/server/services/slo/aggregations/get_custom_metric_indicator_aggregation.ts

...vability/server/services/slo/aggregations/get_timeslice_metric_indicator_aggregation.test.ts

...observability/server/services/slo/aggregations/get_timeslice_metric_indicator_aggregation.ts

x-pack/plugins/observability/server/services/slo/get_preview_data.ts

x-pack/plugins/observability/server/services/slo/transform_generators/timeslice_metric.test.ts

…timeslice_metric/timeslice_metric_indicator.tsx Co-authored-by: Kevin Delemme <[email protected]>

…erators/timeslice_metric.test.ts Co-authored-by: Kevin Delemme <[email protected]>

…etric-indicator

kdelemme

LGTM!

kibana-ci · 2023-10-17T20:13:54Z

💚 Build Succeeded

Buildkite Build
Commit: 79f5f78

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`observability`	482	486	+4

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/slo-schema`	111	126	+15

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`observability`	1.0MB	1.1MB	+20.3KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/slo-schema`	114	129	+15

History

💛 Build #168651 was flaky 4dcb0fd
💚 Build #168398 succeeded 727b721
💛 Build #167512 was flaky 630ee05
💔 Build #167461 failed 91fdab3
💛 Build #167129 was flaky 0843e0b
💔 Build #166825 failed 4852b6d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kdelemme

@lucasmoore

## Summary This PR adds the new Timeslice Metric indicator for SLOs. Due to the nature of these statistical aggregations, this indicator requires the budgeting method to be set to `timeslices`; we ignore the timeslice threshold in favor of the threshold set in the metric definition. Users can create SLOs based on the following aggregations: - Average - Min - Max - Sum - Cardinality - Percentile - Document count - Std. Deviation - Last Value Other notable feature include: - The ability to define an equation which supports basic math and logic - Users can define a threshold based on the equation for good slices vs bad slices <img width="800" alt="image" src="https://github.com/elastic/kibana/assets/41702/05af1ced-40cd-4a05-9dfc-e83ea7bfb5ab"> ### Counter Metric Example <img width="800" alt="image" src="https://github.com/elastic/kibana/assets/41702/05eb5a0f-3043-493d-8add-df0d3bf8de02"> CC: @lucasmoore --------- Co-authored-by: Kevin Delemme <[email protected]>

[SLO] Add timeslice metric indicator

737b0bd

simianhacker added 3 commits October 10, 2023 17:17

Removing duplicate min agg option

f129d20

removing unused variable from test

4852b6d

Merge branch 'main' of github.com:elastic/kibana into add-timeslice-m…

0843e0b

…etric-indicator

simianhacker added 3 commits October 12, 2023 10:23

Adding OpenAPI docs

634cd45

Merge branch 'main' of github.com:elastic/kibana into add-timeslice-m…

91fdab3

…etric-indicator

fixing copypasta

3b4b423

simianhacker marked this pull request as ready for review October 12, 2023 19:51

simianhacker requested a review from a team as a code owner October 12, 2023 19:51

Merge branch 'main' of github.com:elastic/kibana into add-timeslice-m…

630ee05

…etric-indicator

simianhacker added the release_note:enhancement label Oct 12, 2023

kdelemme self-assigned this Oct 16, 2023

kdelemme reviewed Oct 16, 2023

View reviewed changes

simianhacker and others added 9 commits October 16, 2023 16:46

Update x-pack/plugins/observability/docs/openapi/slo/components/schem…

843245f

…as/indicator_properties_timeslice_metric.yaml Co-authored-by: Kevin Delemme <[email protected]>

Update x-pack/plugins/observability/docs/openapi/slo/components/schem…

3993786

…as/indicator_properties_timeslice_metric.yaml Co-authored-by: Kevin Delemme <[email protected]>

Merge branch 'main' of github.com:elastic/kibana into add-timeslice-m…

28bee35

…etric-indicator

Fixing bundle

3820c2f

Fixing process_slo_form_values

0f04ea3

Disabling add and delete metric buttons when indexPattern is not valid

e95481d

Moving the form rows under the controller and fixing validation for l…

727b721

…abel

Adding size:0 to aggregation queries for preview

c0fc146

Merge branch 'main' of github.com:elastic/kibana into add-timeslice-m…

4dcb0fd

…etric-indicator

kdelemme reviewed Oct 17, 2023

View reviewed changes

simianhacker and others added 5 commits October 17, 2023 12:41

Update x-pack/plugins/observability/public/pages/slo_edit/components/…

546a3bd

…timeslice_metric/timeslice_metric_indicator.tsx Co-authored-by: Kevin Delemme <[email protected]>

Update x-pack/plugins/observability/server/services/slo/transform_gen…

21ad914

…erators/timeslice_metric.test.ts Co-authored-by: Kevin Delemme <[email protected]>

improving test to ensure variables can be repeated

46580c8

Making the switch statement more robust

263a9ca

Merge branch 'main' of github.com:elastic/kibana into add-timeslice-m…

79f5f78

…etric-indicator

kdelemme approved these changes Oct 17, 2023

View reviewed changes

simianhacker merged commit befbe10 into elastic:main Oct 17, 2023

kibanamachine added the backport:skip This commit does not require backporting label Oct 17, 2023

simianhacker mentioned this pull request Oct 26, 2023

[SLO] create timeslice metric indicator #168012

Closed

dedemorton mentioned this pull request Jan 26, 2024

[Request]: Document new Timeslice Metric indicator for SLOs elastic/observability-docs#3580

Closed

simianhacker deleted the add-timeslice-metric-indicator branch April 17, 2024 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SLO] Add timeslice metric indicator #168539

[SLO] Add timeslice metric indicator #168539

simianhacker commented Oct 10, 2023 •

edited

Loading

apmmachine commented Oct 10, 2023

elasticmachine commented Oct 12, 2023

kdelemme commented Oct 16, 2023

kdelemme left a comment

kdelemme left a comment

kdelemme left a comment

kibana-ci commented Oct 17, 2023

API count

[SLO] Add timeslice metric indicator #168539

[SLO] Add timeslice metric indicator #168539

Conversation

simianhacker commented Oct 10, 2023 • edited Loading

Summary

Counter Metric Example

apmmachine commented Oct 10, 2023

🤖 GitHub comments

elasticmachine commented Oct 12, 2023

kdelemme commented Oct 16, 2023

kdelemme left a comment

Choose a reason for hiding this comment

kdelemme left a comment

Choose a reason for hiding this comment

kdelemme left a comment

Choose a reason for hiding this comment

kibana-ci commented Oct 17, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

History

simianhacker commented Oct 10, 2023 •

edited

Loading