Quick pass on the writing exporters docs #935
Conversation
|
Also, where do folks stand with regard to the Oxford comma? |
matthiasr
left a comment
matthiasr
left a comment
There was a problem hiding this comment.
Added some comments. I'm not happy about the uneven line lengths now – I'd prefer to stay in line with how the document already is. We can un-break lines separately.
I'm not passionate about the oxford comma but found it causes less bike shedding to just add it everywhere.
| continuously with new versions, if you try to get things perfect then you’ve | ||
| signed yourself up for a lot of ongoing work. The [mysql | ||
| exporter](https://github.com/prometheus/mysqld_exporter) is on this end of the | ||
| If, on the other hand, the system has hundreds of metrics that change |
There was a problem hiding this comment.
I'd rephrase this a little more to get rid of the double if, like:
On the other hand, if you try to get things perfect when the system has hundreds of metrics that change frequently with new versions, then you’ve signed yourself up for a lot of ongoing work.
There was a problem hiding this comment.
Yes - thanks much nicer.
| may be too granular and expensive on large setups (e.g. the haproxy exporter | ||
| allows filtering of per-server stats). Similarly there may be expensive metrics | ||
| that are disabled by default. | ||
| may be too granular and expensive on large setups, for example the [HAProxy exporter](https://github.com/prometheus/haproxy_exporter) allows filtering of per-server stats. Similarly there may be expensive metrics that are disabled by default. |
There was a problem hiding this comment.
The line breaks are very inconsistent now.
There was a problem hiding this comment.
I'm going to fix the whole file.
| slaves you wanted to run some business queries against the data to then export. | ||
| Having an exporter that uses your usual load balancing approach to talk to one | ||
| slave is the sanest approach. | ||
| The second exception is where you’re pulling some stats out of a random instance of a system and don’t care which one you’re talking to. Consider a set of MySQL slaves you wanted to run some business queries against the data to then export. Having an exporter that uses your usual load balancing approach to talk to one slave is the sanest approach. |
There was a problem hiding this comment.
While we're at it, WDYT about fixing slavery metaphors here? I think "replica" is a perfectly self-explanatory alternative.
There was a problem hiding this comment.
Awesome - wasn't sure - some projects are receptive to this - others it's a 500 comment storm. Will fix. I already fixed the Mesos one but the MySQL is much more ... sticky.
There was a problem hiding this comment.
In this case replica is actually more accurate.
| Firstly, when do you expire metrics? Collected and things talking to Graphite | ||
| both export regularly, and when they stop we want to stop exposing the metrics. | ||
| Collected includes an expiry time so we use that, Graphite doesn’t so it’s a | ||
| Collected includes an expiry time so we use that, Graphite doesn’t so it is a |
There was a problem hiding this comment.
I think this should be Collectd?
|
|
||
| The seconds is to have an `myexporter_up` (e.g. `haproxy_up`) variable that’s | ||
| 0/1 depending on whether the scrape worked. | ||
| The seconds is to have an `myexporter_up`, e.g. `haproxy_up`, variable that has a value of 0 or 1 depending on whether the scrape worked. |
|
|
||
| Metrics must use base units (e.g. seconds, bytes) and leave converting them to | ||
| something more readable to the graphing software. No matter what units you end | ||
| something more readable to upstream tools. No matter what units you end |
There was a problem hiding this comment.
I meant graphing software, this is a frontend concern.
|
|
||
| Metric names should not include the labels that they’re exported with (e.g. | ||
| `by_type`) as that won’t make sense if the label is aggregated away. | ||
| Metric names should not include the labels that they’re exported with, e.g. |
There was a problem hiding this comment.
I don't think this is as clear
| so sometimes it’s best to leave them as-is. | ||
|
|
||
| Exposed metrics should not contain colons, these are for users to use when | ||
| Exposed metrics should not contain colons, these are reserved for use when |
There was a problem hiding this comment.
It needs to be clear that these are for end-user use. "aggregating" has a few different meanings depending on who you're talking to.
| way to expose this is as one metric for total requests and another metric for | ||
| failed requests. This makes it easy to calculate the failure ratio. Do not use | ||
| one metric with a failed/success label. Similarly with hit/miss for caches, | ||
| one metric with a failed or success label. Similarly with hit/miss for caches, |
There was a problem hiding this comment.
Be consistent between 'or' and '/'
| automatically processing a set of metrics), use `UNTYPED` in that case. In | ||
| general `UNTYPED` is a safe default. | ||
| Often it won’t be obvious what the type of metric is, especially if | ||
| you’re automatically processing a set of metrics, use `UNTYPED` in that |
There was a problem hiding this comment.
I think end the sentence at "metrics" might be a little clearer.
There was a problem hiding this comment.
Yeah. It's a bit repetitive.
| [StatsD](https://github.com/prometheus/statsd_exporter) exporters also | ||
| requiring configuration to extract labels. | ||
|
|
||
| Providing a configuration that produces some output automatically and a |
There was a problem hiding this comment.
If configuration is provided then it's not automatic
|
|
||
| Providing a configuration that produces some output automatically and a | ||
| selection of example configurations for transformation, if required, is | ||
| advised. Here are some guidelines for writing or adapting custom |
There was a problem hiding this comment.
Last sentence here doesn't fit.
| within a metrics should (almost) always have the same unit (consider if fan | ||
| speeds were mixed in with the voltages, and you had no way to automatically | ||
| separate them). | ||
| Read or write and send or receive are best as separate metrics, rather than as |
There was a problem hiding this comment.
Read and write is a set here, so 'or' doesn't make sense.
| latter breaks sum and is quite difficult to work with. Some client | ||
| libraries, for example Go, will actively try to stop you doing the | ||
| latter in a custom collector, and all client libraries should stop you | ||
| from doing the former with direct instrumentation. Never do either of |
| libraries, for example Go, will actively try to stop you doing the | ||
| latter in a custom collector, and all client libraries should stop you | ||
| from doing the former with direct instrumentation. Never do either of | ||
| former in a custom collector, and all client libraries should stop you |
There was a problem hiding this comment.
They both should be "latter". There's no way we can really stop the former.
| YAML is the standard Prometheus configuration format, all configuration | ||
| should use YAML by default. | ||
|
|
||
| Here are some guidelines for writing or adapting custom exporters. |
There was a problem hiding this comment.
This sentence should be removed, this is not an overview section.
|
Thanks! |
Signed-off-by: rleungx <rleungx@gmail.com>
3 participants
Emphasis on clarity, consistency, and tone.