Add ContextManager interface and AsyncHooks implementation #62
Conversation
vmarchaud
requested review from
danielkhan,
iredelmeier,
justindsmith and
mayurkale22
as code owners
| @@ -0,0 +1,126 @@ | |||
| /** | |||
There was a problem hiding this comment.
This file is pretty much the same as Opencensus: https://github.com/census-instrumentation/opencensus-node/blob/master/packages/opencensus-core/src/internal/cls-ah.ts, i didn't saw the point of re-implementing differently since this one is already used in production with Census.
|
|
||
| /** A key/value object for anything that can be stored in the context */ | ||
| export interface ContextStore { | ||
| [key: string]: unknown; |
There was a problem hiding this comment.
I believe the simpler is the API, the better it is. So instead of having a custom class and get/set method i choose to just expose a object there.
| * handler). | ||
| * @param fn A function to which to bind the trace context. | ||
| */ | ||
| wrapFunction<T>(fn: Func<T>): Func<T>; |
There was a problem hiding this comment.
Pretty much the most important function to propagate context, i re-used the signature of the Census implementation.
| * There are specific case where the context is not propagated by specific | ||
| * code (ex: user space queueing) so expect it to be null sometimes. | ||
| */ | ||
| getCurrentContext(): ContextStore | null; |
There was a problem hiding this comment.
We should also choose if we return a empty object or null when there are no active context. I would prefer to use null there since users would not trust it's content.
|
Thanks for the PR. I would suggest to split this huge PR in a few smaller PRs.
thoughts? |
|
Sure no problem @mayurkale22 |
As defined in #46, i've not use types already available in the
corepackage since the goal is for those package to be standalone.I didn't really tested the implementation (specially the async hooks one) since the API isn't finished but i will add more test in a separate PR.
I mostly based the API on the Opencensus one, i will explain the choice i made for each function so we have a starting point for the discussion.