puzpuzpuz / cls-rtracer

Request Tracer - CLS-based request id generation for Express, Fastify, Koa and Hapi, batteries included
MIT License
311 stars 24 forks source link

I have a question #24

Closed love8587 closed 4 years ago

love8587 commented 4 years ago

Hi, I'm so impressed by cls-rtracer so I researched about cls and cls-hooked.

I'm creating my module using cls-hooked to learn deeply.

I've following and trying this post: here

However, It's a little bit different implemented code against your code. https://github.com/puzpuzpuz/cls-rtracer/blob/master/index.js#L61

And my code is :

const traceId = (opts: { test: number } | undefined): Middleware => {
  return async (ctx: Context, next: Function) => {
    // req and res are event emitters. We want to access CLS context inside of their event callbacks
    clsNamespace.bindEmitter(ctx.req);
    clsNamespace.bindEmitter(ctx.res);

    const traceID = uuidv4();

    clsNamespace.run(() => {
      clsNamespace.set('traceID', traceID);
      return next();
    });
  };
};

I'm wondering why you didn't use clsNamespace.run() . I think I'm not enough to understand at all. Is it same working code?

best wishes.

Steve.

puzpuzpuz commented 4 years ago

First of all, thanks a lot for the interest!

If I'm not mistaken, your snippet stands for Koa middleware. In this case, it must return a Promise and your implementation doesn't seem to do it (or I'm reading it wrong). That's the only reason why cls-rtracer uses Namespace.bind instead of Namespace.run. But I guess, the Koa middleware could be implemented on top of Namespace.run or Namespace.runAndReturn.

A side note. You should use uuid.v1() instead of uuid.v4(). See #21

love8587 commented 4 years ago

1) Wow. Thanks. I changed uuid from v4 to v1. I generally know v4 is better than v1. But performance is important when logging. So I agree with your opinion.

2) If I delete return next() from the Namespace.run() it will stop. So I think it's not a problem and I don't need to return Promise. Am I wrong?

puzpuzpuz commented 4 years ago

So I think it's not a problem and I don't need to return Promise. Am I wrong?

For non-async functions, you have to a promise. For async functions you need to await next middleware. I forgot to mention this important details before.

Please refer to Koa documentation or this blog post to understand why it's important for Koa middlewares.

love8587 commented 4 years ago

@puzpuzpuz many thanks. That's very important and helpful to me. My code is starting to look very similar to your code. :)

puzpuzpuz commented 4 years ago

@love8587 no problem at all. I'm glad to be able to help.

I'll close this issue as the question seems to be answered. Feel free to reopen it or create another one.