You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by lulf <gi...@git.apache.org> on 2017/04/06 09:40:28 UTC

[GitHub] qpid-dispatch pull request #159: Metrics support

GitHub user lulf opened a pull request:

    https://github.com/apache/qpid-dispatch/pull/159

    Metrics support

    @ganeshmurthy @ted-ross This is a proposal of metrics in the router and expose them to a prometheus agent. 
    
    This work would allow anyone running the router on openshift to easily integrate it with hawkular or any other store that supports reading prometheus endpoints.
    
    The PR consists of:
    
    * An API for a metric manager, that keeps track of all metrics in the router, and that is able to export them (see metric_manager.h and metric_manager.c)
    * An API for a metric, used to increment/set values for a particular metric, with labels (see metric.h, metric_private.h, metric.c)
    * Integration with the http-libwebsockets for exposing the metrics in prometheus format on http://host:httpport/metrics (http-libwebsockets.c)
    * An example of a metric in router_core/connections.c when opening a connection. (router_core/router_core.c, router_core/connections.c)
    
    The libraries are written with the idea that incrementing a metric should be as cheap as possible (though there are several optimizations to be made for efficiently looking up labels). A metric is registered in the metric manager, which will store it in its internal database. A handle to the metric (num_connections for instance) is stored for efficient metric reporting. The handle is used whenever the metric should be incremented, decremented or set.
    
    The API also contains a set of macros that makes passing labels less verbose.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lulf/qpid-dispatch metrics

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/qpid-dispatch/pull/159.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #159
    
----
commit 03e0ebbdaf446199be42033b5ea63c654a89ddfc
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-03-31T11:30:13Z

    Add data structures for tracking metrics
    
    * Add num_connections metric as initial metric for testing

commit 2e3b688d1cc862c69632e8aa6256fa6e944a3b87
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-03-31T13:53:23Z

    Add support for labels in the interface

commit e7659af42a23f72c4ffb73841f821668f827a7c3
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-03-31T14:23:48Z

    Support reporting with labels

commit 0bbe0368cf7ee042c406401625a89f74f043e152
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-03-31T15:03:12Z

    Pass some labels

commit 37a5e0b2d1d5ff8b40d84245a5cbfcb07e39df2a
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-04-06T08:59:52Z

    Add metric manager for keeping track of metrics and exporting

commit 0cede49376a81f6c7904b3d29c056697ab94a20d
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-04-06T09:00:07Z

    Expose metrics in http

commit 39cdaed1244d6dcae52e06d03f32bf303ae1995a
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-04-06T09:11:13Z

    Free resources at shutdown

commit b259e244a98c528f9ddeee0da7179d2d490781dc
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-04-06T09:25:36Z

    Initialize all memory

commit c847e9bc3a5f3731f33a6e88db514b4b60e1f6ae
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2017-04-06T09:33:35Z

    Add dec with label

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-dispatch pull request #159: Metrics support

Posted by lulf <gi...@git.apache.org>.
Github user lulf closed the pull request at:

    https://github.com/apache/qpid-dispatch/pull/159


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-dispatch issue #159: Metrics support

Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on the issue:

    https://github.com/apache/qpid-dispatch/pull/159
  
    At the risk of contradicting my previous post, I would add that having an HTTP front end on management data might be a worthwhile enhancement for some use cases as long as it used the same data sources as the AMQP front end.  Mind would have to be paid to the security model for such HTTP accesses to management data.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-dispatch issue #159: Metrics support

Posted by ChugR <gi...@git.apache.org>.
Github user ChugR commented on the issue:

    https://github.com/apache/qpid-dispatch/pull/159
  
    This patch adds a competing statistic for one that already exists. Going forward it would require a completely redundant set and every statistic would be counted twice. That's not desirable for the long run.
    
    Another approach might be to hook up the http-libwebsockets to the existing statistics and then to export them in the desired format. The existing management database has configuration data and runtime statistics together. See https://github.com/apache/qpid-dispatch/blob/master/python/qpid_dispatch/management/qdrouter.json.readme.txt 
    
    This would be more work to hook up the the metric_manager as the code would have to hunt for all the values where they exist today. But in the end the resulting reports would have identical descriptions and values reported by qdstat and the other management tools.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-dispatch issue #159: Metrics support

Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:

    https://github.com/apache/qpid-dispatch/pull/159
  
    Thank you for your comments. My example metric was perhaps too simplicic, apologies for that. Here are more examples of the metrics I'd like to be able to provide:
    
    * Counting the number of connections made (tagged with container id) in total, even if they are not active now
    * The number of failed connection attempts, link attaches etc.
    * Internal router errors of different types (could be non-amqp specific)
    * Latency metrics (time spent in core thread, in I/O thread etc), possibly as a histogram (not supported in this example API, but part of the prometheus metric spec: https://prometheus.io/docs/practices/histograms/)
    * Ability to report quantiles and histograms
    
    Is this information that I can retrieve in the same way as qdstat or specify in the management.json?  My impression was that qdstat only allowed for 'current state' type of metrics. If we can represent the above using management, I agree it could be better to use that.
    
    However, there is also the question of the impact of collecting metrics (tradeoff between quering for the metrics vs. the cost of reporting the metrcs). Reporting a metric explicitly may scale better, since a counter can be incremented and queried without locking in the common case. Correct me if I'm wrong, but my impression was that management operations either has to run in the core thread or do some locking?
    
    Would an alternative here be to have management/qdstat retrieve the metrics from the metrics manager? This would resolve the concerns of reporting metrics twice, and could also potentially enrich the qdstat output with values from histograms etc.
    
    For reference, here is the document describing the format: https://prometheus.io/docs/instrumenting/exposition_formats/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-dispatch issue #159: Metrics support

Posted by ted-ross <gi...@git.apache.org>.
Github user ted-ross commented on the issue:

    https://github.com/apache/qpid-dispatch/pull/159
  
    I share Chuck's concern that this PR is introducing an alternative and redundant management access mechanism.  I would certainly prefer to add the desired metrics to the AMQP-based access mechanism that is already in place.
    One of the advantages of the AMQP management mechanism is that it is routable.  With a single connection to a router in a network, a process can query data from all of the routers in the network (and eventually brokers as well).  Using the HTTP access point only allows access to a single router.
    Is there a way to bridge the gap between the AMQP management protocol and Prometheus?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org