enhancement: Cluster worker scrape timeouts/errors should be recorded

[xginn8]

If a worker is heavily loaded or otherwise pathologically performant, a cluster master may time out when scraping that worker (https://github.com/siimon/prom-client/blob/master/lib/cluster.js#L51-L61). The cluster master then returns an incomplete set of data (since some subset of workers have returned data, but not necessarily all). I think it makes sense to record an error counter for clustered worker scrapes, such that it's exposed to downstream consumers.

If this feature makes sense, I am happy to implement it! (cc @dt-rush)

[dt-rush]

Workaround for now, as @xginn8 devised:

max(scrape_duration_seconds{job="vpn/cluster"}) < bool 5 will return 1 when a scrape takes less than 5s, and 0 when it takes longer.

combining that with the query we already have, sum(vpn_online_devices{instance=~"$instance"}) * (max(scrape_duration_seconds{job="vpn/cluster"}) < bool 5) > 0 will essentially drop any "incomplete" datapoints, which we can both a) visualize and b) ignore in the larger visualization, to avoid confusion

[dt-rush]

Dropping the incomplete datapoints query-side is one thing, but dropping them entirely seems to be worth something in my view, unless (this is the only workable "solution", in the case a cluster worker times out) some kind of interpolation logic client-side were included, to "guess" the missing cluster worker value according to its previous values, or according to its siblings.

[dt-rush]

If the number of cluster workers is known, and the number of cluster workers timing out is known (presently such metric are not exported), such an averaged "guess" of the timed out worker would be possible client-side with:

metric_A * (promclient_n_cluster_workers{metric="metric_A"} / ((promclient_n_cluster_workers{metric="metric_A"} -promclient_cluster_workers_timed_out{metric="metric_A"}) > 0))

If, say, 3 out of 5 cluster workers timed out, we'd have 2/5ths of the cluster workers reporting. Scaling that up by a factor of 5/(5 - 3) approximates what we might observe if all workers reported (assumption that workers are responsible for approximately equal portions of the aggregated total). This may not be a fair assumption, but if it is, allows an approximated picture despite worker timeouts, when they occur.

Additionally, graphs could be annotated at the point these "approximations" are occurring by checking when promclient_cluster_workers_timed_out > 0, or drop the metric, as well, in the case that promclient_cluster_workers_timed_out > 0.

[zbjornson]

I'm not super keen on interpolation (rather have no data than fake data), but agree this could be handled better. I don't know what Prometheus does during a scrape if we were to switch to all-or-none agglomeration -- would that work for you guys? How long of a timeout do you need to avoid missed metrics entirely? (I've only encountered them a handful of times personally.)

[dt-rush]

For the record, this also occurs when a worker has died, not just in the case of timeouts (this is what @xginn8 meant by "timeouts / errors")

I see there to be 2 problems here:

• hard-coded timeout
• portion of aggregate metrics can be dropped without any way to detect it

[xginn8]

@zbjornson I think discarding an entire scrape due to failed worker(s) is the wrong approach, as we'd be throwing away useful data that may not be aggregated at all (from the master process). My preference would simply be to increment an error counter with some information about the missing scrape, such that it's clear to any consumers that the data is potentially incomplete. In this manner, those consumers can act on that as they wish, but the library makes no assumptions about the utility of that "partial" scrape.

In the case of a worker dying, no timeout will be sufficient since the PID will never come back.

[jdmarshall]

@xginn8 Incomplete scrapes don't harm gauges as much as they do counters and histograms. The problem is that IME most production metrics are counters and histograms, and in particular, most alerts are linked to counters and histograms. We are doing nobody any good reporting ratty data.

With 0.16, we anticipate collection time will be down about 30-50%, which will help with the timeouts. You simply do not want collection to take 5 seconds, ever, for any online service (exceptions may apply for batch processing services, possibly maybe).

Additionally #803 addresses the concern of dead workers. Short version, if you hook unhandled errors you can flush your metrics to the aggregator before terminating, and then they will be kept as long as the aggregator doesn't also crash.

That said, I'm not averse to a default metric on this.

[jdmarshall]

FYI: 0.16 will support collecting cluster metrics from the primary thread, so if we wanted to wire this up, there's already plumbing in place to get the values sent through to cluster_metrics

Prior to a few PRs ago this would not have been straightforward to do.

[nightjard]

Hey, I’d like to work on this. From the latest comments, I understand that the new primary-process metrics support should make this fairly straightforward.
Should we keep returning the partial metrics and increment a counter when a worker times out/errors? Also, should it count failed workers or failed collection attempts, and do you have a preferred metric name?

[jdmarshall]

You're welcome to work on this.

Clarification: We do not report partial metrics at present. 'sum' fields get strange artifacts and can cause production alerts to fire and so I'm loathe to add that in. Better no answers than incomplete answers for sum fields.

As an example: If traffic falls off sharply, this may indicate a routing error that the Ops team needs to investigate. Traffic falling off would be based on watching the deltas of a counter failing to meet a minimum rate of change. This could be an alert, or just an on call user noticing drops in req/s graphs, which are also calculated off of counters.

In some respects this will still happen anyway because one member of the cluster does not report any stats, but those stats will show up in the next scrape, the long-term trend lines will correct themselves, and the lack of telemetry for one machine will show up clearly in some of the charts. It's not perfect, but it's detectable.

For your final question, that's a bit tricky. Initial instinct is to use a gauge and report the number of workers that failed to reply. Since the gauge will not be updated until the tail end of the collection, we could still use a gauge and get the number of failures on the last failed collection, but that seems like something that could break in a subsequent PR, and what if there are 3 failures in a row, and the last failure only missed 1 response but the previous two failed to get telemetry from all but one worker? So a histogram might be a better metric here.

[sornambiga-7]

Hi, I'd like to work on this issue. I can implement the error counter for cluster worker scrape timeouts/errors and add the relevant tests. Please let me know if that's okay.

[jdmarshall]

#850 is already open but we are waiting for changes.