Improve doc and configuration of prometheus emitter (#13028)

* Improve doc and validation

* Add configuration for peon tasks

* Update doc

* Update test case

* Fix typo

* Update docs/development/extensions-contrib/prometheus.md

Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

* Update docs/development/extensions-contrib/prometheus.md

Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

docs/development/extensions-contrib/prometheus.md

@@ -28,22 +28,38 @@ To use this Apache Druid extension, [include](../../development/extensions.md#lo
 ## Introduction
 
 This extension exposes [Druid metrics](https://druid.apache.org/docs/latest/operations/metrics.html) for collection by a Prometheus server (https://prometheus.io/).
+
 Emitter is enabled by setting `druid.emitter=prometheus` [configs](https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics) or include `prometheus` in the composing emitter list. 
 
 ## Configuration
 
 All the configuration parameters for the Prometheus emitter are under `druid.emitter.prometheus`.
 
-|property|description|required?|default|
+| property                                      | description                                                                                                                                                                                                                            | required? | default                              |
-|--------|-----------|---------|-------|
+|-----------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------|--------------------------------------|
-|`druid.emitter.prometheus.strategy`|The strategy to expose prometheus metrics. Default strategy `exporter` would expose metrics for scraping purpose. Only peon task (short-lived jobs) need to use `pushgateway` strategy.|yes|exporter|
+| `druid.emitter.prometheus.strategy`           | The strategy to expose prometheus metrics. <br/>Should be one of `exporter` and `pushgateway`. Default strategy `exporter` would expose metrics for scraping purpose. Peon tasks (short-lived jobs) should use `pushgateway` strategy. | yes       | exporter                             |
-|`druid.emitter.prometheus.port`|The port on which to expose the prometheus HTTPServer. Required if using exporter strategy.|no|none|
+| `druid.emitter.prometheus.port`               | The port on which to expose the prometheus HTTPServer. Required if using `exporter` strategy.                                                                                                                                          | no        | none                                 |
-|`druid.emitter.prometheus.namespace`|Optional metric namespace. Must match the regex `[a-zA-Z_:][a-zA-Z0-9_:]*`|no|"druid"|
+| `druid.emitter.prometheus.namespace`          | Optional metric namespace. Must match the regex `[a-zA-Z_:][a-zA-Z0-9_:]*`                                                                                                                                                             | no        | druid                                |
-|`druid.emitter.prometheus.dimensionMapPath`|JSON file defining the Prometheus metric type, desired dimensions, help text, and conversionFactor for every Druid metric.|no|Default mapping provided. See below.|
+| `druid.emitter.prometheus.dimensionMapPath`   | JSON file defining the Prometheus metric type, desired dimensions, help text, and conversionFactor for every Druid metric.                                                                                                             | no        | Default mapping provided. See below. |
-|`druid.emitter.prometheus.addHostAsLabel`|Flag to include the hostname as a prometheus label.|no|false|
+| `druid.emitter.prometheus.addHostAsLabel`     | Flag to include the hostname as a prometheus label.                                                                                                                                                                                    | no        | false                                |
-|`druid.emitter.prometheus.addServiceAsLabel`|Flag to include the druid service name (e.g. `druid/broker`, `druid/coordinator`, etc.) as a prometheus label.|no|false|
+| `druid.emitter.prometheus.addServiceAsLabel`  | Flag to include the druid service name (e.g. `druid/broker`, `druid/coordinator`, etc.) as a prometheus label.                                                                                                                         | no        | false                                |
-|`druid.emitter.prometheus.pushGatewayAddress`|Pushgateway address. Required if using Pushgateway strategy|no|none|
+| `druid.emitter.prometheus.pushGatewayAddress` | Pushgateway address. Required if using `pushgateway` strategy.                                                                                                                                                                         | no        | none                                 |
 
+### Override properties for Peon Tasks
+
+Peon tasks are created dynamically by middle managers and have dynamic host and port addresses. Since the `exporter` strategy allows Prometheus to read only from a fixed address, it cannot be used for peon tasks.
+So, these tasks need to be configured to use `pushgateway` strategy to push metrics from Druid to prometheus gateway.
+
+If this emitter is configured to use `exporter` strategy globally, some of the above configurations need to be overridden in the middle manager so that spawned peon tasks can still use the `pushgateway` strategy.
+
+```
+#
+# Override global prometheus emitter configuration for peon tasks to use `pushgateway` strategy.
+# Other configurations can also be overridden by adding `druid.indexer.fork.property.` prefix to above configuration properties.
+# 
+druid.indexer.fork.property.druid.emitter.prometheus.strategy=pushgateway
+druid.indexer.fork.property.druid.emitter.prometheus.pushGatewayAddress=http://<push-gateway-address>
+```
 
 ### Metric names
 
@@ -58,15 +74,34 @@ be provided as a JSON file.  Additionally, this mapping specifies which dimensio
 histogram timers to use Seconds as the base unit.  Timers which do not use seconds as a base unit can use the `conversionFactor` to set
 the base time unit. If the user does not specify their own JSON file, a default mapping is used.  All
 metrics are expected to be mapped. Metrics which are not mapped will not be tracked.
+
 Prometheus metric path is organized using the following schema:
-`<druid metric name> : { "dimensions" : <dimension list>, "type" : <timer|counter|gauge>, conversionFactor: <conversionFactor>, "help" : <help text>,}`
+
-e.g.
+```json
-`query/time" : { "dimensions" : ["dataSource", "type"], "conversionFactor": 1000.0, "type" : "timer", "help": "Seconds taken to complete a query."}`
+<druid metric name> : { 
+  "dimensions" : <dimension list>, 
+  "type" : <timer|counter|gauge>, 
+  "conversionFactor": <conversionFactor>, 
+  "help" : <help text>
+}
+```
+
+For example:
+```json
+"query/time" : { 
+  "dimensions" : ["dataSource", "type"],
+  "type" : "timer",
+  "conversionFactor": 1000.0,
+  "help": "Seconds taken to complete a query."
+}
+```
 
 For metrics which are emitted from multiple services with different dimensions, the metric name is prefixed with
-the service name.
+the service name. For example:
-e.g.
+
-`"coordinator-segment/count" : { "dimensions" : ["dataSource"], "type" : "gauge" },
+```json
- "historical-segment/count" : { "dimensions" : ["dataSource", "tier", "priority"], "type" : "gauge" }`
+"coordinator-segment/count" : { "dimensions" : ["dataSource"], "type" : "gauge" },
+"historical-segment/count" : { "dimensions" : ["dataSource", "tier", "priority"], "type" : "gauge" }
+```
  
-For most use-cases, the default mapping is sufficient.
+For most use cases, the default mapping is sufficient.

extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitterConfig.java

@@ -70,15 +70,16 @@ public class PrometheusEmitterConfig
       @JsonProperty("addServiceAsLabel") boolean addServiceAsLabel
   )
   {
-
     this.strategy = strategy != null ? strategy : Strategy.exporter;
     this.namespace = namespace != null ? namespace : "druid";
     Preconditions.checkArgument(PATTERN.matcher(this.namespace).matches(), "Invalid namespace " + this.namespace);
+    if (strategy == Strategy.exporter) {
+      Preconditions.checkArgument(port != null, "For `exporter` strategy, port must be specified.");
+    } else if (strategy == Strategy.pushgateway) {
+      Preconditions.checkArgument(pushGatewayAddress != null, "For `pushgateway` strategy, pushGatewayAddress must be specified.");
+    }
     this.dimensionMapPath = dimensionMapPath;
     this.port = port;
-    if (this.strategy == Strategy.pushgateway) {
-      Preconditions.checkNotNull(pushGatewayAddress, "Invalid pushGateway address");
-    }
     this.pushGatewayAddress = pushGatewayAddress;
     this.addHostAsLabel = addHostAsLabel;
     this.addServiceAsLabel = addServiceAsLabel;

extensions-contrib/prometheus-emitter/src/test/java/org/apache/druid/emitter/prometheus/PrometheusEmitterTest.java

@@ -137,7 +137,30 @@ public class PrometheusEmitterTest
   @Test
   public void testEmitterConfigCreationWithNullAsAddress()
   {
-    Assert.assertThrows(NullPointerException.class, () -> new PrometheusEmitterConfig(PrometheusEmitterConfig.Strategy.pushgateway, "namespace4", null, 0, null, true, true));
+    // pushGatewayAddress can be null if it's exporter mode
+    new PrometheusEmitterConfig(
+        PrometheusEmitterConfig.Strategy.exporter,
+        "namespace5",
+        null,
+        1,
+        null,
+        true,
+        true
+    );
+
+    Assert.assertThrows(
+        "For `pushgateway` strategy, pushGatewayAddress must be specified.",
+        IllegalArgumentException.class,
+        () -> new PrometheusEmitterConfig(
+            PrometheusEmitterConfig.Strategy.pushgateway,
+            "namespace5",
+            null,
+            null,
+            null,
+            true,
+            true
+        )
+    );
   }
 
   @Test
@@ -157,4 +180,33 @@ public class PrometheusEmitterTest
     pushEmitter.start();
     Assert.assertNotNull(pushEmitter.getPushGateway());
   }
+
+  @Test
+  public void testEmitterConfig()
+  {
+    Assert.assertThrows(
+        "For `exporter` strategy, port must be specified.",
+        IllegalArgumentException.class,
+        () -> new PrometheusEmitterConfig(
+            PrometheusEmitterConfig.Strategy.exporter,
+            "namespace5",
+            null,
+            null,
+            "https://pushgateway",
+            true,
+            true
+        )
+    );
+
+    // For pushgateway strategy, port can be null
+    new PrometheusEmitterConfig(
+        PrometheusEmitterConfig.Strategy.pushgateway,
+        "namespace5",
+        null,
+        null,
+        "https://pushgateway",
+        true,
+        true
+    );
+  }
 }