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

[WIP][prometheusexporter] Initial contribution #9726

Closed
wants to merge 1 commit into from

Conversation

pravussum
Copy link
Contributor

This is an initial PR for a new version of a Prometheus metrics exporter plugin.

It's inspired by the existing one discussed in https://community.openhab.org/t/new-add-on-bundle-for-prometheus-health-metrics/48094/43

The produced metric names are almost fully compatible (details in the readme).

My approach is different, though. Instead of providing a REST interface the produced metrics are written to items instead.
The existing REST API for accessing the item state can be used to feed Prometheus.
There are ongoing discussions, though.

Signed-off-by: Robert Bach <openhab@mortalsilence.net>
@pravussum pravussum requested a review from a team as a code owner January 6, 2021 22:33
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!
Let us move the discussion from openhab/openhab-core#774 to here then. I have a few issues with the current approach, why I think this does not fit this way into the openHAB architecture:

  • The name "exporter" suggests that data is exported by this binding, while bindings rather import data. If I understand it correctly, nothing is actually exported, but item states are provided by it. A name like "openHAB Metrics Binding" might thus be a better match.
  • You refer to some prometheusexporterItem type, which does not exist and cannot be introduced. If I understand correctly, you intend to write Prometheus-formatted strings as item states - that's a complete misuse of items and definitely not a valid approach.
  • If you'd go for an "openHAB Metrics Binding", you could have channels that directly provide the metric values as Number items, which could then be used by users to show in UIs and use them as items are supposed to be used. In that case, your binding would not have any relation to Prometheus anymore. The strong advice here would actually be not to introduce a new binding, but to add those channels to the existing systeminfo binding.
  • If you'd prefer to go for providing Prometheus-compatible data through the REST API, you need to go for an IO add-on instead of a binding. Such an "Prometheus IO add-on" can register a REST resource or a servlet or on its own and can provide anything it likes through this.

To me, both approaches ("Systeminfo Binding" and "Prometheus IO add-on") make sense and can also clearly coexist and be accepted. For both it would make a lot of sense to implement openhab/openhab-core#774 in the core framework, which could provide the relevant data through a clean interface for both (and other future) use cases.

@pravussum
Copy link
Contributor Author

Hm, ok, thanks for the insights, although that's not what I wanted to hear. I somehow hoped to get around the whole "provide a REST resource" thingy by using the already existing REST API (to quickly get to work my monitoring again after the OH3 update).
My main intention was to simplify the setup, the old binding included a lot of fiddling to get it to work.
But if you call it "complete misuse of items" that's pretty explicit 😄
Anyway, I'll have to think about it and will come up with a new approach, I guess it'll include a combination of a systeminfo update (to gather the missing info like thread pool figures and JVM stuff) plus an IO addon to transform systeminfo values into a Prometheus compatible format.

@kaikreuzer
Copy link
Member

Don't be afraid about implementing additional REST resources in an IO add-on - it is pretty simple to do, see e.g. this example.

Would you also be interested in looking into openhab/openhab-core#774? I probably won't find time to work on that anytime soon, so help would be highly appreciated!

@pravussum pravussum closed this Jan 18, 2021
@pravussum
Copy link
Contributor Author

Closing in favour of openhab/openhab-core#2133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants