You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/08/23 22:54:24 UTC
[GitHub] [solr] janhoy opened a new pull request, #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
janhoy opened a new pull request, #984:
URL: https://github.com/apache/solr/pull/984
https://issues.apache.org/jira/browse/SOLR-16340
* Added label `cluster_id` on all metrics from exporter
* Added filter (template) for `cluster_id` in grafana board
* Defaults to `base_url` or `zk_host`, but can be overridden by `--cluster-id foo` or `--id foo` command line
Lacks a test for the actual setting of `clusterId` and argument parsing, since existing junit tests do not invoke the exporter through shell.
I keep `clusterId` as a static var on `SolrExporter.java` to avoid passing it through all kinds of intermediate method calls to `SolrScraper`, but there is probably a more elegant and more testable way..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1230404601
> Should grafana board display some exporter metrics ?
Yeah probably. I haven't looked over the grafana changes here, but will hopefully have time to help with those later this week!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy commented on code in PR #984:
URL: https://github.com/apache/solr/pull/984#discussion_r953618571
##########
solr/prometheus-exporter/bin/solr-exporter.cmd:
##########
@@ -82,6 +82,7 @@ IF NOT "%SCRAPE_INTERVAL%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -s %SCRAPE_INTE
IF NOT "%NUM_THREADS%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -n %NUM_THREADS%
IF NOT "%ZK_HOST%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -z %ZK_HOST%
IF NOT "%SOLR_URL%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -b %SOLR_URL%
+IF NOT "%CLUSTER_ID%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -i %CLUSTER_ID%
Review Comment:
Is this space-safe wrt quoting?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1239060507
Yes I thought about choosing a static default, but the downside is of course that you always need to configure it in a multi-cluster setting, and I'm a fan of 0-config wherever practically possible.
So another option could perhaps be defaulting to a hash of `zkHost` or `solrUrl`, e.g. the first 10 chars of a sha hash, which should be unique enough for this purpose. You won't be able to understand what cluster a hash is for until you select it, but at least it will always be unique. I'm ok with 'main' as well if you prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy commented on code in PR #984:
URL: https://github.com/apache/solr/pull/984#discussion_r957838975
##########
solr/prometheus-exporter/bin/solr-exporter.cmd:
##########
@@ -82,6 +82,7 @@ IF NOT "%SCRAPE_INTERVAL%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -s %SCRAPE_INTE
IF NOT "%NUM_THREADS%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -n %NUM_THREADS%
IF NOT "%ZK_HOST%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -z %ZK_HOST%
IF NOT "%SOLR_URL%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -b %SOLR_URL%
+IF NOT "%CLUSTER_ID%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -i %CLUSTER_ID%
Review Comment:
```suggestion
IF NOT "%CLUSTER_ID%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -i "%CLUSTER_ID%"
```
Think normal quoting will do...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1239816724
Ah, that was why the Docker test failed. Nice to have end-to-end tests this way.
I’ll proceed with merge tomorrow..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1238170410
> Looking forward to final reviews.
Going to spin this up today with a few clusters and poke around!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1238533794
Ok the dashboard looks good! Just a few changes from my end, mainly that "Shard Leaders", "Replica State History" and "QPS (Collection)" Did not specify the cluster_id, and "QPS (Collection)" did not group by it either. I will push a change with this, but the only problem is the default cluster_id. If it's a long zk-connection-string, then this will probably become problematic and make the UI somewhat unusable. (since the cluster_id will be extremely long).
Maybe instead we use a default such as "main", and require users to override it? I get the convenience of using a zk-connection-string, since that is by-default a 1-1 mapping with cluster_id, but maybe we require users to provide the id?
Would appreciate your opinion here.
Also sidenote, in the future maybe we can change this to a graph for multiple clusters? (Not in this PR though)
<img width="991" alt="Screen Shot 2022-09-06 at 2 55 27 PM" src="https://user-images.githubusercontent.com/3376422/188716277-c7b1654d-402e-48d6-8203-239557c06a25.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1232835455
Committed quoting of ClusterID in windows script, and resolved CHANGES conflict. Looking forward to final reviews.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1239444219
> So another option could perhaps be defaulting to a hash of zkHost or solrUrl, e.g. the first 10 chars of a sha hash
I love it.
I fixed a small error, but I think we should be good to go now!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy merged pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy merged PR #984:
URL: https://github.com/apache/solr/pull/984
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1227665586
@janhoy I added a PR that removed the use of the static variable, and also added zk_host and base_url labels to the scrapingErrors metric.
Feel free to revert if you don't like it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
janhoy commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1229164467
Nice improvement. Good point about labeling exporter’s own metrics! Should grafana board display some exporter metrics ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a diff in pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on code in PR #984:
URL: https://github.com/apache/solr/pull/984#discussion_r954295709
##########
solr/prometheus-exporter/bin/solr-exporter.cmd:
##########
@@ -82,6 +82,7 @@ IF NOT "%SCRAPE_INTERVAL%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -s %SCRAPE_INTE
IF NOT "%NUM_THREADS%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -n %NUM_THREADS%
IF NOT "%ZK_HOST%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -z %ZK_HOST%
IF NOT "%SOLR_URL%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -b %SOLR_URL%
+IF NOT "%CLUSTER_ID%"=="" set EXPORTER_ARGS=%EXPORTER_ARGS% -i %CLUSTER_ID%
Review Comment:
I'm not really sure here... We would probably need someone with better windows knowledge...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #984: SOLR-16340 Provide a cluster_id label for all metrics exposed by the Solr Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #984:
URL: https://github.com/apache/solr/pull/984#issuecomment-1238603018
Here is an example of using zkConnectionString as cluster_id:
<img width="1967" alt="Screen Shot 2022-09-06 at 4 11 00 PM" src="https://user-images.githubusercontent.com/3376422/188729137-3f9f7267-7a51-4b91-9cf8-fbc816d27dbd.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org