kfaraz commented on code in PR #17689:
URL: https://github.com/apache/druid/pull/17689#discussion_r1938187399
##########
docs/development/extensions-contrib/prometheus.md:
##########
@@ -90,6 +90,7 @@ Prometheus metric path is organized using the following
schema:
"dimensions" : <dimension list>,
"type" : <timer|counter|gauge>,
"conversionFactor": <conversionFactor>,
+ "histogramBuckets": <bucket values>,
Review Comment:
```suggestion
"histogramBuckets": <array of bucket values for timer metric>,
```
##########
extensions-contrib/prometheus-emitter/src/main/resources/defaultMetrics.json:
##########
@@ -1,5 +1,5 @@
{
- "query/time" : { "dimensions" : ["dataSource", "type"], "type" : "timer",
"conversionFactor": 1000.0, "help": "Seconds taken to complete a query."},
+ "query/time" : { "dimensions" : ["dataSource", "type"], "type" : "timer",
"conversionFactor": 1000.0, "histogramBuckets": [0.1, 0.25, 0.5, 0.75, 1.0,
2.5, 5.0, 7.5, 10.0, 30.0, 60.0, 120.0, 300.0], "help": "Seconds taken to
complete a query."},
Review Comment:
Why override this? Isn't this already the default value?
##########
website/.spelling:
##########
@@ -360,6 +360,7 @@ hashcode
hashtable
high-QPS
historicals
+histogramBuckets
Review Comment:
I don't think this should be needed since this word occurs within code
blocks in the docs.
##########
docs/development/extensions-contrib/prometheus.md:
##########
@@ -40,7 +40,7 @@ All the configuration parameters for the Prometheus emitter
are under `druid.emi
| `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.dimensionMapPath` | JSON file defining the
Prometheus metric type, desired dimensions, conversionFactor, histogram buckets
and help text for every Druid metric.
| no |
Default mapping provided. See below. |
Review Comment:
Please also add a line in the `### Metric mapping` section about the default
values of the histogram buckets.
--
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]
