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

CPU leakage #50

Open
nettad opened this issue Nov 13, 2019 · 3 comments
Open

CPU leakage #50

nettad opened this issue Nov 13, 2019 · 3 comments

Comments

@nettad
Copy link

nettad commented Nov 13, 2019

Hi, I've started using this library on our production servers. It's a great help and saves a lot of "passing down" from one function to the next. However, over time I see that it increasingly uses more and more CPU resources, specifically around things related to async-hooks. I took a CPU profile of the server and I see emitHook going from 7% to 30%. Inside emitHook, the processes that grow the most are destroy (1% to 8%), before grows from 2% to ~6%, after (~1% to 6$). Is this expected. Am I using it wrong? Please advise.
Below is my code snippet.

  const _NAMESPACE = 'FOO_LOGGING'; 
  const _LOGGER_CONTEXT = 'logger'; 

 class LoggingContext { 
 constructor(token, instance, level = 'debug') {
      this._loggerBuilder =
         aLogger()
           .withLogentriesToken(token)
           .withLevel(level);

      this._instance = instance;
   }

   runWithContext(aspects, fn, ...args) {
     try {
        const userSession = aspects['session'];
        const requestId = AspectsApi.getRequestId(aspects) || uuid();
        const context = {
            instance: this._instance || 'none',
            requestId,
            userId: userSession && userSession.userGuid
       };
         const logger = this._loggerBuilder.withContext(context);
          const session = createNamespace(_NAMESPACE);
          return session.runAndReturn(() => {
             session.set(_LOGGER_CONTEXT, logger);
             return fn(...args);
         });
         } catch (e) {
            this._loggerBuilder.build().error(e);
         }
         }

    static get() {
       try {
          const session = getNamespace(_NAMESPACE);
          const logger = session.get(_LOGGER_CONTEXT);
          if (logger) {
            return logger.build();
         }
      else {
         console.log('error retrieving logger in context');
      return console;
     }
   } catch (e) {
      if (global.infoLogger) {
        return global.infoLogger;
      } else {
         console.log('error retrieving logger in context:', e);
         return console;
     }
   }  }
@wanglihui
Copy link

in my application , when many request coming, cpu will up too , and not down. when remove cls, will be ok! expect answer, 3q

@Themandunord
Copy link

Themandunord commented Dec 11, 2019

Same issue... :/

When I have deployed the feature :
image
image

I'm on Docker, Node 12 Alpine, NestJS

My server have 3000req/15s

@thesciz
Copy link

thesciz commented Jan 30, 2020

I'm also seeing graphs strikingly similar to what @Themandunord is seeing. Something's definitely leaky - also using Docker, Node 12 Alpine.

So in my case, I was definitely using this wrong.

I had some middleware that was doing something like this:

const { createNamespace } = require('cls-hooked');

module.exports = (req, res, next) => {
    req.headers['x-request-id'] = 'whatever';

    const reqTrace = createNamespace('reqTrace');
    reqTrace.run(() => {
        reqTrace.set('requestId', req.headers['x-request-id']);
        next();
    });
}

We were creating the namespace every time the middle was invoked; moving const reqTrace = createNamespace('reqTrace'); out of the function seems to have resolved this.

Obvious in hindsight, but perhaps there should be a warning or error if you try to create a namespace that already exists?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants