elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.47k stars 8.04k forks source link

[Response Ops][Task Manager] change task claiming interface to stop using observables as result #184952

Open pmuellr opened 3 weeks ago

pmuellr commented 3 weeks ago

As part of implement task claiming strategy mget #180485, you can see some fairly artificial code constructing an observable to return the results back to the caller:

export function claimAvailableTasksMget(opts: TaskClaimerOpts): Observable<ClaimOwnershipResult> {
  const taskClaimOwnership$ = new Subject<ClaimOwnershipResult>();

  claimAvailableTasksApm(opts)
    .then((result) => {
      taskClaimOwnership$.next(result);
    })
    .catch((err) => {
      taskClaimOwnership$.error(err);
    })
    .finally(() => {
      taskClaimOwnership$.complete();
    });

  return taskClaimOwnership$;
}

async function claimAvailableTasksApm(opts: TaskClaimerOpts): Promise<ClaimOwnershipResult> {
  const apmTrans = apm.startTransaction(
    TASK_MANAGER_MARK_AS_CLAIMED,
    TASK_MANAGER_TRANSACTION_TYPE
  );

  try {
    const result = await claimAvailableTasks(opts);
    apmTrans.end('success');
    return result;
  } catch (err) {
    apmTrans.end('failure');
    throw err;
  }
}

async function claimAvailableTasks(opts: TaskClaimerOpts): Promise<ClaimOwnershipResult> {
  // actual work done here!
}

The function claimAvailableTasks() is doing the real work, but we "wrap it" in an observable, as that's the current contract between task claimers and their callers.

We think we can probably remove this, to make things a little less confusing, and have it return the results directly. This is part of a general theme of removing the extensive usage of RxJs within task manager where it's not actually needed.

elasticmachine commented 3 weeks ago

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