[Response Ops][Task Manager] Only resetting TM metrics when metrics_reset_interval has passed since last reset

[ymao1]

The task manager metrics are collected in a monotonically increasing manner and they can be reset one of two ways: (1) When the metrics are collected via the HTTP API (using an external collection mechanism like metricbeat) and (2) when the configured metrics_reset_interval has passed. The second way is there as a fail-safe in case the external collection mechanism fails, however there was a bug where the metrics would always get reset when the metrics_reset_interval had passed, instead of only if the metrics_reset_interval had passed since the last collection. This bug could be causing metrics to be unnecessarily reset even if they're regularly being collected (and reset via that mechanism).

[elasticmachine]

Pinging @elastic/response-ops (Team:ResponseOps)

[pmuellr]

I instrumented the new code to see when it was calculating whether to reset or not, and saw some interesting results:

2024-05-01T16:34:01.765Z tm metrics: resetting because interval has passed
2024-05-01T16:34:01.769Z tm metrics: not resetting because interval has not passed

2024-05-01T16:34:31.765Z tm metrics: not resetting because interval has not passed
2024-05-01T16:34:31.769Z tm metrics: resetting because interval has passed

2024-05-01T16:35:01.764Z tm metrics: resetting because interval has passed
2024-05-01T16:35:01.770Z tm metrics: resetting because interval has passed

2024-05-01T16:35:31.765Z tm metrics: resetting because interval has passed
2024-05-01T16:35:31.769Z tm metrics: not resetting because interval has not passed

The two calls are I assume from the double polls (everything-but-reports, then reports). Curious that most of the time the first one resets, the second one doesn't. \o/. But sometimes not, like at 16:35:01.

I don't think this is an issue with this code, but is interesting!

[pmuellr]

LGTM

Per other comment in the PR, the "double path" through here because of the double polls pretty much works, though I guess there's a bit of a race condition there when it resets on both. I don't think that's a problem - resetting twice so close together should pretty much be a no-op, right? Or would we drop something the first one might have sent?

Beyond that, I let it run, and then did some metric?result=true calls, which I could force at various times by watching the logs. Worked fine!

[ymao1]

double polls

I'm not sure what you mean by the double polls...where did you add the log messages? Wondering if there's another bug in this code? 🙈

[ymao1]

@elasticmachine merge upstream

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: fcf7db1

Failed CI Steps

• FTR Configs #64

Test Failures

• [job] [logs] FTR Configs #64 / Action Task Params migrations "before all" hook for "7.16.0 migrates action_task_params to use references array"

Metrics [docs]

✅ unchanged

History

• 💛 Build #206738 was flaky 70b962e
• 💔 Build #206730 failed bbf65aa

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ymao1

[ymao1]

Did some more testing. The double logs are due to the two different aggregators we create. Each one maintains its own lastResetTime. There are times when the observable interval events occur at slightly less than the interval (for a 60,000 millisecond interval, sometimes the observable will fire at 59,994 milliseconds) which is why without metrics collection, sometimes it won't reset the values but worse case, it'll go 2x the metrics reset interval without resetting.