Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Synthetics] Status overview embeddable #188807

Merged
merged 14 commits into from
Jul 22, 2024

Conversation

shahzad31
Copy link
Contributor

Summary

Added status overview embeddable !!

status.panel.mov

@shahzad31 shahzad31 requested review from a team as code owners July 22, 2024 06:13
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Jul 22, 2024
@elasticmachine
Copy link
Contributor

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

@obltmachine
Copy link

🤖 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 shahzad31 added the release_note:skip Skip the PR/issue when compiling release notes label Jul 22, 2024
* 2.0.
*/

export const STATUS_OVERVIEW_EMBEDDABLE = 'STATUS_OVERVIEW_EMBEDDABLE';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shahzad31 This is the name with which the embeddable gets registered to the embeddable registry. I recommend synthetics is include in the name, something like SYNTHETICS_OVERVIEW_EMBEDDABLE

@shahzad31 shahzad31 requested a review from mgiota July 22, 2024 08:59
fetchSubscription.unsubscribe();
};
}, []);
return <StatusOverviewComponent reload$={reload$} />;
Copy link
Contributor

@mgiota mgiota Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shahzad31 Every embeddable needs to have a data-shared-item="" in order to not report timeout errors in the Kibana screenshot tool. In the Dashboard app you have the option to export the Dashboard as PDF.

Screenshot 2024-07-22 at 10 55 18

Right now it reports a timeout error because it is missing this attribute. Try to add following:

 <div
              data-shared-item="" // TODO: Remove data-shared-item and data-rendering-count as part of https://github.com/elastic/kibana/issues/179376
            >
              <StatusOverviewComponent />
            </div>

This manual adding of the data-shared-item attribute in each embeddable is temporary and should be holistically addressed later on in this issue.

You can find more information about this here

@mgiota
Copy link
Contributor

mgiota commented Jul 22, 2024

@shahzad31 In general it looks good! I was able to add a new Synthetics Status Overview embeddable. I left a few comments regarding removing the edit option from your embeddable and adding the data-shared-item attribute. I have a few more comments:

  • Refresh button: I would expect the embeddable to react to this button. As a quick test, I added a new monitor in the Synthetics app and then I clicked the Refresh button in the Dashboard app. I would expect the embeddable to react to this change, without me having to reload whole page
  • Initial dimensions We have a way to control the initial dimensions of the panel. I suggest we make the panel smaller by default. Let me search how I handled it in the overview embeddable and I will share.
Screenshot 2024-07-22 at 11 16 15

@mgiota
Copy link
Contributor

mgiota commented Jul 22, 2024

@ThomThomson This is a new Observebility embeddable that @shahzad31 is adding!

@mgiota
Copy link
Contributor

mgiota commented Jul 22, 2024

@shahzad31 Can you make the new Synthetics panel clickable, so when users click on the panel, they are redirected to the Synthetics app?

We should also think how we handle empty state. When user has no monitors set up, we should display message and a link to the Synthetics app.

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kibana.jsonc lgtm

Copy link
Contributor

@mgiota mgiota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve this PR. As discussed we will handle the clicking/filtering/empty state in a follow up PR!

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 22, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
synthetics 913 973 +60

Async chunks

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

id before after diff
synthetics 854.5KB 847.8KB -6.8KB

Page load bundle

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

id before after diff
synthetics 19.5KB 40.0KB +20.5KB

History

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

@shahzad31 shahzad31 merged commit eb71438 into elastic:main Jul 22, 2024
24 checks passed
@shahzad31 shahzad31 deleted the status-overview-panel branch July 22, 2024 12:21
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Jul 22, 2024
@ThomThomson
Copy link
Contributor

ThomThomson commented Jul 22, 2024

@mgiota thanks for the tag!

One thing I'm noticing straight away here is that the status doesn't stretch to fill the vertical panel space on the Dashboard. This makes me worry that if the panel is too small content will be cut off. @andreadelrio would you mind helping out with a design audit of this?

@nreese we should prioritize that functional test that fails when a new item is added to the add panel menu so we can get pinged for review when folks add a new embeddable type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants