[any][17.08] notifications_designate.info queue has many unconsumed "dns.pool.update" messages due to ceilometer and designate notification_topics mismatch and missing events in event_definitions.yaml

I see the same:

ubuntu@juju-2758f7-xenial-pike-23:~$ sudo rabbitmqctl list_queues -p openstack name consumers messages | grep designate
notifications_designate.info 0 962

I think that ceilometer's event_definitions.yaml could be extended to consume those events.

https://github.com/openstack/ceilometer/blob/stable/ocata/etc/ceilometer/event_definitions.yaml#L405-L435

So, to break the comms down, designate components do both sending and receiving via AMQP.

1. nova to designate

https://git.io/vFMbn
nova -> {compute.instance.create.end, compute.instance.delete.start} -> designate-sink -> nova-handler

cfg.CONF.register_opts([
    cfg.ListOpt('notification-topics', default=['notifications']),
    cfg.StrOpt('control-exchange', default='nova'),

2. neutron to designate

https://git.io/vFMb8
neutron -> {floatingip.update.end, floatingip.delete.start} -> designate-sink -> neutron-handler

cfg.CONF.register_opts([
    cfg.ListOpt('notification-topics', default=['notifications']),
    cfg.StrOpt('control-exchange', default='neutron'),

https://docs.openstack.org/designate/pike/contributor/architecture.html#designate-sink
designate-sink is an optional service which listens for event Notifications, such as compute.instance.create.end, handlers are available for Nova and Neutron. Notification events can then be used to trigger record creation & deletion.

3. central -> other consumers (e.g. ceilometer-agent-notification)

https://github.com/openstack/designate/blob/stable/ocata/designate/central/service.py#L163-L164
https://github.com/openstack/designate/blob/stable/ocata/designate/central/service.py#L2213

The designate charm hard-codes the notification_topics to "notifications_designate" (outgoing traffic)

https://git.io/vFMAP
notification_topics = notifications_designate

While per oslo.messaging the default is "notifications" which is overridden by that

https://github.com/openstack/oslo.messaging/blob/stable/ocata/oslo_messaging/notify/notifier.py#L49-L57

    cfg.ListOpt('topics',
                default=['notifications', ],
                deprecated_opts=[
                    cfg.DeprecatedOpt('topics',
                                      group='rpc_notifier2'),
                    cfg.DeprecatedOpt('notification_topics',
                                      group='DEFAULT')
                ],
                help='AMQP topic used for OpenStack notifications.'),
]

charm-ceilometer, OTOH, doesn't redefine anything like that, which is why not only dns.pool.update but other designate-related messages as well are not received by it - even those that are in event_definitions.yaml.

As I pasted above, there are also no event_definitions on the ceilometer side for dns.pool.* including dns.pool.update.

https://github.com/openstack/ceilometer/blob/stable/ocata/etc/ceilometer/event_definitions.yaml#L405-L435

The use-case may be event/audit logging but at least in this case we will solve the lack of other consumers.

AFAIKs, designate uses standard DNS protocol messages so any AMQP messages are rather for custom logic that could be implemented via ceilometer + aodh or pipelines in addition to event logging to panko https://docs.openstack.org/designate/pike/contributor/architecture.html#designate-minidns

For that reason I would add all possible emitted messages to event_definitions.yaml:

https://docs.openstack.org/designate/pike/admin/notifications.html#emitters

4. mdns -> DNS backend <- nothing to do here

It's questionable which traits to add to event_definitions.yaml by default on the ceilometer side.

Each object has it's own set of traits (FIELDS) + inherited traits from base classes.

class Pool(base.DictObjectMixin, base.PersistentObjectMixin,
           base.DesignateObject):
    FIELDS = {

I would say that ceilometer needs to render event_definitions.yaml with a sane set of default event definitions but should allow one to supply a completely arbitrary event_definitions.yaml.

I could add PersistentObjectMixin and DesignateObject at least.

event_definitions.yaml is highly customizable and I do not think it is necessary to preseed it - it should just be taken from a user because modeling and parsing event and trait definitions in a charm is cumbersome. Another reason is that an operator may want to reduce unnecessary chatter in telemetry but given a charm manages the config it's not possible to just go and edit it.

https://docs.openstack.org/ceilometer/pike/contributor/events.html#definitions-file-format

Likewise, meters.yaml should be replaceable as well https://github.com/openstack/ceilometer/blob/stable/pike/ceilometer/data/meters.d/meters.yaml

This is necessary to avoid charm forks just to add another trait or two.

@dmitriis

Re your comments in #4 - ceilometer should ship with sane defaults for events and meters - any work to improve coverage should be done as part of ceilometer, not as part of the charm IMHO.

@james-page I do agree about upstream ceilometer defaults. I think the new object CRUD events were just not added there.

I think this goes both ways (upstream & charms) though to make things configurable in terms of traits (not just events processed).

An operator may want to limit event traits to FIELDS of certain mixins only or to parts of mixins. For that we may just allow supplying a yaml directly (if you need that granularity - feel free to write your own stuff in other words).

the topic side of this:

https://review.openstack.org/#/q/status:open%20topic:designate-notifications

Reviewed: https://review.openstack.org/521073
Committed: https://git.openstack.org/cgit/openstack/charm-neutron-api/commit/?id=45792d3c865149f2c94cef42b19c1385d7f7b0ee
Submitter: Zuul
Branch: master

commit 45792d3c865149f2c94cef42b19c1385d7f7b0ee
Author: Dmitrii Shcherbakov <email address hidden>
Date: Fri Nov 17 15:31:01 2017 +0300

    drop notifications_designate

    A separate topic has been used for nova, neutron -> designate. Let's use
    a single one instead.

    Change-Id: I17d0036f6dcbfd0e87849091e453d2a8b2bc498f
    Closes-Bug: #1710831

Reviewed: https://review.openstack.org/521072
Committed: https://git.openstack.org/cgit/openstack/charm-nova-compute/commit/?id=b89de21d47dc9114d20456452cd867041bcd83c5
Submitter: Zuul
Branch: master

commit b89de21d47dc9114d20456452cd867041bcd83c5
Author: Dmitrii Shcherbakov <email address hidden>
Date: Fri Nov 17 15:13:22 2017 +0300

    drop driver and topic settings in designate ctx

    A similar change will land in designate to drop this extra topic as
    well.

    Also, driver and topic overrides in a service-specific context are not a
    good idea.

    Change-Id: I804a34fb044090010ecfd2560594cc1f55e9bd21
    Closes-Bug: #1710831
    Closes-Bug: #1738100

Fix proposed to branch: stable/17.11
Review: https://review.openstack.org/528304

Fix proposed to branch: stable/17.11
Review: https://review.openstack.org/531334

Change abandoned by James Page (<email address hidden>) on branch: stable/17.11
Review: https://review.openstack.org/528304

Reviewed: https://review.openstack.org/521075
Committed: https://git.openstack.org/cgit/openstack/charm-designate/commit/?id=1879b16ea543b076c9555fdc0258355d39af9ad0
Submitter: Zuul
Branch: master

commit 1879b16ea543b076c9555fdc0258355d39af9ad0
Author: Dmitrii Shcherbakov <email address hidden>
Date: Fri Nov 17 14:59:50 2017 +0300

    drop notifications_designate, add missing section

    A separate topic has been used for nova, neutron -> designate. Let's use
    a single one instead.

    Change-Id: I709a6aca1843918a67d43bc4808a0013168fbc55
    Closes-Bug: #1710831

I think charm-based customization of event_definitions.yaml is needed after all although ceilometer needs to ship a more up-to-date list of events.

Reviewed: https://review.openstack.org/531334
Committed: https://git.openstack.org/cgit/openstack/charm-nova-compute/commit/?id=4adfc46f842e1aec20e739a7b6d94ab14e7f4cfe
Submitter: Zuul
Branch: stable/17.11

commit 4adfc46f842e1aec20e739a7b6d94ab14e7f4cfe
Author: Dmitrii Shcherbakov <email address hidden>
Date: Fri Nov 17 15:13:22 2017 +0300

    drop driver and topic settings in designate ctx

    A similar change will land in designate to drop this extra topic as
    well.

    Also, driver and topic overrides in a service-specific context are not a
    good idea.

    Closes-Bug: #1710831
    Closes-Bug: #1738100
    (cherry picked from commit b89de21d47dc9114d20456452cd867041bcd83c5)

    Change-Id: Ie2d50b1ccd31ac430ce09cb96aceace3a31b2b92

Fix proposed to branch: stable/17.11
Review: https://review.openstack.org/534504

Fix proposed to branch: stable/17.11
Review: https://review.openstack.org/534505

Fix proposed to branch: stable/17.11
Review: https://review.openstack.org/534506

Related fix proposed to branch: master
Review: https://review.openstack.org/540833

Related fix proposed to branch: master
Review: https://review.openstack.org/540834

Related fix proposed to branch: master
Review: https://review.openstack.org/540837

Reviewed: https://review.openstack.org/540833
Committed: https://git.openstack.org/cgit/openstack/charm-designate/commit/?id=269ba2bb1d48ea515b315a9a7226dc010784ccaa
Submitter: Zuul
Branch: master

commit 269ba2bb1d48ea515b315a9a7226dc010784ccaa
Author: Dmitrii Shcherbakov <email address hidden>
Date: Mon Feb 5 14:22:54 2018 +0300

    reintroduce designate_notifications topic

    Change I709a6aca1843918a67d43bc4808a0013168fbc55, commit hash
    1879b16ea543b076c9555fdc0258355d39af9ad0 introduced a
    backwards-incompatible change in the fact that designate stopped
    consuming messages using designate_notifications topic from nova and
    neutron.

    While designate.conf change in the mentioned commit is needed to make
    sure Designate emits its own notifications on the topic that ceilometer
    uses ("notifications"), for nova and neutron to designate notifications
    a separate topic should be used as before.

    A revert of the previous commit is not done as it addresses the previous
    issue but introduces a new one.

    Change-Id: I6b5fd1b5f9bae1cd4a1683fd476b456e81d6c79e
    Related-Bug: #1710831

Reviewed: https://review.openstack.org/540834
Committed: https://git.openstack.org/cgit/openstack/charm-nova-compute/commit/?id=6b9b02c11201d1afd960af0196eff56c6f81ffef
Submitter: Zuul
Branch: master

commit 6b9b02c11201d1afd960af0196eff56c6f81ffef
Author: Dmitrii Shcherbakov <email address hidden>
Date: Thu Feb 8 17:06:50 2018 +0300

    emit notifications on notifications_designate

    Change I804a34fb044090010ecfd2560594cc1f55e9bd21, commit hash
    b89de21d47dc9114d20456452cd867041bcd83c5 dropped notifications_designate
    completely to solve a problem described in pad.lv/1738100, however, to
    make this change backwards compatible notifications_designate should be
    used in addition to "notifications" topic used by default.

    This way designate will continue to receive notifications on
    notifications_designate from nova, and ceilometer will get notifications
    using the "notifications" topic.

    Change-Id: I245f5b263994c204a5e521dad542ed83952f54b8
    Related-Bug: #1710831
    Related-Bug: #1738100

Reviewed: https://review.openstack.org/540837
Committed: https://git.openstack.org/cgit/openstack/charm-neutron-api/commit/?id=ff2547c6d552e85890c86d0677126ca1c09a9a04
Submitter: Zuul
Branch: master

commit ff2547c6d552e85890c86d0677126ca1c09a9a04
Author: Dmitrii Shcherbakov <email address hidden>
Date: Thu Feb 8 17:16:57 2018 +0300

    Revert "drop notifications_designate"

    This reverts commit 45792d3c865149f2c94cef42b19c1385d7f7b0ee.

    During the discussions in reviews relevant to the bugs below the
    decision was to use both notifications_designate and notifications
    topics.

    Related-Bug: #1710831
    Related-Bug: #1738100
    Change-Id: I496f54d9917fe8c8279ebb91cc2fed02b74b181c

Change abandoned by Dmitrii Shcherbakov (<email address hidden>) on branch: stable/17.11
Review: https://review.openstack.org/534505

Change abandoned by Dmitrii Shcherbakov (<email address hidden>) on branch: stable/17.11
Review: https://review.openstack.org/534504

Change abandoned by Dmitrii Shcherbakov (<email address hidden>) on branch: stable/17.11
Review: https://review.openstack.org/534506
Reason: Old code.