Making it possible to add custom request properties to CLS namespace.

[marciopd]

Hi,

It would be awesome to add custom request properties into the CLS namespace, besides the request ID header.

This PR makes it possible by adding a new option to the middleware, a custom namespace properties builder function. This function receives the current request and CLS namespace variable so that client code can add any needed new properties based on the request.

If you like the idea, please feel free to make any changes in the PR.

I'm looking forward to use this new feature, otherwise I'll need to implement again 🙈 .

Best regards!

[aigoncharov]

IMHO, it's better to keep things simple and have a library do one thing, but do it well. I would suggest to create and apply another middleware together with cls-rtracer to add any other arbitrary stuff to the namespace.

[puzpuzpuz]

@marciopd thanks for your contribution. I see why you need it and understand your use case. Let me think a bit about this feature and whether it fits the cls-rtracer concept.

[puzpuzpuz]

@marciopd I've taken a look at the changes and would like to suggest you another approach, namely to extend the concept of id and make id generation customizable.

The main blocker for me is that customNamespacePropertiesBuilder exposes cls-hooked API. Eventually, I'd like to migrate cls-rtracer to either my own CLS API (https://github.com/nodejs/node/pull/31016) or the upcoming standard CLS API (https://github.com/nodejs/node/pull/26540) and that's why I don't want to expose the underlying library.

So, I propose to do the following changes in options:

useHeader = false, // this option works only when idGenerator is not used
// ...
idGenerator = undefined // new option (function)

When used, idGenerator should have the following signature:

(req) => { // accepts native request object (from 'http' module)
  const id = // generates an id (any type)
  return id; // returns it
}

Thus, when idGenerator option is used rTracer.id() will return objects returned from the function. You can generate a uuidv1 in this function and merge it with whatever you want in a single object. On the other hand, the default behavior will be kept as is, so this feature will be backwards-compatible.

I'd appreciate if you could change your PR in the described way. WDYT?

[marciopd]

Hi, sounds good.

But my use case is mostly about being able to add other request related variables into the same CLS context. For instance, I would like to have the client's IP.

So for my use case, maybe instead of exposing directly the cls-hooked API to customNamespacePropertiesBuilder, we could expose an abstraction and then use internally cls-hooked or any other API in the future.

What do you think?

[puzpuzpuz]

So for my use case, maybe instead of exposing directly the cls-hooked API to customNamespacePropertiesBuilder, we could expose an abstraction and then use internally cls-hooked or any other API in the future.

What do you think?

I'd rather keep cls-rtracer focused on id generation and avoid turning it into another CLS middleware toolkit. That's why I want to allow users to override id generation behavior (say, someone might want a plain number counter instead of uuid strings), which in your case could be used as a workaround to propagate other data as a part of id object.

[marciopd]

Ok, fair enough. Thanks for your time and the quick replies!

Best regards

[puzpuzpuz]

@marciopd

But my use case is mostly about being able to add other request related variables into the same CLS context. For instance, I would like to have the client's IP.

I didn't make it clear, probably, but idGenerator option will allow you to do that. Yes, that will be a workaround, but you'll be able to store anything along with the generated request id and obtain it later in the async call chain.

In any case, if you don't have the time to change this PR or submit another one, feel free to open a feature request issue. I may find some time to implement it in the nearest future.

[marciopd]

@puzpuzpuz I could do it.

Just to double-check if I got it right, the use case you guys were referring is something like:

"As a client of cls-rtracer, I want to use a custom request id generator, so that I can customize id generation and be able to return a custom object as the request id."

Is that it?

[puzpuzpuz]

"As a client of cls-rtracer, I want to use a custom request id generator, so that I can customize id generation and be able to return a custom object as the request id."

Is that it?

Yes, that's right. Thanks in advance!

[marciopd]

Hello @puzpuzpuz I have some time to work on this now.

Is it still relevant? I saw a related PR which seems to be related (https://github.com/puzpuzpuz/cls-rtracer/pull/47).

[puzpuzpuz]

@marciopd the main part of this feature was delivered in v2.4.0 (#40), yet the supported signature of factory function is () => unknown. #47 aims to change it to (req?: IncomingMessage | any) => unknown, but it seems to be stale (I'd say it's almost complete). If you need the req object in the factory function, you could pick up that PR or start another one from a scratch.

[marciopd]

Yes, that's exactly what I need. Ok, then I'll pick it up during the weekend.

Thx!