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

Send grafana link with dashboard data #85

Merged
merged 1 commit into from Mar 7, 2022

Conversation

sjd78
Copy link
Member

@sjd78 sjd78 commented Feb 17, 2022

To be able to display a link to the Montioring Portal on the
dashboard on ovirt-engine-ui-extensions, the link needs to be
passed along with the rest of the dashboard info. If grafana is
not configured on the engine, null is returned.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482

@sjd78 sjd78 requested a review from rszwajko February 17, 2022 01:13
sjd78 added a commit to sjd78/ovirt-engine-ui-extensions that referenced this pull request Feb 17, 2022
If the URL for the engine's grafana instance is available from
the `database_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85
sjd78 added a commit to sjd78/ovirt-engine-ui-extensions that referenced this pull request Feb 17, 2022
If the URL for the engine's grafana instance is available from
the `dashboard_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

If no URL is provided from the `dashboard_data` fetch, no link is
rendered.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85
@sjd78
Copy link
Member Author

sjd78 commented Feb 17, 2022

This PR is a requirement for oVirt/ovirt-engine-ui-extensions#33.

sjd78 added a commit to sjd78/ovirt-engine-ui-extensions that referenced this pull request Feb 17, 2022
If the URL for the engine's grafana instance is available from
the `dashboard_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

If no URL is provided from the `dashboard_data` fetch, no link is
rendered.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85
@sgratch sgratch added the UX label Feb 17, 2022
@sjd78
Copy link
Member Author

sjd78 commented Feb 23, 2022

The URL lookup now only considers the URL config key and does not report any error if it is missing. Reporting error like this to the server log duplicates what is done in the BrandingManager.

Copy link
Member

@sgratch sgratch left a comment

Choose a reason for hiding this comment

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

The URL lookup now only considers the URL config key and does not report any error if it is missing. Reporting error like this to the server log duplicates what is done in the BrandingManager.

There is one problem with that approach of avoid validating the conf file:
If the 10-setup-grafana-access.conf file doesn't include the ENGINE_GRAFANA_FQDN parameter for some reason or the parameter exists but no value is set, then branding will raise an error and "Monitoring Portal" entry won't be added to welcome page, but it will be added to oVirt dashboard and point to https:///ovirt-engine-grafana/ which is always a wrong url.

Maybe worth just checking that those 2 parameters are set to something.

@sjd78
Copy link
Member Author

sjd78 commented Feb 28, 2022

The URL lookup now only considers the URL config key and does not report any error if it is missing. Reporting error like this to the server log duplicates what is done in the BrandingManager.

There is one problem with that approach of avoid validating the conf file: If the 10-setup-grafana-access.conf file doesn't include the ENGINE_GRAFANA_FQDN parameter for some reason or the parameter exists but no value is set, then branding will raise an error and "Monitoring Portal" entry won't be added to welcome page, but it will be added to oVirt dashboard and point to https:///ovirt-engine-grafana/ which is always a wrong url.

Maybe worth just checking that those 2 parameters are set to something.

The code now checks to make sure both of the grafana keys are not blank before returning the URL.

@sjd78 sjd78 requested a review from sgratch February 28, 2022 18:21
sjd78 added a commit to sjd78/ovirt-engine-ui-extensions that referenced this pull request Mar 1, 2022
If the URL for the engine's grafana instance is available from
the `dashboard_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

If no URL is provided from the `dashboard_data` fetch, no link is
rendered.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85
Copy link
Member

@sgratch sgratch left a comment

Choose a reason for hiding this comment

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

Please see my comment.

@sjd78 sjd78 requested a review from sgratch March 2, 2022 17:53
sjd78 added a commit to sjd78/ovirt-engine-ui-extensions that referenced this pull request Mar 2, 2022
If the URL for the engine's grafana instance is available from
the `dashboard_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

If no URL is provided from the `dashboard_data` fetch, no link is
rendered.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85
@mwperina mwperina requested a review from avlitman March 4, 2022 07:40
@mwperina
Copy link
Member

mwperina commented Mar 4, 2022

@avlitman We are passing engine FQDN into remote DWH installation, but do we have some where engine stored the information about remote DWH FQDN so we can provide a link to remote Grafana installation in webadmin dashboard?

@avlitman
Copy link
Member

avlitman commented Mar 4, 2022

@avlitman We are passing engine FQDN into remote DWH installation, but do we have some where engine stored the information about remote DWH FQDN so we can provide a link to remote Grafana installation in webadmin dashboard?

Hi, yes we are passing engine FQDN into DWH in all cases (remote or local).
I can't find any column in the engine db that points to remote/local DWH FQDN,
as mention above when configuring Grafana (now it's by default) the engine have a file: /etc/ovirt-engine/engine.conf.d/10-setup-grafana-access.conf that includes 2 parameters ENGINE_GRAFANA_FQDN, and ENGINE_GRAFANA_BASE_URL
If dwh is local: ENGINE_GRAFANA_FQDN=engine_fqdn
If dwh is remote: ENGINE_GRAFANA_FQDN=dwh_fqdn

To sum it up I think the best way is to use those 2 parameters, and as Sharon said check that those 2 parameters are set to something.

@avlitman
Copy link
Member

avlitman commented Mar 4, 2022

I'm wondering if we can set the monitoring portal link to be on all pages in the admin portal,
like the Toolbar:
Screenshot from 2022-03-04 12-28-27
I think it will make great impact for the customers to be able to navigate to the monitoring portal easily from any page in the admin portal.

@sgratch @sjd78 Do you think it's a possibility ?

@sjd78
Copy link
Member Author

sjd78 commented Mar 4, 2022

I'm wondering if we can set the monitoring portal link to be on all pages in the admin portal, like the Toolbar: Screenshot from 2022-03-04 12-28-27 I think it will make great impact for the customers to be able to navigate to the monitoring portal easily from any page in the admin portal.

@sgratch @sjd78 Do you think it's a possibility ?

It can be done, but that isn't what was asked for in the BZ. This change specifically enables the link to be displayed on the dashboard (which is over in ovirt-engine-ui-extensions). I'm willing to add that in another PR, but I'm not sure I'll have time for it before feature freeze.

To be able to display a link to the Montioring Portal on the
dashboard on `ovirt-engine-ui-extensions`, the link needs to be
passed along with the rest of the dashboard info.  If grafana is
not configured on the engine, `null` is returned.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
@avlitman
Copy link
Member

avlitman commented Mar 7, 2022

I'm wondering if we can set the monitoring portal link to be on all pages in the admin portal, like the Toolbar: Screenshot from 2022-03-04 12-28-27 I think it will make great impact for the customers to be able to navigate to the monitoring portal easily from any page in the admin portal.
@sgratch @sjd78 Do you think it's a possibility ?

It can be done, but that isn't what was asked for in the BZ. This change specifically enables the link to be displayed on the dashboard (which is over in ovirt-engine-ui-extensions). I'm willing to add that in another PR, but I'm not sure I'll have time for it before feature freeze.

@sjd78 +1 Thank you, much appreciated! so should I open a second bug for that?

@sgratch
Copy link
Member

sgratch commented Mar 7, 2022

@sjd78 +1 Thank you, much appreciated! so should I open a second bug for that?

@avlitman please do if you think it's valuable, but most sure we won't have time to handle that for 4.5 since there are more urgent stuff planned.

Copy link
Member

@sgratch sgratch left a comment

Choose a reason for hiding this comment

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

LGTM

@sgratch sgratch merged commit 43cc067 into oVirt:master Mar 7, 2022
sgratch pushed a commit to oVirt/ovirt-engine-ui-extensions that referenced this pull request Mar 7, 2022
* Add link to Monitoring Portal on Dashboard

If the URL for the engine's grafana instance is available from
the `dashboard_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

If no URL is provided from the `dashboard_data` fetch, no link is
rendered.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85

* remove unused styles

* adjust monitoring link styles to PF4 CSS vars
@sjd78 sjd78 deleted the dashboard_grafana branch March 17, 2022 14:17
mkemel pushed a commit to mkemel/ovirt-engine-ui-extensions that referenced this pull request Mar 28, 2022
* Add link to Monitoring Portal on Dashboard

If the URL for the engine's grafana instance is available from
the `dashboard_data` fetch, render the link in the upper right
hand side of the dashboard.  This allows the user to click directly
to the monitoring portal without jumping back to the welcome page
first.

If no URL is provided from the `dashboard_data` fetch, no link is
rendered.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1991482
Require ovirt-engine PR: oVirt/ovirt-engine#85

* remove unused styles

* adjust monitoring link styles to PF4 CSS vars
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants