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

Add nats-prometheus to the official images #4069

Closed

Conversation

ColinSullivan1
Copy link

This PR is to add an image for the Prometheus NATS Exporter in the the NATS product family.

Checklist for Review

NOTE: This checklist is intended for the use of the Official Images maintainers both to track the status of your PR and to help inform you and others of where we're at. As such, please leave the "checking" of items to the repository maintainers. If there is a point below for which you would like to provide additional information or note completion, please do so by commenting on the PR. Thanks! (and thanks for staying patient with us ❤️)

  • associated with or contacted upstream?
  • does it fit into one of the common categories? ("service", "language stack", "base distribution")
  • is it reasonably popular, or does it solve a particular use case well?
  • does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • dockerization review for best practices and cache gotchas/improvements (ala the official review guidelines)?
  • 2+ dockerization review?
  • existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
  • if FROM scratch, tarballs only exist in a single commit within the associated history?
  • passes current tests? any simple new tests that might be appropriate to add? (https://github.com/docker-library/official-images/tree/master/test)

@ColinSullivan1
Copy link
Author

Anything I need to add here? Thanks!

@igorvpcleao
Copy link

Hi @ColinSullivan1!

How can I help you with this task?
What is missing to have this in production?

Thanks!

@ColinSullivan1
Copy link
Author

@igorvpcleao , thanks for offering to help!

I'm not aware of anything missing, and was asking if was there anything holding this back from being merged? If it looks OK, we'd love to see the Prometheus NATS Exporter added as an official image...

Thanks,
Colin

@igorvpcleao
Copy link

@tianon @yosifkit Could you help to get this PR merged?

@ColinSullivan1
Copy link
Author

Thanks @igorvpcleao!

@tianon
Copy link
Member

tianon commented May 21, 2018

Sorry for the delay -- I think the initial concern here is in the further proliferation of nats-* images. IMO this would make perfect sense living in tags of the nats image itself (ala nats:prometheus-0.1.0 or something, especially since it's only useful in the context of a running NATS server instance), or even as a separate image under a generic NATS Project Docker Hub organization instead. Thoughts?

@igorvpcleao
Copy link

Oh, makes sense. The only problem I see of having specific tags living in nats image itself is that it can get a bit messy, IMO. It would be great to have Prometheus metrics exposed within nats-server itself, but I see nats-server and nats-exporter as independent projects. I really enjoyed the idea of having nats-server and related projects within a generic NATS organization. Are there disadvantages to this approach?

@gcolliso
Copy link

gcolliso commented Jun 4, 2018

@igorvpcleao @tianon Is there any update on this PR merge?

NATS would like to do whatever is the most prudent wrt to the individual NATS images but we need some guidance as to what is the correct path with advantages and disadvantages. A ticket was logged on 5/23 with Support but no response as of yet. Any assistance would be appreciated.

@caarlos0
Copy link

caarlos0 commented Jul 3, 2018

any updates on this? I'm looking forward to use this image :D

@docker-library-bot
Copy link
Member

Hello! ✨

Thanks for your interest in contributing to the official images program. 💭

As you may have noticed, we've usually got a pretty decently sized queue of new images (not to mention image updates and maintenance of images under @docker-library which are maintained by the core official images team). As such, it may be some time before we get to reviewing this image (image updates get priority both because users expect them and because reviewing new images is a more involved process than reviewing updates), so we apologize in advance! Please be patient with us (and avoid poking us about your image via other communication means -- rest assured, we've seen your PR and it's in the queue). ❤️

We do try to proactively add and update the "new image checklist" on each PR, so if you haven't looked at it yet, that's a good use of time while you wait. ☔

Thanks! 💖 💙 💚 ❤️

@tianon
Copy link
Member

tianon commented Jul 18, 2018

Thanks @docker-library-bot

I really enjoyed the idea of having nats-server and related projects within a generic NATS organization. Are there disadvantages to this approach?

Sorry for the delay! If you're speaking of something like nats/xyz, I think the disadvantages there would simply be that you don't get the benefits provided by this program (Dockerfile review, multi-architecture support/infrastructure, top-level discoverability, etc). If you don't need or have other means of getting similar benefits, that'd be a fine strategy.

I think we're also willing to accept this as a new image if it really doesn't make sense as a tag of the generic nats image, but I'm not totally convinced (especially since it doesn't really provide any value without a nats server being in the mix already). It's already a little weird IMO to have both nats and nats-streaming, and I think adds to user confusion to a certain extent, but not enough that I think it's worth being a blocker on. I think the argument that nats-prometheus really isn't useful by itself is a stronger one that makes me question the viability of this as a totally separate thing.

@ColinSullivan1
Copy link
Author

@tianon , Thanks! I can understand your position. This image does require a NATS server or NATS streaming server to really be useful.

There are a number of reasons we wanted to provide this as a separate image though:

  • This keeps the NATS server images slim and provide users a choice of including exporter functionality through docker compose, pairing with NATS server or streaming server in docker swarm. or as a image alongside a NATS server image in a Kubernetes pod. This image will act as an exporter for both the NATS server and NATS streaming server.
  • A characteristic of NATS that our users love is it being lightweight - and as docker images can be run in constrained environments, we don't want to include functionality users may not want or need.
  • Someday we may provide other monitoring plugins. This allows the user to choose which without having various flavors of the NATS server/streaming server images or adding complexity and weight to the NATS server images.
  • Keeping this separate allows the NATS Prometheus exporter image to detect and report a problem with server, without being tied to the server process or container itself. So if there is a hang, the server process dies, etc, the exporter image can accurately reflect that, as well as someday allow us to track additional statistics like # of times restarted/reloaded.

That being said, we're happy to discuss this further and if you feel there there is a better way, we would like your insight on other approaches and their advantages.

Thanks,
Colin

@tianon
Copy link
Member

tianon commented Jul 18, 2018

Oh right, I'm not at all suggesting it be included in the nats image itself, but rather as tags of the nats repository, sorry for not being more clear there.

@yosifkit
Copy link
Member

yosifkit commented Feb 4, 2019

Closing given absence of progress. We apologize for the delays with our responses. 🙇‍♂️ Not to excuse our slow or missing replies, we often have to prioritize maintaining the current images over adding new images. Thanks for your contribution.

If you'd still like to propose a new image, let us know here and we'll try to help it move along. If you've since moved on to other endeavors, we wish you the best. ❤️

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

Successfully merging this pull request may close these issues.

7 participants