You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/09/07 14:46:03 UTC
[GitHub] [druid] kfaraz commented on a diff in pull request #13028: Improve doc and configuration of prometheus emitter
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org