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

prometheus:stop() hangs when run from an ejabberd module #66

Open
marcphilipp opened this issue May 24, 2017 · 13 comments
Open

prometheus:stop() hangs when run from an ejabberd module #66

marcphilipp opened this issue May 24, 2017 · 13 comments

Comments

@marcphilipp
Copy link

First of all, thanks for creating this library, very useful!

I'm trying to implement an ejabberd module that uses prometheus:

Here's what I have:

-module(mod_prometheus).

-include("ejabberd_http.hrl").
-include("logger.hrl").

-behavior(gen_mod).

-export([
  start/2,
  stop/1,
  depends/2,
  mod_opt_type/1,
  process/2
]).

start(Host, _Opts) ->
  ?INFO_MSG("~s starting on ~p", [?MODULE, Host]),
  prometheus:start().

stop(Host) ->
  ?INFO_MSG("~s stopping on ~p", [?MODULE, Host]),
  prometheus:stop(),
  ?INFO_MSG("~s stopped on ~p", [?MODULE, Host]),
  ok.

depends(_Host, _Opts) ->
  [].

mod_opt_type(_) ->
  [].

process([], #request{method = 'GET'}) ->
  {
    200,
    [{<<"Content-Type">>, prometheus_text_format:content_type()}],
    prometheus_text_format:format()
  }.

However, when I shutdown/restart ejabberd, prometheus:stop() never returns:

[...]
2017-05-24 17:55:34.446 [info] <0.63.0>@mod_prometheus:stop:30 mod_prometheus stopping on <<"localhost">>

Do you have an idea how I can find out what's going on? Any help would be highly appreciated!

@deadtrickster
Copy link
Owner

Hi, glad you found this lib useful :-) Will look into this shortly.

@marcphilipp
Copy link
Author

I've done some further investigation. It looks like there's a deadlock because the application_controller is already busy stopping the ejabberd application and handle_call on it blocks indefinitely.

I guess I could just refrain from stopping the prometheus application completely. ejabberd does this as well for a bunch of internally used applications. But that doesn't really feel clean.

I've also tried to call prometheus_sup:start_link() directly, but then all the reports are empty. So I guess it has to be used as an application.

@deadtrickster
Copy link
Owner

Frankly I'm not that familiar with ejabberd plugin architecture (although I am (was) planning to write a plugin). So I need some time to learn it. The problem as I see from your latest comment is that this stop callback is called when main ejabberd app is stopping -> deadlock. Am I right?

Also I don't know what's your use-case, but there is https://github.com/deadtrickster/prometheus-httpd with prometheus_http_impl module just for implementing /metric endpoints.

@marcphilipp
Copy link
Author

Frankly I'm not that familiar with ejabberd plugin architecture (although I am (was) planning to write a plugin).

Interesting! Are you planning on collecting additional application-specific metrics like you did for RabbitMQ (e.g. online users per vhost)?

The problem as I see from your latest comment is that this stop callback is called when main ejabberd app is stopping -> deadlock. Am I right?

Yep, I think so.

Also I don't know what's your use-case, but there is https://github.com/deadtrickster/prometheus-httpd with prometheus_http_impl module just for implementing /metric endpoints.

Yeah, I've seen that. However, ejabberd has a built-in HTTP server that we're already using, so I want to expose it there. As you can see, it's not a lot of code (thanks to you).

@deadtrickster
Copy link
Owner

Are you planning on collecting additional application-specific metrics like you did for RabbitMQ (e.g. online users per vhost)?

You know, it'd be really cool if you find time to write the plugin. I can help of course.

However, ejabberd has a built-in HTTP server that we're already using, so I want to expose it there

Yeah, but you missing content negotiation, compression and authentication. prometheus_http_impl module abstracts this and it's server-agnostic (e.g. https://gitlab.com/barrel-db/barrel-platform/blob/master/apps/barrel_monitor/src/barrel_monitor_exporter_handler.erl).

Yep, I think so.

Can we somehow inject prometheus to ejabber supervision tree? You tried it and it didn't work?

@marcphilipp
Copy link
Author

marcphilipp commented May 24, 2017

You know, it'd be really cool if you find time to write the plugin. I can help of course.

Let me get a barebones plugin working locally and then we can talk about it. 🙂

Yeah, but you missing content negotiation, compression and authentication. prometheus_http_impl module abstracts this and it's server-agnostic.

I wasn't aware of prometheus_http_impl, thanks for the pointer!

Can we somehow inject prometheus to ejabber supervision tree? You tried it and it didn't work?

I didn't add prometheus_sup to a supervisor, just linked it to the ejabberd process that starts modules and stopped prometheus_sup when the module was stopped. That produced empty output of prometheus_text_format:format().

@deadtrickster
Copy link
Owner

deadtrickster commented Jul 9, 2017

Hi, how's it going? Maybe spawn_link a new process in stop/1, trap_exit and after 'main' process exits call prometheus:stop? Also, why you want to stop it anyway :-)

@marcphilipp
Copy link
Author

ejabberd modules have to implement the gen_mod behavior which includes start/2 and stop/1. Right now, I don't stop it at all... which has no immediate implications but still feels wrong.

Would you mind sketching out your idea in a bit more detail?

@deadtrickster
Copy link
Owner

deadtrickster commented Jul 10, 2017

Would you mind sketching out your idea in a bit more detail?

Yes, hopefully soon.

So stop/1 called with Host. Will it be called for all Hosts or just for one?

@marcphilipp
Copy link
Author

It depends on how you register it. Since the general metrics are not host-specific, I worked around this by just registering it for a single host, i.e. localhost.

@deadtrickster
Copy link
Owner

Thanks, But can we stop ejabberd plugin for specific Host? or they stopped globally? In former case stopping prometheus isn't good for other running Hosts...

@marcphilipp
Copy link
Author

Yes, you can stop plugins for a specific host. For me, that's not a problem. But for the general case that's undesirable. I guess it would be better to "attach" it to the ejabberd application but I don't think ejabberd provides a hook to do so.

@jpds
Copy link

jpds commented Apr 28, 2023

I spent today working on a gatherer for ejabberd, and actually didn't get much further than the code in the first comment on my own.

There is also a basic exporter at https://github.com/skythet/ejabberd-prometheus-exporter/blob/master/mod_prometheus.erl - however I'd much rather use this library.

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

No branches or pull requests

3 participants