Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Use monitor component #64

Merged
merged 16 commits into from
Apr 17, 2018
Merged

Use monitor component #64

merged 16 commits into from
Apr 17, 2018

Conversation

mjs
Copy link
Contributor

@mjs mjs commented Apr 10, 2018

This PR changes influx-spout so that all components report their own metrics in Prometheus format. These are consumed and aggregated by the monitor and are then served up over HTTP.

Highlights:

  • added "Snapshot" concept to Stats type (more convenient than Clone())
  • removed Stats.Clone() (no longer used)
  • added helpers for generating Prometheus lines from a Stats Snapshot.
  • added test helpers for checking metrics sent to the monitor subject
  • listener, filter & writer now publish to the NATS monitor subject using Prometheus format
  • LineFormatter type removed as it is no longer used
  • Set a default port for the monitor HTTP listener (9331, see below)
  • Updated the README.md to include the monitor
  • Added the monitor to the end-to-end test

I have registered port 9331 as the "official" default port for influx-spout Prometheus metrics (https://github.com/prometheus/prometheus/wiki/Default-port-allocations). This can of course be overridden through configuration.

I have tested the monitor /metrics HTTP endpoint with the telegraf Prometheus input to ensure that influx-spouts' metrics are correctly collected.

Closes #29.

mjs added 15 commits April 10, 2018 11:03
This will make it more convenient to generate Prometheus lines from a
Stats.
New function to takes a Stats Snapshot and convert it to Prometheus
metrics lines.
All usage of this have been replaced by AssertMonitor.
This helper is useful for cases where a single Prometheus metric line
is required (instead of generating from a Snapshot).
No longer used now that the components are publishing metrics using
Prometheus format.
This has been superseded by the Snapshot method.
@mjs mjs requested review from wrigtim, amnonbc and oplehto April 10, 2018 06:07
@mjs
Copy link
Contributor Author

mjs commented Apr 10, 2018

One thing I'd like feedback on is the structure of the metrics reported by the various influx-spout components. For example, one of the listener's metrics looks like this:

received{listener="listener_name"} 5 1523337329528

All the metrics include at a minimum a label like this which identifies the component type and name. Is that sufficiently convenient?

@oplehto
Copy link
Contributor

oplehto commented Apr 10, 2018

I haven't played around much with Prometheus yet but perhaps a preferrable approach would be to have {component="listener", name="endpoint_name"}.

This way we could set up Grafana templates in the form of received{name="$name"} which would cover all the components (of course not all the metrics would be applicable for all components but at least some).

@mjs
Copy link
Contributor Author

mjs commented Apr 10, 2018

@oplehto Yep, that sounds better. What's there now is just a direct mapping of the InfluxDB line protocol mappings but separating out the component type and name will make the metrics easier to deal with. I'll update the PR.

Copy link
Contributor

@amnonbc amnonbc left a comment

Choose a reason for hiding this comment

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

LGTM

This simplifies querying of influx-spout metrics.
@mjs mjs merged commit 203cc15 into jumptrading:master Apr 17, 2018
@mjs mjs deleted the use-monitor branch April 17, 2018 04:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants