[Alerting] Improving health status check

[ymao1]

Resolves #93062

Summary

Made several changes to the alerting health check:

1. Added a share() to the combineLatest operator that combines the the core status with the alerting health status. I was seeing duplicate observable streams being created from getHealthStatusStream (88!), each firing at a 5 minute interval. Maybe it's possible that this many concurrent get requests to the task manager saved object was contributing to the 503 socket hangup errors?

2. Moved catchError from the top level interval observable to within the switchMap. When catchError was at the top level, it would handle the error and complete the stream, which means once the alerting status became unavailable, it would stop polling for updated status and remain in an error state.

3. Added a retryWhen operator which retries getting the status a few times before propagating the error status.

Checklist

Delete any items that are not applicable to this PR.

• Unit or functional tests were updated or added to match the most common scenarios

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[ymao1]

@elasticmachine merge upstream

[YulNaumenko]

LGTM! Awesome improvements!

[gmmorris]

Could we add an assertion that mockTaskManager.get was actually called MAX_RETRY_ATTEMPTS times?

Otherwise, in theory, this test. won't catch anything if the stream never emits any values as all expects are inside the subscription handler.
(this is from experience... I've missed a bug before because I made the exact same mistake) :)

[gmmorris]

Is it worth using getHealthStatusStream directly here with the interval being an argument (we can use default value in the implementation)?

That way the unit tests ensure the composition is behaving as expected.
Just incase someone changes the switchMap in getHealthStatusStream in the future to something that behaved differently... 🤔

[gmmorris]

LGTM from a logical perspective.
I haven't been able to test this locally because I'm not sure how to cause the failure case.

Any advice on whether that can be tested? 🤔

[ymao1]

Any advice on whether that can be tested? 🤔

I did something like this in getLatestTaskState:

let getLatestTaskState = 0;
const shouldThrowError = [false, false, true, true, true, false, false...];
async function getLatestTaskState(taskManager: TaskManagerStartContract) {
  if (getLatestTaskState < shouldThrowError.length && shouldThrowError[getLatestTaskState]) {
    throw new Error();
  }
  getLatestTaskState++;
  
  try {
    const result = await taskManager.get(HEALTH_TASK_ID);
  }

}

Also shortened the HEALTH_STATUS_INTERVAL and RETRY_DELAY so it wouldn't take so long to run.

I changed the sequence in shouldThrowError to test out successful retries and maxing out retry attempts and made sure that even after a maxed out retry that returned unavailable, the interval continued polling.

[pmuellr]

I was seeing duplicate observable streams being created from getHealthStatusStream (88!), each firing at a 5 minute interval. Maybe it's possible that this many concurrent get requests to the task manager saved object was contributing to the 503 socket hangup errors?

Aha!

I was really curious how we were seeing 50x responses, since I thought these were auto-retried and such. It seems like it would make sense that if we had that many requests running, at least one of them could end up failing on each retry, and finally give up and return the 50x as the final reply.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: ab58c7f
• Storybooks Preview

Metrics [docs]

✅ unchanged

History

• 💚 Build #110509 succeeded b142573
• 💚 Build #110450 succeeded efff617

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

cc @ymao1

[kibanamachine]

💚 Backport successful

✅ 7.x / #93639

Successful backport PRs will be merged automatically after passing CI.