fix(slo): Override transform _id to ensure uniqness #198610

kdelemme · 2024-10-31T19:20:26Z

Resolves #198602

🌮 Summary

This PR add a ingest pipeline set processor in the rollup pipelines to overwrite the document _id with the slo.id and slo.revision prefix. This will ensure that two SLO transforms using the same pivot group by fields will yield unique _id and not overwrite each other.

The issue started when we moved slo.id and slo.revision fields to an ingest pipeline to improve the performance of the rollup transform by removing some runtime fields. Unfortunately, the document _id yielded by a transform only uses the pivot field values. Therefore two transforms using with the same pivot fields will yield the same documents, overwriting each other.

Testing

Generate some data node x-pack/scripts/data_forge.js --events-per-cycle 50 --lookback now-7d --dataset fake_stack --install-kibana-assets --kibana-url http://localhost:5601/kibana
Wait for the data to be ingested until now (look in discover)
Create some SLO without group by and with group by
Wait for the data to catch up
Clone both
Assert original and cloned SLOs have SLI data generated

elasticmachine · 2024-10-31T20:00:56Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

…re uniqness

github-actions · 2024-10-31T22:53:29Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

shahzad31

slo.id with spaces would be broken, we need to generate a randomId

shahzad31 · 2024-11-01T07:47:34Z

...k/plugins/observability_solution/slo/server/assets/ingest_templates/slo_pipeline_template.ts

+    {
+      set: {
+        field: '_id',
+        value: `${slo.id}-${slo.revision}-{{{_id}}}`,


we need to make sure this is a valid document id

…m/kdelemme/kibana into fix/revert-slo-id-transform-pivot

...k/plugins/observability_solution/slo/server/assets/ingest_templates/slo_pipeline_template.ts

shahzad31

LGTM !!

Things looks normal across multiple SLOs !!

I pushed an update for slo id validation, let me know if should be kept

kdelemme · 2024-11-01T13:09:49Z

@shahzad31 thanks for the review.
~~Does it fail in main if the id has spaces? I think it would since we attempt at creating transforms with a space.~~ Nevermind i just saw the screenshot error coming from the transform.

If we really want to add such validation, let's do it in the existing validate_slo function: https://github.com/kdelemme/kibana/blob/fix/revert-slo-id-transform-pivot/x-pack/plugins/observability_solution/slo/server/domain/services/validate_slo.ts#L77-L81

…_templates/slo_pipeline_template.ts Co-authored-by: Shahzad <[email protected]>

kdelemme · 2024-11-01T13:44:22Z

x-pack/packages/kbn-slo-schema/src/schema/slo.ts

+  const MIN_ID_LENGTH = 8;
+  const MAX_ID_LENGTH = 48;
+  const validLength = MIN_ID_LENGTH <= id.length && id.length <= MAX_ID_LENGTH;
+  return validLength && /^[a-zA-Z0-9-_]+$/.test(id);


Can you remove A-Z, that's not valid per transform validation.

elasticmachine · 2024-11-01T14:45:48Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 23b080c
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-198610-23b080c93b57

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #17 / AllCasesListGeneric Actions Row actions should delete a case
[job] [logs] FTR Configs #98 / serverless observability UI Dataset Quality Degraded fields flyout testing root cause for ignored fields current field limit issues should validate and show error callout when API call fails

Metrics [docs]

Async chunks

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

id	before	after	diff
`observability`	470.9KB	471.2KB	+294.0B
`slo`	856.0KB	856.3KB	+294.0B
`synthetics`	1.1MB	1.1MB	+294.0B
total			+882.0B

History

💚 Build #247727 succeeded 07ac83f
💔 Build #247718 failed 96ea48d
💚 Build #247686 succeeded b25b00a
💛 Build #247662 was flaky b6fa512
💔 Build #247619 failed e8fffde

cc @kdelemme

kibanamachine · 2024-11-01T15:36:59Z

Starting backport for target branches: 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11631975563

(cherry picked from commit 98db4d6)

kibanamachine · 2024-11-01T15:41:53Z

💚 All backports created successfully

Status	Branch	Result
✅	8.16
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…198699) # Backport This will backport the following commits from `main` to `8.16`: - [fix(slo): Override transform _id to ensure uniqness (#198610)](#198610)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Kevin Delemme <[email protected]>

…198700) # Backport This will backport the following commits from `main` to `8.x`: - [fix(slo): Override transform _id to ensure uniqness (#198610)](#198610)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Kevin Delemme <[email protected]>

kdelemme self-assigned this Oct 31, 2024

kdelemme added v8.16.0 bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team labels Oct 31, 2024

kdelemme marked this pull request as ready for review October 31, 2024 20:00

kdelemme requested review from a team as code owners October 31, 2024 20:00

kdelemme added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Oct 31, 2024

Overwrite the transform document _id from the rollup pipeline to ensu…

b6fa512

…re uniqness

kdelemme force-pushed the fix/revert-slo-id-transform-pivot branch from e8fffde to b6fa512 Compare October 31, 2024 22:53

botelastic bot added the ci:project-deploy-observability Create an Observability project label Oct 31, 2024

kdelemme changed the title ~~fix(slo): transform pivot needs to be unique across all transforms~~ fix(slo): Override transform _id to ensure uniqness Oct 31, 2024

Merge branch 'main' into fix/revert-slo-id-transform-pivot

b25b00a

shahzad31 requested changes Nov 1, 2024

View reviewed changes

shahzad31 reviewed Nov 1, 2024

View reviewed changes

shahzad31 removed the request for review from a team November 1, 2024 10:15

shahzad31 and others added 5 commits November 1, 2024 11:24

added slo id validation

b658d8f

[CI] Auto-commit changed files from 'node scripts/notice'

96ea48d

i18n

a921301

Merge branch 'fix/revert-slo-id-transform-pivot' of https://github.co…

bd3f93d

…m/kdelemme/kibana into fix/revert-slo-id-transform-pivot

[CI] Auto-commit changed files from 'node scripts/notice'

07ac83f

shahzad31 reviewed Nov 1, 2024

View reviewed changes

...k/plugins/observability_solution/slo/server/assets/ingest_templates/slo_pipeline_template.ts Outdated Show resolved Hide resolved

shahzad31 approved these changes Nov 1, 2024

View reviewed changes

kdelemme and others added 2 commits November 1, 2024 09:31

Update x-pack/plugins/observability_solution/slo/server/assets/ingest…

ff726ed

…_templates/slo_pipeline_template.ts Co-authored-by: Shahzad <[email protected]>

Update snapshots after change in _id

b1a7708

kdelemme commented Nov 1, 2024

View reviewed changes

Remove uppercase from allowed chars

23b080c

kdelemme enabled auto-merge (squash) November 1, 2024 13:49

kdelemme merged commit 98db4d6 into elastic:main Nov 1, 2024
27 checks passed

kibanamachine added the v9.0.0 label Nov 1, 2024

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 1, 2024

fix(slo): Override transform _id to ensure uniqness (elastic#198610)

c75b96c

(cherry picked from commit 98db4d6)

kibanamachine mentioned this pull request Nov 1, 2024

[8.16] fix(slo): Override transform _id to ensure uniqness (#198610) #198699

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 1, 2024

fix(slo): Override transform _id to ensure uniqness (elastic#198610)

9255fae

(cherry picked from commit 98db4d6)

kibanamachine mentioned this pull request Nov 1, 2024

[8.x] fix(slo): Override transform _id to ensure uniqness (#198610) #198700

Merged

kibanamachine added the v8.17.0 label Nov 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(slo): Override transform _id to ensure uniqness #198610

fix(slo): Override transform _id to ensure uniqness #198610

kdelemme commented Oct 31, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

shahzad31 left a comment

shahzad31 Nov 1, 2024

shahzad31 left a comment

kdelemme commented Nov 1, 2024 •

edited

Loading

kdelemme Nov 1, 2024

elasticmachine commented Nov 1, 2024 •

edited

Loading

kibanamachine commented Nov 1, 2024

kibanamachine commented Nov 1, 2024

fix(slo): Override transform _id to ensure uniqness #198610

fix(slo): Override transform _id to ensure uniqness #198610

Conversation

kdelemme commented Oct 31, 2024 • edited by kibanamachine Loading

🌮 Summary

Testing

elasticmachine commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

🤖 GitHub comments

shahzad31 left a comment

Choose a reason for hiding this comment

shahzad31 Nov 1, 2024

Choose a reason for hiding this comment

shahzad31 left a comment

Choose a reason for hiding this comment

kdelemme commented Nov 1, 2024 • edited Loading

kdelemme Nov 1, 2024

Choose a reason for hiding this comment

elasticmachine commented Nov 1, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

History

kibanamachine commented Nov 1, 2024

kibanamachine commented Nov 1, 2024

💚 All backports created successfully

Questions ?

kdelemme commented Oct 31, 2024 •

edited by kibanamachine

Loading

kdelemme commented Nov 1, 2024 •

edited

Loading

elasticmachine commented Nov 1, 2024 •

edited

Loading