kfaraz commented on code in PR #13028:
URL: https://github.com/apache/druid/pull/13028#discussion_r964920670
##########
docs/development/extensions-contrib/prometheus.md:
##########
@@ -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.
-`query/time" : { "dimensions" : ["dataSource", "type"], "conversionFactor":
1000.0, "type" : "timer", "help": "Seconds taken to complete a query."}`
+
+```json
+<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.
-e.g.
-`"coordinator-segment/count" : { "dimensions" : ["dataSource"], "type" :
"gauge" },
- "historical-segment/count" : { "dimensions" : ["dataSource", "tier",
"priority"], "type" : "gauge" }`
+the service name. For example:
+
+```json
+"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.
Review Comment:
Good one 😄 !
##########
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|
-|--------|-----------|---------|-------|
-|`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.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.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.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|
+| property | description
| required? | default
|
+|-----------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------|--------------------------------------|
+| `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.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.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.pushGatewayAddress` | Pushgateway address.
Required if using `pushgateway` strategy.
| no | none
|
+
+### Override properties for Peon Tasks
+
+Since peon tasks are created dynamically by MiddleManagers, it's not able to
use `exporter` strategy for these tasks to let prometheus read metrics from
fixed addresses.
Review Comment:
```suggestion
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.
```
Tried to simplify the language a bit.
##########
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|
-|--------|-----------|---------|-------|
-|`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.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.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.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|
+| property | description
| required? | default
|
+|-----------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------|--------------------------------------|
+| `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.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.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.pushGatewayAddress` | Pushgateway address.
Required if using `pushgateway` strategy.
| no | none
|
+
+### Override properties for Peon Tasks
+
+Since peon tasks are created dynamically by MiddleManagers, it's not able to
use `exporter` strategy for these tasks to let prometheus read metrics from
fixed addresses.
+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 above
configurations need to be overridden in the MiddleManager's configuration file
for peon tasks as shown below.
Review Comment:
```suggestion
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.
```
##########
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|
-|--------|-----------|---------|-------|
-|`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.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.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.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|
+| property | description
| required? | default
|
Review Comment:
Style:
I think I prefer the original formatting style. Having the columns aligned
is good, but it becomes difficult to read once the text begins to wrap. Content
written in the older style can be easily read through without having to
consciously skip the whitespaces.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
