[GitHub] [lucene-solr] janhoy commented on a change in pull request #2038: SOLR-14955: Add env var options to Prometheus Export scripts.

[GitBox <gi...@apache.org>]

janhoy commented on a change in pull request #2038:
URL: https://github.com/apache/lucene-solr/pull/2038#discussion_r514156548



##########
File path: solr/solr-ref-guide/src/monitoring-solr-with-prometheus-and-grafana.adoc
##########
@@ -81,34 +81,44 @@ $ ./bin/solr-exporter -p 9854 -z localhost:2181/solr -f ./conf/solr-exporter-con
 
 === Command Line Parameters
 
-The parameters in the example start commands shown above:
+The list of available parameters for the Prometheus Exporter.
+All parameters can be provided via an environment variable, instead of through the command line.
 
 `h`, `--help`::
 Displays command line help and usage.
 
-`-p`, `--port`::
-The port where Prometheus will listen for new data. This port will be used to configure Prometheus. It can be any port not already in use on your server. The default is `9983`.
+`-p`, `--port`, `$PORT`::
+The port where Prometheus will listen for new data. This port will be used to configure Prometheus.
+It can be any port not already in use on your server. The default is `9983`.

Review comment:
       I find it puzzling that default port is the same as the default port for embedded Zookeeper, i.e. if you start `bin/solr -c` and then start exporter on same host, you'll have a port collision.

##########
File path: solr/contrib/prometheus-exporter/bin/solr-exporter
##########
@@ -135,15 +134,42 @@ if $cygwin; then
   [ -n "$REPO" ] && REPO=`cygpath --path --windows "$REPO"`
 fi
 
+# Convert Environment Variables to Command Line Options
+EXPORTER_ARGS=()
+
+if [[ -n "$CONFIG_FILE" ]]; then
+  EXPORTER_ARGS+=(-f "$CONFIG_FILE")
+fi
+
+if [[ -n "$PORT" ]]; then

Review comment:
       I like the simplicity of `PORT`, but is there a risk that such generic naming could lead to conflict, i.e. that a user hast this env set for some other app, and then the exporter will use 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org