[Telemetry] The local strategies should call areAllCollectorsReady

[afharo]

We've noticed that the local telemetry strategies should call the areAllCollectorsReady method before fetching the data.

We should validate all collectors are ready before pushing anything.

⚠️ Mind race conditions: X-Pack might get false but OSS to get true (when they are iterating over the same collectors because of the way UsageCollection is done)

[elasticmachine]

Pinging @elastic/kibana-telemetry (Team:KibanaTelemetry)

[TinaHeiligers]

Good catch!

[afharo]

All praise to @rudolf ⭐

[TinaHeiligers]

@afharo is #79156 what you had in mind? It's very rough and doesn't handle the "race conditions" you speak of...
We need to implement the check when we send the usage data.

[Bamieh]

I've debugged this and I dont think it is valid. areAllCollectorsReady is only called by the monitoring bulk uploader which sends the data to the monitoring indices. It will keep retrying until they are.

When telemetry collection is called to fetch usage it will fetch whatever is available at that time to send it. We dont want to miss sending the whole usage because of some collectors not being ready (or even all of them).

We cannot also hold from sending until they are ready because we are bound by request timeouts.

We can wait for collectors to be ready from the server side sender before sending the usage from the fetcher. I'll submit a PR for it, although this is an enhancement not a bug at this point.

[afharo]

I agree with @Bamieh. It’s an enhancement, not a bug (updated the labels). And by holding and waiting for all collectors to be ready, we risk a collector will always return isReady: false and block any telemetry collection.

On the flip side, we might suffer from race conditions where we attempt to send telemetry and miss some fields that may take a bit longer to be ready (they'll likely be on the next report 24h later).

Maybe the solution to this issue is a detailed reason of why we won't fix it 🙂
Or an improvement to only run the fetch methods for those reporting isReady: true.

[afharo]

And by holding and waiting for all collectors to be ready, we risk a collector will always return isReady: false and block any telemetry collection.

We've had a discussion about this and the method areAllCollectorsReady already takes care of this use case. So the changes in #79398 make sense to me! Thank you @Bamieh!

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)