-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: enable metrics collection on docker compose example vec-409 #73
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but we need a way to cover all the other docker-compose files. Specifically prism, quote search, and 6.4.
@hev I added metrics collection to the other compose files and centralized its config in the .internal directory. I tested these changes locally, each worked and metrics were visible on prometheus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didnt test all the variations but approving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to do some kind of disclosure here. Something like the following.
This repo contains examples that collects basic usage data about the product including network information. You can opt out of this by disabling Prometheus in any of the install instructions.
I added disclosures to the readme files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. Looks good.
…earch' in docker-compose files (#80)
Tested locally after Joe reverted the service rename. docker, quote search, and prism search compose files all worked for me. |
I'm going to do a little more testing tomorrow to make sure metrics are showing up in our monitoring stack still, then will merge this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's hold off on merging this. Waiting on feedback for leadership.
No longer moving forward on this. |
No description provided.