[SLO] Fix SLO plugin context #183966

mgiota · 2024-05-21T21:22:20Z

Fixes #183967

This PR reverts the changes introduced in this PR regarding SLO Plugin context.

Before

Notice how the embeddable breaks when user clicks on the group accordion in order to expand it.

broken.mov

After

This is the fixed version, where clicking on the group shows the respective SLOs

fixed.mov

apmmachine · 2024-05-21T21:22:35Z

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

elasticmachine · 2024-05-21T21:43:58Z

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

shahzad31

Can we please add a functional e2e test for this?

shahzad31 · 2024-05-22T08:05:08Z

x-pack/plugins/observability_solution/slo/public/hooks/use_plugin_context.tsx

-  const context = useContext(PluginContext);
-  if (!context) {
-    throw new Error('Plugin context value is missing!');
-  }


why is this removed?

I pretty much reverted the changes that broke it. As discussed, I am actually refactoring it and I will bring this back

shahzad31 · 2024-05-22T08:08:03Z

x-pack/plugins/observability_solution/slo/public/context/plugin_context.tsx

-
-export const PluginContext = createContext<PluginContextValue | null>(null);
+export const PluginContext = createContext(
+  {} as PluginContextValue | OverviewEmbeddableContextValue


shouldn't this always be PluginContextValue ?

On a dashboard context it should be OverviewEmbeddableContextValue otherwise PluginContextValue. Let me push the refactored code. I disabled an eslint rule though to make it work, so I'd like your thoughts on this.

mgiota · 2024-05-22T08:41:12Z

x-pack/plugins/observability_solution/slo/public/hooks/use_get_filtered_rule_types.ts

+  const executionContextName = executionContext.get().name;
+
+  const { observabilityRuleTypeRegistry } =
+    // eslint-disable-next-line react-hooks/rules-of-hooks


@shahzad31 This solution works, but honestly I don't like I had to disable this eslint rule. Instead I am thinking to do the executionContext check in this file x-pack/plugins/observability_solution/slo/public/pages/slos/components/common/burn_rate_rule_flyout.tsx and pass the context as a param in the useGetFilteredRuleTypes function. What do you think?

I will also need to change the way useGetFilteredRuleTypes is called in one more place as well x-pack/plugins/observability_solution/slo/public/pages/slos/components/compact_view/slo_list_compact_view.tsx.

mgiota · 2024-05-22T09:23:32Z

@shahzad31 I refactored as per our discussion in this commit, where I created a new SloOverviewEmbeddableContext. Everything works, but I get a ts error in all slo pages that use ObservabilityPageTemplate, since I made it optional. Do you know how I can fix? Above ts error is the reason I didn't use this approach in the first place.

...gins/observability_solution/slo/public/embeddable/slo/common/overview_embeddable_context.tsx

mgiota · 2024-05-22T10:31:18Z

x-pack/plugins/observability_solution/slo/public/context/plugin_context.tsx

@@ -16,14 +16,8 @@ export interface PluginContextValue {
  isServerless?: boolean;
  appMountParameters?: AppMountParameters;
  observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry;
-  ObservabilityPageTemplate: React.ComponentType<LazyObservabilityPageTemplateProps>;
+  ObservabilityPageTemplate?: React.ComponentType<LazyObservabilityPageTemplateProps>;


@shahzad31 I don't know how to proceed with the ts error I got regarding ObservabilityPageTemplate. Any ideas?

shahzad31

SLO Oveview ebeddable group-by is now working !!

kibana-ci · 2024-05-22T11:38:14Z

💚 Build Succeeded

Buildkite Build
Commit: e48c938
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-183966-e48c938ea0f1
Observability Deployment

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`slo`	845	846	+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`	870.2KB	870.4KB	+160.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	-	5410	+5410
`total size`	-	8.8MB	+8.8MB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`slo`	22.9KB	22.9KB	-66.0B

History

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

cc @mgiota

fix slo plugin context

fee423b

mgiota marked this pull request as ready for review May 21, 2024 21:24

mgiota requested a review from a team as a code owner May 21, 2024 21:24

mgiota changed the title ~~fix slo plugin context~~ [SLO] fix slo plugin context May 21, 2024

mgiota changed the title ~~[SLO] fix slo plugin context~~ [SLO] Fix SLO plugin context May 21, 2024

mgiota self-assigned this May 21, 2024

mgiota added the Team:obs-ux-management Observability Management User Experience Team label May 21, 2024

mgiota added v8.15.0 Feature:SLO bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes labels May 21, 2024

shahzad31 reviewed May 22, 2024

View reviewed changes

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

shahzad31 reviewed May 22, 2024

View reviewed changes

mgiota force-pushed the fix_slo_plugin_context branch from 3e11ab2 to a643cb3 Compare May 22, 2024 08:38

refactor based on executionContext

105d664

mgiota force-pushed the fix_slo_plugin_context branch from a643cb3 to 105d664 Compare May 22, 2024 08:39

mgiota commented May 22, 2024

View reviewed changes

mgiota requested a review from shahzad31 May 22, 2024 09:24

shahzad31 reviewed May 22, 2024

View reviewed changes

...gins/observability_solution/slo/public/embeddable/slo/common/overview_embeddable_context.tsx Outdated Show resolved Hide resolved

mgiota requested a review from shahzad31 May 22, 2024 09:32

mgiota added 4 commits May 22, 2024 12:26

create an Embeddable Overview context

20ed8d2

PR feedback

3f65091

remove previous OverviewEmbeddableContextValue leftovers

d142de6

remove unused stuff

c67063c

mgiota force-pushed the fix_slo_plugin_context branch from 685cdeb to c67063c Compare May 22, 2024 10:27

mgiota commented May 22, 2024

View reviewed changes

mgiota mentioned this pull request May 22, 2024

[SLO] Functional tests for Overview Embeddable Follow up #184001

Open

fix type

e48c938

shahzad31 approved these changes May 22, 2024

View reviewed changes

mgiota enabled auto-merge (squash) May 22, 2024 10:56

mgiota merged commit e6614fa into elastic:main May 22, 2024
20 checks passed

kibanamachine added the backport:skip This commit does not require backporting label May 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SLO] Fix SLO plugin context #183966

[SLO] Fix SLO plugin context #183966

mgiota commented May 21, 2024 •

edited

apmmachine commented May 21, 2024

elasticmachine commented May 21, 2024

shahzad31 left a comment

shahzad31 May 22, 2024

mgiota May 22, 2024

shahzad31 May 22, 2024

mgiota May 22, 2024

mgiota May 22, 2024 •

edited

mgiota May 22, 2024

mgiota commented May 22, 2024 •

edited

mgiota May 22, 2024

shahzad31 left a comment

kibana-ci commented May 22, 2024 •

edited

[SLO] Fix SLO plugin context #183966

[SLO] Fix SLO plugin context #183966

Conversation

mgiota commented May 21, 2024 • edited

Before

After

apmmachine commented May 21, 2024

🤖 GitHub comments

elasticmachine commented May 21, 2024

shahzad31 left a comment

Choose a reason for hiding this comment

shahzad31 May 22, 2024

Choose a reason for hiding this comment

mgiota May 22, 2024

Choose a reason for hiding this comment

shahzad31 May 22, 2024

Choose a reason for hiding this comment

mgiota May 22, 2024

Choose a reason for hiding this comment

mgiota May 22, 2024 • edited

Choose a reason for hiding this comment

mgiota May 22, 2024

Choose a reason for hiding this comment

mgiota commented May 22, 2024 • edited

mgiota May 22, 2024

Choose a reason for hiding this comment

shahzad31 left a comment

Choose a reason for hiding this comment

kibana-ci commented May 22, 2024 • edited

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Canvas Sharable Runtime

Page load bundle

History

mgiota commented May 21, 2024 •

edited

mgiota May 22, 2024 •

edited

mgiota commented May 22, 2024 •

edited

kibana-ci commented May 22, 2024 •

edited