fix(slo): Overview Embeddable drilldown actions by kdelemme · Pull Request #201870 · elastic/kibana

kdelemme · 2024-11-26T21:11:33Z

Resolves #196740

🌸 Summary

This PR brings back the drilldown actions on the SLO Overview embeddable. I had to change the visibility of the embeddable-enhanced plugin to be able to use it from the slo plugin since these plugins don't share the same group.

In a following PR, I'll address the other embeddable components.

🧬 Testing

Create some SLO Overview embeddable (with single/grouped by SLOs)
Assert the drilldown action is available

elasticmachine · 2024-11-26T21:12:56Z

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

mgiota · 2024-11-26T22:17:40Z

@kdelemme I did a quick code review and changes LGTM. I checked the visualize embeddable and the use of dynamicActionsApi that you use in your PR. I will test it out locally as well.

@Heenawter In your initial feedback you mentioned possible use of HasSupportedTriggers. I see Kevin added it as part of his PR. The visualize_embeddable doesn't use HasSupportedTriggers, but the image embeddable does. Can you explain what it does?

Heenawter · 2024-11-26T22:55:17Z

@mgiota I'm seeing that the visualize embeddable does use HasSupportedTriggers:

kibana/src/plugins/visualizations/public/embeddable/types.ts

Line 99 in 4bc9a59

HasSupportedTriggers &

kibana/src/plugins/visualizations/public/embeddable/visualize_embeddable.tsx

Lines 183 to 187 in 4bc9a59

    
           supportedTriggers: () => [ 
        
             ACTION_CONVERT_TO_LENS, 
        
             APPLY_FILTER_TRIGGER, 
        
             SELECT_RANGE_TRIGGER, 
        
           ],

HasSupportedTriggers is used for when you want to be able to trigger a drilldown via some other action - the image embeddable has a custom supported trigger so that clicking the image will cause the drilldown to happen, for example. Since you are returning an empty array currently, the only trigger that your drilldown can be attached to is the context menu.

Right now we are requiring supportedTriggers to be defined in order for both createDrilldownAction and editDrilldownAction to be compatible - hence, why you had to define supportedTriggers and return an empty array for those to be compatible. This is actually overkill IMO, because this is only necessary if you need other other triggers for drilldowns (other than the CONTEXT_MENU_TRIGGER) - we should probably make this an optional dependency for compatibility. cc @nreese

mgiota · 2024-11-27T08:04:06Z

I'm seeing that the visualize embeddable does use HasSupportedTriggers

@Heenawter yep you are right! I initially did a quick scan for HasSupportedTriggers in the visualize_embeddable and thus my confusion. It is defined in the VisualizeApi

Thanks for the super detailed explanation! Indeed making this an optional dependency would make it simpler to use.

mgiota

@kdelemme I tested it locally and I verify it works fine!

Before

After

github-actions · 2024-11-27T08:32:46Z

🤖 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!)

mgiota · 2024-11-27T08:35:53Z

@elasticmachine merge upstream

gsoldevila · 2024-11-29T10:03:05Z

  ],
  "group": "platform",
-  "visibility": "private",
+  "visibility": "shared",


@elastic/kibana-presentation this PR opens up embeddableEnhanced so that it is used directly from solutions (in this case from o11y SLOs).

Up until now, it was only used from "platform" plugins (e.g. visualizations, dashboard_enhanced, lens, maps).

Are you okay with that transition to "shared"?

Yes this makes sense IMO. Solutions are expected to register embeddable types, and some of those will use drilldowns.

Heenawter

Switching embeddable_enhanced to "shared" LGTM 👍

kdelemme · 2024-12-02T18:17:20Z

@gsoldevila Can you approve please?

elasticmachine · 2024-12-02T19:51:06Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 51c1d95
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-201870-51c1d9548354

Failed CI Steps

FTR Configs #89

Test Failures

[job] [logs] FTR Configs #89 / Dataset Quality Dataset quality handles user privileges User can read logs-* User cannot monitor any data stream Estimated data are not available due to underprivileged user

Metrics [docs]

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
`slo`	47	48	+1

Async chunks

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

id	before	after	diff
`slo`	842.6KB	843.2KB	+603.0B

Unknown metric groups

API count

id	before	after	diff
`slo`	47	48	+1

History

💛 Build #256118 was flaky a1f7a40
💛 Build #256026 was flaky eeabe35
💚 Build #255360 succeeded a35605d
💔 Build #255117 failed 725a634
💔 Build #255031 failed 655dff4

gsoldevila

LGTM!

kibanamachine · 2024-12-03T13:44:42Z

Starting backport for target branches: 8.x

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

kibanamachine · 2024-12-03T13:49:43Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts You might need to backport the following PRs to 8.x: - fix(slo): Access Cases from SLO details page (#201834)

Manual backport

To create the backport manually run:

node scripts/backport --pr 201870

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2024-12-04T13:49:44Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201870 locally

kdelemme · 2024-12-04T13:53:31Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

(cherry picked from commit 5b391b0)

kibanamachine · 2024-12-05T13:50:00Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

kibanamachine · 2024-12-06T14:49:03Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

) # Backport This will backport the following commits from `main` to `8.x`: - [fix(slo): Overview Embeddable drilldown actions (#201870)](#201870)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Shahzad <shahzad31comp@gmail.com>

kdelemme added 4 commits November 26, 2024 15:48

Change visibility to shared

81a1f93

Add embeddable-enhanced plugin

b261574

Implements interfaces for enabling drilldown action

15cb895

Inline variable

b1ff551

kdelemme added release_note:skip Skip the PR/issue when compiling release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. v8.18.0 backport:prev-minor labels Nov 26, 2024

kdelemme marked this pull request as ready for review November 26, 2024 21:12

kdelemme requested review from a team as code owners November 26, 2024 21:12

mgiota self-requested a review November 26, 2024 22:17

mgiota approved these changes Nov 27, 2024

View reviewed changes

botelastic Bot added the ci:project-deploy-observability Create an Observability project label Nov 27, 2024

elasticmachine and others added 4 commits November 27, 2024 08:35

Merge branch 'main' into fix/embeddables-drilldown

089cf28

Merge branch 'main' into fix/embeddables-drilldown

2e3039e

Merge branch 'main' into fix/embeddables-drilldown

655dff4

Merge branch 'main' into fix/embeddables-drilldown

725a634

Heenawter mentioned this pull request Nov 27, 2024

[Embeddable Rebuild] Make HasSupportedTriggers optional for drilldown actions #202054

Closed

Merge branch 'main' into fix/embeddables-drilldown

a35605d

gsoldevila reviewed Nov 29, 2024

View reviewed changes

Heenawter approved these changes Nov 29, 2024

View reviewed changes

kdelemme added 2 commits December 2, 2024 09:22

Merge branch 'main' into fix/embeddables-drilldown

eeabe35

Merge branch 'main' into fix/embeddables-drilldown

a1f7a40

Merge branch 'main' into fix/embeddables-drilldown

51c1d95

gsoldevila approved these changes Dec 3, 2024

View reviewed changes

kdelemme merged commit 5b391b0 into elastic:main Dec 3, 2024

kdelemme deleted the fix/embeddables-drilldown branch December 3, 2024 13:44

kibanamachine added the v9.0.0 label Dec 3, 2024

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 4, 2024

kdelemme mentioned this pull request Dec 4, 2024

[8.x] fix(slo): Overview Embeddable drilldown actions (#201870) #202925

Merged

kdelemme added a commit to kdelemme/kibana that referenced this pull request Dec 4, 2024

fix(slo): Overview Embeddable drilldown actions (elastic#201870)

cff7c80

(cherry picked from commit 5b391b0)

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 6, 2024

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024

fix(slo): Overview Embeddable drilldown actions (elastic#201870)

efcac6d

Conversation

kdelemme commented Nov 26, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌸 Summary

🧬 Testing

Uh oh!

elasticmachine commented Nov 26, 2024

Uh oh!

mgiota commented Nov 26, 2024

Uh oh!

Heenawter commented Nov 26, 2024

Uh oh!

mgiota commented Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgiota left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 27, 2024

🤖 GitHub comments

Uh oh!

mgiota commented Nov 27, 2024

Uh oh!

gsoldevila Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

ThomThomson Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

Heenawter left a comment

Choose a reason for hiding this comment

Uh oh!

kdelemme commented Dec 2, 2024

Uh oh!

elasticmachine commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Async chunks

API count

History

Uh oh!

gsoldevila left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 3, 2024

Uh oh!

kibanamachine commented Dec 3, 2024

💔 All backports failed

Manual backport

Questions ?

Uh oh!

kibanamachine commented Dec 4, 2024

Uh oh!

kdelemme commented Dec 4, 2024

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Dec 5, 2024

Uh oh!

kibanamachine commented Dec 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kdelemme commented Nov 26, 2024 •

edited by kibanamachine

Loading

mgiota commented Nov 27, 2024 •

edited

Loading

elasticmachine commented Dec 2, 2024 •

edited

Loading