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

SOLR-15954: Move the Prometheus Exporter from modules #564

Merged
merged 9 commits into from Jan 26, 2022

Conversation

HoustonPutman
Copy link
Contributor

Copy link
Contributor

@janhoy janhoy left a comment

Choose a reason for hiding this comment

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

I browsed the PR, built the branch, started Solr and then the exporter. Think this is ok.

I was going to builda Docker image from the binary distro to test the exporter from docker, but looking in docker/ folder in binary distro, there is no description on how to build the image. No description in main README.md either. I think there is one in the ref.guide, but should there not be a docker/README.md with a sample command?

Another nitpick not related to the move: Starting the exporter, it logs

INFO  - 2022-01-24 21:34:21.636; org.apache.solr.prometheus.exporter.SolrExporter; Starting Solr Prometheus Exporting

Nowhere does it echo the port it was started on in the default case. Would be nice if the log said Started the exporter on port NNNN so folks trying it the first time will at least easily find it in browser.

PS: The link to ref-guide in solr-exporter/README.md is broken link. Should be https://solr.apache.org/guide/8_11/monitoring-solr-with-prometheus-and-grafana.html, not https://solr.apache.org/guide/monitoring-with-prometheus-and-grafana.html as it is now.

@HoustonPutman
Copy link
Contributor Author

I was going to builda Docker image from the binary distro to test the exporter from docker, but looking in docker/ folder in binary distro, there is no description on how to build the image. No description in main README.md either. I think there is one in the ref.guide, but should there not be a docker/README.md with a sample command?

I think this is a good point, but kind of separate from this ticket. I'll create a JIRA and do that separately.

Nowhere does it echo the port it was started on in the default case. Would be nice if the log said Started the exporter on port NNNN so folks trying it the first time will at least easily find it in browser.

Again good point, but will fix separately.

PS: The link to ref-guide in solr-exporter/README.md is broken link. Should be https://solr.apache.org/guide/8_11/monitoring-solr-with-prometheus-and-grafana.html, not https://solr.apache.org/guide/monitoring-with-prometheus-and-grafana.html as it is now.

This is the name of the page starting with 9.0, so I agree that it isn't great that the link is broken until 9.0 is released. However, given that it will be released relatively soon, I think we can keep the future-proof URL.

@janhoy
Copy link
Contributor

janhoy commented Jan 25, 2022

Great. Did not check the new refguide link, makes sense.

@HoustonPutman HoustonPutman merged commit 2a3e9ca into apache:main Jan 26, 2022
@HoustonPutman HoustonPutman deleted the move-prom-exp branch January 26, 2022 16:07
HoustonPutman added a commit that referenced this pull request Jan 26, 2022
HoustonPutman added a commit that referenced this pull request Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants