open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.65k stars 765 forks source link

api.context.with() is lacking parameter for parameters to specified function #1870

Closed jonaskello closed 3 years ago

jonaskello commented 3 years ago

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

0.15.0

What version of Node are you using?

v14.15.4

Please provide the code you used to setup the OpenTelemetry SDK

What did you do?

Checked signature of api.context.with() in typings:

with<T extends (...args: unknown[]) => ReturnType<T>>(context: Context, fn: T): ReturnType<T>;

It takes two parameters, context and fn. The type of fn implies that fn can accept parameters but there is no way to provide them to with()

What did you expect to see?

I expected the signature to take a third optional rest parameter with params for fn. The function it ends up calling is asyncLocalStorage.run and that takes a third param like this asyncLocalStorage.run(store, callback[, ...args])

I think either a third param could be added or the type of fn should be changed to not accept parameters?

Flarna commented 3 years ago

I think we should allow arguments like in asyncLocalStorage.run(). I had this already in mind but haven't found the time to actually do it.

dyladan commented 3 years ago

I would support this change. For now you can work around like this (I know it is not ideal):

context.with(ctx, () => myFn(...args));
marcbachmann commented 3 years ago

it would also be nice to support passing a custom this context. There's less overhead with that, but the api would look weird.

context.with.call(someInstance, ctx, someInstance.something, arg1)