[SLO] Add dashboard references to SLO saved object by cesco-f · Pull Request #232583 · elastic/kibana

cesco-f · 2025-08-22T08:18:20Z

First part for closing issue #228509.

This update handles only the server-side implementation. The solution follows a similar approach to the one used for attaching dashboards to rules.

Screen.Recording.2025-08-22.at.10.13.50.mov

…(server part)

elasticmachine · 2025-08-22T09:29:15Z

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

…te --fix'

… src/core/server/integration_tests/ci_checks'

kdelemme

Good job, overall this looks good to me.

I have some basic questions for my own understanding of the need for refId (cf comments)

Another question I have is about the usage: Are we going to display the dashboard name when linking to the dashboard, and if so, are we planning to fetch the name ad-hoc or fetch it when we read the references? Or is there a dashboard "locator" that takes care of rendering the name based on the provided id?

Right now we are storing the dashboard id as part of the SLO Definition, do we think it can be useful to store it as part of the SLO Summary as well? I don't know yet, and this would require a bit of changes so let's maybe delay this decision to later. Just wanted to write my thought down.

One thing I would like to test is the upgrade path (should be fine since everything is optional): creating an SLO on main, and then running this branch and trying to read/edit this SLO. Could you confirm everything is fine on this side? Thanks!

kdelemme · 2025-08-24T11:15:03Z

x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts

+  const references: SavedObjectReference[] = [];
+  if (slo.artifacts?.dashboards?.length) {
+    slo.artifacts.dashboards.forEach(({ id }, index) => {
+      const refId = `dashboard-${index}`;


Are we using index because we want to allow the same dashboard (id) to be referenced multiple time?

I think it doesn't really matter what we store in the refId as long as it's unique and it's the same as the name property we store in the reference.

Exactly point is that refId should be unique and same as the name property in the reference.

kdelemme · 2025-08-24T11:23:15Z

x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts

+  return {
+    storedSLO: storedSloDefinitionSchema.encode({
+      ...slo,
+      artifacts: { dashboards: dashboardsRef },


Have we considered storing the dashboard id directly instead of the refId? I understand we introduce this refId to handle the case where the same dashboard is added many times but should it be a valid use case? Is there other reason for this custom refId?

Also, how will we search across all SLOs for a given dashboard id (use case: find all SLO related to this dashboard)? Is there an API on the saved object service that we can leverage to search all references of a given type (slo) at once? Or do we need to iterate over each SLO? If we store the dashboard id directly, we should be able to search them directly.

I tried to replicate the approach we’re using to store references for rules, as described in the Kibana developer documentation.

According to the documentation:

Note how dashboard.panels[0].visualization stores the name property of the reference (not the id directly) to be able to uniquely identify this reference. This guarantees that the id the reference points to always remains up to date. If a visualization id was directly stored in dashboard.panels[0].visualization there is a risk that this id gets updated without updating the reference in the references array.

To find SLOs related to a given dashboard, I think we can leverage this property of the find method in the saved objects client:

kdelemme · 2025-08-24T11:26:46Z

x-pack/solutions/observability/test/api_integration_deployment_agnostic/apis/slo/update_slo.ts

      await transformHelper.assertExist(getSLOSummaryTransformId(sloId, 2));
    });
+
+    it('updates dashboard artifacts and returns them', async () => {


🍰 nit: can you create two sub describe (one for with revision bump and one without). This test belongs in the without revision bump. I want to make sure we document (by test) the behaviour of updating a dashboard artifacts:

only save the definition with the dashboard

no revision bump (maybe assert for this)?

Is this what you had in mind?
d757b3c

kdelemme · 2025-08-24T11:29:01Z

x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts

-    return response.saved_objects
-      .map((slo) => this.toSLO(slo))
-      .filter((slo) => slo !== undefined) as SLODefinition[];
+    return response.saved_objects.map((so) => this.toSLO(so)).filter(this.isSLO);


👍🏻 thanks for the type guard filter

kdelemme · 2025-08-24T11:38:21Z

x-pack/solutions/observability/plugins/slo/server/saved_objects/slo.ts

+        properties: {
+          dashboards: {
+            properties: {
+              refId: { type: 'keyword' },


If we were using the dashboard id, we could find every SLO with a specific dashboard id. I guess that's possible with the references also, can you confirm?

We can leverage the hasReference property of the find method in the saved objects client.

cesco-f · 2025-08-25T07:14:24Z

Another question I have is about the usage: Are we going to display the dashboard name when linking to the dashboard, and if so, are we planning to fetch the name ad-hoc or fetch it when we read the references? Or is there a dashboard "locator" that takes care of rendering the name based on the provided id?

We can discuss this further in the PR for the frontend part, but my idea was to reuse the component we already use to link dashboards to rules. Then, we could create a custom hook that leverages the content management client to fetch the dashboard name based on its ID. I’m not completely sure whether this logic should live in the backend, but I’d suggest handling it in the next PR, when we’ll need to display the dashboard name in the SLO details view.

Right now we are storing the dashboard id as part of the SLO Definition, do we think it can be useful to store it as part of the SLO Summary as well? I don't know yet, and this would require a bit of changes so let's maybe delay this decision to later. Just wanted to write my thought down.

I’m not sure what the SLO Summary is or how we’re using it in Kibana. Is there any documentation I can read to learn more about it?

One thing I would like to test is the upgrade path (should be fine since everything is optional): creating an SLO on main, and then running this branch and trying to read/edit this SLO. Could you confirm everything is fine on this side? Thanks!

I tested this path and I didn't find any issues, all good 👍

x-pack/solutions/observability/plugins/slo/server/saved_objects/slo.ts

afharo

LGTM. To give us more freedom, I'd remove the mappings addition for now.

x-pack/solutions/observability/plugins/slo/server/saved_objects/slo.ts

This reverts commit d4f733a.

…ngs_update --fix'" This reverts commit d850881.

…ation -u src/core/server/integration_tests/ci_checks'" This reverts commit 3dc1b49.

elasticmachine · 2025-08-26T18:43:49Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 6d5ae96

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #109 / OneChat Endpoints Builtin Tools internal API POST /internal/chat/tools/_bulk_delete should handle mixed bulk delete with builtin and custom tools
[job] [logs] FTR Configs #109 / OneChat Endpoints Builtin Tools internal API POST /internal/chat/tools/_bulk_delete should handle mixed bulk delete with builtin and custom tools

History

💔 Build #332792 failed ce90e86
💛 Build #331766 was flaky 3dc1b49
💔 Build #331725 failed 9842d78

mgiota

Excellent job! Great implementation replicating the approach we’re using to store references for rules!

This PR is a follow-up of #232583 and it closes #228509. Dashboards can be linked to SLOs. https://github.com/user-attachments/assets/a9593926-ad01-4c40-be1b-27b44eef81ae --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

This PR is a follow-up of elastic#232583 and it closes elastic#228509. Dashboards can be linked to SLOs. https://github.com/user-attachments/assets/a9593926-ad01-4c40-be1b-27b44eef81ae --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

First part for closing issue elastic#228509. This update handles only the server-side implementation. The solution follows a similar approach to the one used for attaching dashboards to rules. https://github.com/user-attachments/assets/329af612-f84b-426e-99e0-11f82d565079 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

This PR is a follow-up of elastic#232583 and it closes elastic#228509. Dashboards can be linked to SLOs. https://github.com/user-attachments/assets/a9593926-ad01-4c40-be1b-27b44eef81ae --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

cesco-f added release_note:enhancement backport:skip This PR does not require backporting labels Aug 22, 2025

github-actions bot added the author:obs-ux-management PRs authored by the obs ux management team label Aug 22, 2025

feat(slo dashboards): dashboards can be stored as references of slos …

c83a8e7

…(server part)

cesco-f force-pushed the slo-dashboards-server branch from d8681c2 to c83a8e7 Compare August 22, 2025 08:31

cesco-f added 2 commits August 22, 2025 10:54

docs(update): updated docs with artifacts

d1da6b2

test(api integration): added api integration tests

0b88ec1

cesco-f marked this pull request as ready for review August 22, 2025 09:16

cesco-f requested a review from a team as a code owner August 22, 2025 09:16

botelastic bot added the Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. label Aug 22, 2025

fix(deps): removed dependency with deeplink-analytics

9842d78

cesco-f force-pushed the slo-dashboards-server branch from 340e840 to 9842d78 Compare August 22, 2025 09:33

cesco-f and others added 2 commits August 22, 2025 12:33

fix(mapping addition): added mapping addition

d4f733a

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

d850881

…te --fix'

elastic-vault-github-plugin-prod bot requested a review from a team as a code owner August 22, 2025 10:50

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

3dc1b49

… src/core/server/integration_tests/ci_checks'

kdelemme reviewed Aug 24, 2025

View reviewed changes

Merge branch 'main' into slo-dashboards-server

675bf3e

test(update slo): check no revision bump when updating dashboards

d757b3c

rudolf requested changes Aug 26, 2025

View reviewed changes

x-pack/solutions/observability/plugins/slo/server/saved_objects/slo.ts Outdated Show resolved Hide resolved

afharo approved these changes Aug 26, 2025

View reviewed changes

x-pack/solutions/observability/plugins/slo/server/saved_objects/slo.ts Outdated Show resolved Hide resolved

cesco-f and others added 5 commits August 26, 2025 17:49

Revert "fix(mapping addition): added mapping addition"

cffb0a6

This reverts commit d4f733a.

Revert "[CI] Auto-commit changed files from 'node scripts/check_mappi…

8548dfb

…ngs_update --fix'" This reverts commit d850881.

Revert "[CI] Auto-commit changed files from 'node scripts/jest_integr…

332b891

…ation -u src/core/server/integration_tests/ci_checks'" This reverts commit 3dc1b49.

fix(mapping): removed mapping

ce90e86

Merge branch 'main' into slo-dashboards-server

6d5ae96

Merge branch 'main' into slo-dashboards-server

39251cc

cesco-f requested a review from rudolf August 27, 2025 05:15

rudolf approved these changes Aug 27, 2025

View reviewed changes

cesco-f requested a review from a team August 27, 2025 07:30

mgiota self-requested a review August 28, 2025 07:34

mgiota approved these changes Aug 28, 2025

View reviewed changes

cesco-f merged commit a0f70df into elastic:main Aug 28, 2025
12 checks passed

cesco-f deleted the slo-dashboards-server branch August 28, 2025 08:18

kibanamachine added the v9.2.0 label Aug 28, 2025

cesco-f mentioned this pull request Aug 28, 2025

[SLO] Link dashboards to SLO #233265

Merged

Conversation

cesco-f commented Aug 22, 2025

Uh oh!

elasticmachine commented Aug 22, 2025

Uh oh!

kdelemme left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cesco-f Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cesco-f commented Aug 25, 2025

Uh oh!

Uh oh!

afharo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Aug 26, 2025

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

History

Uh oh!

mgiota left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cesco-f Aug 25, 2025 •

edited

Loading