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

Enable examples to run against already running Prometheus #114

Closed
wants to merge 5 commits into from

Conversation

izderadicka
Copy link

If I wanted to use Prometheus from Docker for examples I hit issue that examples wanted to start new one, locally installed.
This small change enables to run examples if Prometheus process is already running locally.

Copy link
Contributor

@emschwartz emschwartz left a comment

Choose a reason for hiding this comment

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

Thanks @izderadicka for this contribution!

examples/util/src/lib.rs Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
izderadicka and others added 2 commits June 19, 2023 16:44
pub fn run_prometheus(enable_exemplars: bool) -> Option<ChildGuard> {
let system = System::new_all();

if system.processes_by_exact_name("prometheus").any(|_| true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, will this return something if it's running in Docker?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, confirmed on linux. There is no magic in docker it's just a process, running in its own kernel namespace. From parent ns you see it.

But it may be issue on other OSes like Win, MacOS, were Docker does not share kernel.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I tested it on mac and it doesn't find the process. I believe the process name is docker :/

Copy link
Author

Choose a reason for hiding this comment

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

As noted below - works for linux and native proccess - I think on other OS still have some value - can use already running instance.


```
# from this directory
docker run -it -v $(pwd)/util/prometheus.yml:/prometheus/prometheus.yml --network host prom/prometheus --enable-feature=exemplar-storage
Copy link
Contributor

Choose a reason for hiding this comment

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

The --network host didn't work for me on Mac but replacing that with the following did work

Suggested change
docker run -it -v $(pwd)/util/prometheus.yml:/prometheus/prometheus.yml --network host prom/prometheus --enable-feature=exemplar-storage
docker run -it -v $(pwd)/util/prometheus.yml:/prometheus/prometheus.yml -p 9090:9090 prom/prometheus --enable-feature=exemplar-storage

Copy link
Author

Choose a reason for hiding this comment

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

Yes this will work for prometheus port, but --network host is there for prometheus to be able to connect to localhost, where example is publishing its metrics.

I guess my approach is focusing on linux, on other OSes it might have use if prometheus is already running as native process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, that makes sense.

I think if we have instructions for running it in Docker, it would be best if they worked across OSes, though I'm not entirely sure what we'd need to do to make that work.

Copy link
Author

Choose a reason for hiding this comment

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

I'm do not have experiences with Docker Desktop on Win nor MacOS. In both cases it I think it runs in VM, so there is no chance to connect to localhost where example is exposing metrics.

However there is host.docker.internal internal DNS record which works for MacOS and Win - but not exactly the same for Linux.

I think there are many options how one can potentially run examples:

  • with locally installed Prometheus and examples will start/stop it - current scenario
  • with locally installed Prometheus, user will start it in advance - here the PR will help on all OS,
  • on Linux with Prometheus running in container - again this works with this PR
  • Win, MacOS (Docker Desktop)- it'll require more modification - first not trying to start Prometheus (env. variable?) , second change configuration localhost -> host.docker.internal
  • Prometheus running on different host?

Question is if this should be captured in examples logic, or just add bit more description to readme.

Definitely containerization of dependencies/services it trend for testing - TestContainers, so this would be also way for examples, unfortunately Prometheus has an "unlucky" feature, that instead of listening on port like majority servers do, it also needs to connect to the example. So I did not find any useful inspiration in TestContainers.

So options I see them now:

  • extend a bit readme
  • or close PR as this is not general enough - Linux only

@gagbo
Copy link
Member

gagbo commented Nov 14, 2023

Hello,

I just caught up with this (sorry for the delay), but it seems the main issue with reusing an existing Prometheus (even without the localhost host.docker.internal problems) is with the scaping configurations. How is the scraping configuration supposed to be passed around if you have an early return None?

And if the running prometheus locally has disabled the runtime management endpoints I don't think there's a way to get Autometrics examples running without disrupting the service that's already running on localhost.

If the main reason for the PR is that run_prometheus has issues with already-bound 9090 port, I think this can be solved by having run_prometheus search for a free port first, and reporting the URL in stdout when running

@izderadicka
Copy link
Author

@gagbo
let's start with reply to last part of your comment. No it's not about port conflict, I need to connect to already running instance of prometheus, not to start new one.

Returning None is just to not return the guard to Temporarily prometheus, which was not created.

Problem here is that my proposed solution works only only on Linux. Not on other platforms where Docker Desktop is used. That was the essence of previous conversations. As I'm not using other platforms I cannot provide general solution. So we can accept this one as special case for Linux or somebody more experienced to provide generalisation or just close it.

@gagbo
Copy link
Member

gagbo commented Nov 15, 2023

Oh I see, so the point of this PR is to automatically start Prometheus in a Docker container if it's missing, instead of having to have Prometheus installed on the computer.

The title of the PR made me think the goal was to connect to something that has been running for months on the machine already, with configured targets, that's why I was confused.

If that's the case, then a simple solution would be to write an extra /util/prometheus.docker.internal.yml config file, that configures scraping targets to point to host.docker.internal instead of localhost, and tell MacOS/Windows users to use this file instead of the current one (I haven't tested it yet, but I'm fairly confident it would work)

A (more complicated) solution could be to:

  • catch when the prometheus binary is missing,
  • if that's the case, directly use std::process::Command to spawn the container with the command,
  • use the Drop implementation of the ChildGuard to kill the spawned command (which will kill the Prometheus container)

The more complex solution still needs the simple version as a stepping stone to work, i.e. using a if cfg! statement to choose whether to use util/prometheus.yml or util/prometheus.docker.yml as the volume mount input argument basically. But if we're able to pull that off, it would keep the examples self contained (so no need to read the docs and find and copy/paste the docker run command), while also working on all major OSes even if you don't have Prometheus installed. What do you think?

If you don't want to commit more time to this though it's fine, we can put in the docs that it'll only work on Linux, and I'll patch the missing bits for different OSes.

(I also noticed that the util/prometheus.yml file also references the autometrics recording and alerting rules files, which allows to see how the instrumented code generates alerts (or not); but this file doesn't exist in this repo anymore so we'll tackle this in another issue)

@izderadicka
Copy link
Author

Actually it is other way around, currently - example always starts new prometheus, but I wanted to connect running instance.

But it's no issue for me now, I just needed it for testing and though somebody else might have similar problem. As general solution for this problem seems to be too complicated for this marginal problem with one example I propose to close it.

@gagbo
Copy link
Member

gagbo commented Nov 17, 2023

I see. Thanks for the input, we'll keep track of the more complicated solution in case it can help people not having to have Prometheus installed!

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.

None yet

3 participants