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

mgr/dashboard: Prometheus integration #25309

Merged
merged 4 commits into from Feb 4, 2019

Conversation

@Devp00l
Copy link
Contributor

Devp00l commented Nov 28, 2018

The backend is now capable of receiving alert notifications from
the Prometheus alertmanager and it can get all alerts with all kinds of
parameters from the API of the same.

Now Prometheus alerts can be found in "Cluster > Alerts". Incoming
notifications can be seen as usual in the notifications popover.

To clarify:
Prometheus alerts are received from the alertmanager API.
Prometheus alert notification are send from the alertmanager to the
backend receiver. An alert notification can have multiple alerts, but
these alerts differ from the prometheus alerts.

To clarify that, I've added some models and services.

If one of the methods to get alerts contains changes the user will be
notified.

In addition to that the following new things were implemented:

  • It's possible to style values inside the key value table based on the
    values.
  • Notifications and alerts show an application icon now, that gives a hint
    about their origin.

Fixes: https://tracker.ceph.com/issues/37950
Fixes: https://tracker.ceph.com/issues/37951
Fixes: https://tracker.ceph.com/issues/36721
Signed-off-by: Stephan Müller smueller@suse.com

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Some impressions:
firing-toasty
listing-active-details
listing-unprossed
notification-connection-failed
suppressed-details
toasty-connection-faild

@Devp00l Devp00l added the dashboard label Nov 28, 2018

@Devp00l Devp00l requested review from LenzGr , tspmelo , votdev , p-na and ricardoasmarques Nov 28, 2018

Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated

@s0nea s0nea added the feature label Nov 29, 2018

How to use a custom API endpoint in a RESTController?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If you have no access restriction you can use ``@Endpoint`` but if you have you

This comment has been minimized.

@s0nea

s0nea Nov 29, 2018

Member
Suggested change Beta
If you have no access restriction you can use ``@Endpoint`` but if you have you
If you don't have any access restriction you can use ``@Endpoint``. But if you do
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If you have no access restriction you can use ``@Endpoint`` but if you have you
use instead ``@RESTController.Collection`` or ``@RESTController.Resource``, if

This comment has been minimized.

@s0nea

s0nea Nov 29, 2018

Member
Suggested change Beta
use instead ``@RESTController.Collection`` or ``@RESTController.Resource``, if
and ``RESOURCE_ID`` is set you'll have to use ``@RESTController.Collection`` or ``@RESTController.Resource`` instead.

This comment has been minimized.

@Devp00l

Devp00l Nov 29, 2018

Author Contributor

You only can use @RESTController.Resource if RESOURCE_ID is set, but you can use @RESTController.Collection without having the RESOURCE_ID set.


If you have no access restriction you can use ``@Endpoint`` but if you have you
use instead ``@RESTController.Collection`` or ``@RESTController.Resource``, if
``RESOURCE_ID`` is set. ``@Endpoint`` will fail here, as it does not know which

This comment has been minimized.

@s0nea

s0nea Nov 29, 2018

Member
Suggested change Beta
``RESOURCE_ID`` is set. ``@Endpoint`` will fail here, as it does not know which
Otherwise ``@Endpoint`` will fail here, as it does not know which
import { NotificationType } from '../enum/notification-type.enum';

export class CdNotificationConfig {

This comment has been minimized.

@s0nea

s0nea Nov 29, 2018

Member

Would it possibly make sense to move all the changes related to CdNotificationConfig to a separate (pre-)PR? Because this one is quite long already.

This comment has been minimized.

@Devp00l

Devp00l Nov 29, 2018

Author Contributor

Done :)

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch 4 times, most recently from f46b72b to bc78ee0 Nov 29, 2018

@Devp00l Devp00l changed the title mgr/dashboard: Prometheus integration [After #25327, #25325] mgr/dashboard: Prometheus integration Nov 29, 2018

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch 3 times, most recently from 6974d1c to 5701d56 Nov 29, 2018

@LenzGr LenzGr added the needs-review label Dec 5, 2018

#. Use the Prometheus Alertmanager API.

#. Use both sources simultaneously.

This comment has been minimized.

@LenzGr

LenzGr Dec 6, 2018

Contributor

What are the benefits/differences between these methods here?
When would one want to use one or the other?
Please elaborate on the rationale for having these two different methods.

@@ -366,6 +366,63 @@ To enable SSO::

$ ceph dashboard sso enable saml2

Enabling Prometheus alerting
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This comment has been minimized.

@LenzGr

LenzGr Dec 6, 2018

Contributor

Please add a short note about what the Prometheus Alert manager is all about, including an URL that points to more information, especially how to install and configure it.

from ..settings import Settings


# The receiver is needed in order to receive alert notifications (reports)

This comment has been minimized.

@p-na

p-na Dec 6, 2018

Contributor

Can you please move this documentation to the PrometheusReceiver class?

Show resolved Hide resolved src/pybind/mgr/dashboard/controllers/prometheus.py Outdated

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch from 5701d56 to 5402a3c Dec 7, 2018

@Devp00l

This comment has been minimized.

Copy link
Contributor Author

Devp00l commented Dec 7, 2018

Addressed all comments

@Devp00l Devp00l changed the title [After #25327, #25325] mgr/dashboard: Prometheus integration [After #25325] mgr/dashboard: Prometheus integration Dec 11, 2018

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch from 5402a3c to ac1fdcd Dec 13, 2018

@Devp00l Devp00l changed the title [After #25325] mgr/dashboard: Prometheus integration mgr/dashboard: Prometheus integration Dec 13, 2018

@Devp00l Devp00l removed the needs-rebase label Dec 13, 2018

@Devp00l

This comment has been minimized.

Copy link
Contributor Author

Devp00l commented Jan 25, 2019

@p-na Sorry I miss understood you I thought you mend squashing all commits into one. Will do that.

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch from 06c4114 to ef43b81 Jan 25, 2019

@Devp00l

This comment has been minimized.

Copy link
Contributor Author

Devp00l commented Jan 25, 2019

Addressed all comments. Put interval outside of angular so that the e2e test will pass again.

@epuertat epuertat requested review from b-ranto and zmc Jan 29, 2019

Requested changes have been resolved.

Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved doc/mgr/dashboard.rst Outdated
Show resolved Hide resolved src/pybind/mgr/dashboard/controllers/prometheus.py Outdated
Show resolved Hide resolved ...d/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts Outdated

it('should have the following column set up if customCss is defined', () => {
component.customCss = {
'answer-of-everything': 42

This comment has been minimized.

@votdev

votdev Jan 30, 2019

Contributor

:-)

Show resolved Hide resolved ...d/mgr/dashboard/frontend/src/app/shared/services/notification.service.ts Outdated
Show resolved Hide resolved ...dashboard/frontend/src/app/shared/services/prometheus-alert-formatter.ts Outdated

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch from ef43b81 to 31597d5 Jan 30, 2019

@Devp00l

This comment has been minimized.

Copy link
Contributor Author

Devp00l commented Jan 30, 2019

@votdev Addressed all your comments 👍

@votdev

votdev approved these changes Jan 30, 2019

@p-na p-na added the needs-rebase label Jan 30, 2019

Devp00l added some commits Nov 6, 2018

mgr/dashboard: Add the Prometheus alerts
The backend is now capable of receiving alert notifications from
the Prometheus alertmanager and it can get all alerts with all kinds of
parameters from the API of the same.

In the frontend Prometheus alerts can be found in "Cluster > Alerts". Incoming
notifications can be seen as usual in the notifications popover.

To clarify:
Prometheus alerts are received from the alertmanager API.
Prometheus alert notification are send from the alertmanager to the
backend receiver. An alert notification can have multiple alerts, but
these alerts differ from the prometheus alerts.

To clarify that, I've added some models and services.

If one of the methods to get alerts contains changes the user will be
notified.

The documentation explains how to configure the alertmanager to use the
dashboard receiver and how to connect the use of the alertmanager API.
Further it explains where to find the alerts and what happens if they
are configured and something is happening.

Fixes: https://tracker.ceph.com/issues/36721
Signed-off-by: Stephan Müller <smueller@suse.com>
mgr/dashboard: Provide values with different style in KV-table
Now it's possible to style values inside the KV-table based on the
values.

Fixes: https://tracker.ceph.com/issues/37951
Signed-off-by: Stephan Müller <smueller@suse.com>
mgr/dashboard: Application icons in notifications
Now notifications and alerts show an application icon, that gives a hint
about their origin.

Fixes: https://tracker.ceph.com/issues/37950
Signed-off-by: Stephan Müller <smueller@suse.com>
mgr/dashboard: Updated messages.xlf
Signed-off-by: Stephan Müller <smueller@suse.com>

@Devp00l Devp00l force-pushed the Devp00l:wip-prometheus-alerting branch from 31597d5 to a768571 Jan 30, 2019

@Devp00l

This comment has been minimized.

Copy link
Contributor Author

Devp00l commented Jan 31, 2019

jenkins test dashboard

@Devp00l

This comment has been minimized.

Copy link
Contributor Author

Devp00l commented Jan 31, 2019

jenkins test dashboard

@s0nea

s0nea approved these changes Jan 31, 2019

Copy link
Member

s0nea left a comment

LGTM

@callithea

This comment has been minimized.

Copy link
Member

callithea commented Feb 4, 2019

http://pulpito.ceph.com/laura-2019-02-01_11:52:18-rados:mgr-wip-lpaduano-testing-prometheus-alerts-distro-basic-smithi/

  • There's one test failure (which does not seem to be related to this PR): Test failure: setUpClass (tasks.mgr.dashboard.test_rgw.RgwUserKeyTest)

@Devp00l Devp00l removed the needs-review label Feb 4, 2019

@s0nea s0nea merged commit 6116c96 into ceph:master Feb 4, 2019

6 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
ceph dashboard tests ceph dashboard tests succeeded
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment