bug: asynchookscopemanager's withAsync leak context

[vmarchaud]

Test case:

const scope1 = '1' as any;
const scope2 = '2' as any;

await contextManager.withAsync(scope1, async () => {
  assert.strictEqual(contextManager.active(), scope1);
  await contextManager.withAsync(scope2, async () => {
    assert.strictEqual(contextManager.active(), scope2);
    done = true;
  });
  assert.strictEqual(contextManager.active(), scope1);
});

// throw because current is '1'
assert.strictEqual(contextManager.active(), Context.ROOT_CONTEXT);

I'm starting to think that we shouldn't have gone through this road of re-implenting it ourselves. It may be worth to just bundle cls-hooked, WDYT ?
cc @open-telemetry/javascript-approvers

[dyladan]

I'm not against adding cls-hooked, but I think it only works in very new versions of node correct? I would likely prefer to add a new context manager which uses cls-hooked.

Can you please take a look at the dependency tree of cls-hooked and see what impact this would have on deployment size?

[vmarchaud]

@dyladan Nope, cls-hooked is based on async hooks like us. The only difference is that it's API is compatible with the old CLS package and it has a fallback for node < 8.

I will check if it's worth to include it as a package or just re-use its implementation. (its only 400 LOC)