Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request - add generic request context object #42

Closed
eugene-kim opened this issue Oct 9, 2020 · 5 comments
Closed

Feature request - add generic request context object #42

eugene-kim opened this issue Oct 9, 2020 · 5 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@eugene-kim
Copy link

Hello,

Thank you for the wonderful library. It's helped a bunch in terms of zeroing in on relevant log messages for a single request.

I have a feature request and was wondering what your thoughts might be around opening up cls-rtracer to have a generic request context object. Rather than simply providing an id that exists throughout the lifetime of the request, there's also a request context object that one can access and put in whatever they want.

Example use case:

Applications that have authorized actions can store the authorized account's id in the request context once authorized. This allows apps to always log an account_id in a single place when relevant, making tracing all of an account's actions very convenient.

@puzpuzpuz
Copy link
Owner

Thanks for the positive feedback.

I think requestIdFactory option added in #40 could be used to achieve what you have described. This feature allows to use an arbitrary JS value, including objects, as the request id. So, you could write a function that would create objects with a context property along with a UUID v1 id (of course, any other id generation algorithm could be used).

@eugene-kim
Copy link
Author

@puzpuzpuz while requestIdFactory looks as if it can be used to create a generic request id type, I don't think it completely addresses the use case described. The requestId is something that should be instantiated as soon as possible during the request's lifecycle. With something like authenticated requests, the authentication may not occur until a later point in the request's lifecycle, so having cls-tracer expose something like setContext or addContext to update the context object when appropriate would provide the desired flexibility.

@puzpuzpuz
Copy link
Owner

Nothing prevents the context object to be empty initially and then the later middlewares/plugins/handlers could be simply reading and updating the context when necessary. I know that it sounds like a workaround, but the goal of cls-rtracer is to solve the problem of request tracing, so that's why its API is kept minimal and focused on the problem.

In general, what you're describing is a general-purpose CLS API, which is what cls-rtracer builds upon. It may be a good option to use AsyncLocalStorage directly in your app, rather than dealing with the requestIdFactory workaround. Performance-wise, there should be no noticeable impact in having multiple AsyncLocalStorage instances vs having a single one. This way you'll get an API close to what was described in this issue.

Does it make sense to go with one of those options in your case?

@puzpuzpuz puzpuzpuz added enhancement New feature or request question Further information is requested labels Oct 12, 2020
@eugene-kim
Copy link
Author

Hi @puzpuzpuz understood. In that case I may have to look into using AsyncLocalStorage myself. Thanks for the response!

@puzpuzpuz
Copy link
Owner

@eugene-kim please let me know if you run into any issues or unclear documentation for AsyncLocalStorage, as the API is relatively new. Core team would appreciate such feedback from developers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants