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

documentation ambiguous, and unclear if this library works #15

Closed
rick-kilgore opened this issue Jan 6, 2014 · 8 comments
Closed

documentation ambiguous, and unclear if this library works #15

rick-kilgore opened this issue Jan 6, 2014 · 8 comments

Comments

@rick-kilgore
Copy link

This may be just a documentation issue, but does this library currently work? I must be missing something, but I can't figure out how to call it from reading README.md. There are inconsistencies in the examples that beg the following questions:
1. do I need to call ns.run() or ns.createContext() - are they the same?
2. do I need to call ns.enter() and ns.exit()? I've not been able to ever retrieve data from my namespace unless I do call ns.enter().
3. should I add data to the namespace with ns.set('field', value) or context.field = value? The former seems to not work.
4. when I need to pull data out of the namespace, should I call cls.getNamespace() each time, or should I do it once at the top of each .js source file?

The best I have figured out how to do so far is if I answer my questions as follows:
1. doesn't matter - use ns.run() or ns.createContext()
2. yes, you HAVE to call ns.enter() (and ns.exit()?)
3. ns.set('field', value) does not work - use context.field = value
4. doesn't matter - call cls.getNamespace() once or each time

I've tried using the library this way with node versions 0.10.22 and 0.10.24 vm and I can't get things working right. Here is the code I have in my express.js app (my app is coffee-script in case that makes a difference, though it shouldn't):

at app startup:

    cls = require('continutation-local-storage')    // at the top of the source file

    ns = cls.createNamespace('catalogRequest')

when a request comes in:

    ns.run (ctx) =>           // => is coffee-script for function() 
        ns.enter(ctx)
        ctx.traceId = traceId
        <asynchronous call chain> =>
            ns.exit(ctx)

in a function called later in the continuation chain:

    cls = require('continutation-local-storage')    // at the top of the file

    ns = cls.getNamespace('catalogRequest')
    logger.info("traceId = #{ns.get('traceId')}")

This call to logger.info() does manage to print out the traceId that I set when I received the request - apparent success. But I still have two problems:
1. if I submit two requests to my service, one after the other, the traceId logged for the second request doesn't change. The namespace retrieves the first value I set on it (for the first request) forever.
2. in another source file - one in a different package that is included as a library - any call to cls.getNamespace('catalogRequest') always returns null. I can't get access to the namespace at all.

@othiym23
Copy link
Owner

othiym23 commented Jan 6, 2014

Sorry the documentation is inadequate! It probably needs to be rewritten to be more conceptual, because the current version presumes a familiarity with Node's APIs that isn't warranted (before Trevor Norris defined the lower-level API CLS uses, my goal was to get CLS added to Node core, and the current docs are written with that in mind, and like the rest of the API docs, they're pretty terse).

So, let me answer your questions in order, and see if it makes things clearer for you.

This may be just a documentation issue, but does this library currently work?

CLS currently works with node v0.8.0-0.11.10. New Relic for Node relies upon it, as well as a few other projects, and to the best of my knowledge, and barring a few known issues, it's working fine.

do I need to call ns.run() or ns.createContext() - are they the same?

You should be using one of ns.run() or ns.bind(). The simplest way to use CLS with express is to write a simple middleware that will run each request in its own CLS context:

index.js:

cls = require 'continuation-local-storage'
ns = cls.createNamespace 'catalog'

# ... Express initialization, although you want CLS middleware early in the chain
app.use (req, res, next) ->
  # make sure that event handlers on the request and response streams are in the CLS context
  ns.bindEmitter req
  ns.bindEmitter res
  # run the rest of the middleware chain within the CLS context
  ns.run next

app.use (req, res) ->
  # fetch traceId however
  ns.set 'traceId', traceId

routes.js:

# once CLS is bootstrapped, no longer necessary to include as first thing in module
ns = require('continuation-local-storage').getNamespace('catalog')

app.get '/traced', (req, res) ->
  logger.info "traceId = #{ns.get 'traceId'}"

You can use ns.createContext() with ns.bind() if you want more control over when the context gets created, but for simple use cases, that's an unnecessary complication. Binding is useful when you want more control over how execution proceeds, and ns.createContext() makes a new context with the active context for that namespace at the time createContext is called. You'll know when you need to understand the distinction there, but it's not something you should need immediately.

do I need to call ns.enter() and ns.exit()? I've not been able to ever retrieve data from my namespace unless I do call ns.enter().

ns.enter() and ns.exit() are for power users, and calling them is handled for you by ns.run() and ns.bind():

Namespace.prototype.run = function (fn) {
  var context = this.createContext();
  this.enter(context);
  try {
    fn(context);
    return context;
  }
  catch (exception) {
    exception[ERROR_SYMBOL] = context;
    throw exception;
  }
  finally {
    this.exit(context);
  }
};

Namespace.prototype.bind = function (fn, context) {
  if (!context) {
    if (!this.active) {
      context = Object.create(this.active);
    }
    else {
      context = this.active;
    }
  }

  var self = this;
  return function () {
    self.enter(context);
    try {
      return fn.apply(this, arguments);
    }
    catch (exception) {
      exception[ERROR_SYMBOL] = context;
      throw exception;
    }
    finally {
      self.exit(context);
    }
  };
};

(By the way, while you shouldn't have to read the code to be able to use CLS, my hope is that it's pretty easy to follow if you do choose to take a look at it.)

You should never need to call ns.enter() and ns.exit() yourself.

should I add data to the namespace with ns.set('field', value) or context.field = value? The former seems to not work.

Always use ns.set() and ns.get(). If they're not working, you're not in an active CLS context (i.e. not in a function being run by ns.run() or ns.bind()). Working directly with context objects will not do what you expect with asynchronous execution.

when I need to pull data out of the namespace, should I call cls.getNamespace() each time, or should I do it once at the top of each .js source file?

You only need to grab the namespace once per module where you're going to be using it.

Let me know if this still leaves you with questions! Thanks for checking out CLS, and I hope it proves useful for you!

@rick-kilgore
Copy link
Author

Forrest,

Thank you so much for the quick response – I really appreciate it! I didn’t get to it today, but I will try using your suggestions tomorrow.

Thanks again,

  • Rick

From: Forrest L Norvell <notifications@github.commailto:notifications@github.com>
Reply-To: othiym23/node-continuation-local-storage <reply@reply.github.commailto:reply@reply.github.com>
Date: Sunday, January 5, 2014 at 10:32 PM
To: othiym23/node-continuation-local-storage <node-continuation-local-storage@noreply.github.commailto:node-continuation-local-storage@noreply.github.com>
Cc: Rick Kilgore <rick.kilgore@hbo.commailto:rick.kilgore@hbo.com>
Subject: Re: [node-continuation-local-storage] documentation ambiguous, and unclear if this library works (#15)

Sorry the documentation is inadequate! It probably needs to be rewritten to be more concept-ual, because the current version presumes a familiarity with Node's APIs that isn't warranted (before Trevor Norris defined the lower-level API CLS uses, my goal was to get CLS added to Node core, and the current docs are written with that in mind, and like the rest of the API docs, they're pretty terse).

So, let me answer your questions in order, and see if it makes things clearer for you.

This may be just a documentation issue, but does this library currently work?

CLS currently works with node v0.8.0-0.11.10. New Relic for Node relies upon it, as well as a few other projects, and to the best of my knowledge, and barring a few known issues, it's working fine.

do I need to call ns.run() or ns.createContext() - are they the same?

You should be using one of ns.run() or ns.bind(). The simplest way to use CLS with express is to write a simple middleware that will run each request in its own CLS context:

index.js:

cls = require 'continuation-local-storage'ns = cls.createNamespace('catalog')# ... Express initialization, although you want CLS middleware early in the chainapp.use (req, res, next) ->

make sure that event handlers on the request and response streams are in the CLS context

ns.bindEmitter req
ns.bindEmitter res

run the rest of the middleware chain within the CLS context

ns.run nextapp.use (req, res) ->

fetch traceId however

ns.set('traceId', traceId)

routes.js:

once CLS is bootstrapped, no longer necessary to include as first thing in modulens = require('continuation-local-storage').getNamespace('catalog')app.get '/traced', (req, res) ->

logger.info "traceId = #{ns.get 'traceId'}"

You can use ns.createContext() with ns.bind() if you want more control over when the context gets created, but for simple use cases, that's an unnecessary complication. Binding is useful when you want more control over how execution proceeds, and ns.createContext() makes a new context with the active context for that namespace at the time createContext is called. You'll know when you need to understand the distinction there, but it's not something you should need immediately

do I need to call ns.enter() and ns.exit()? I've not been able to ever retrieve data from my namespace unless I do call ns.enter().

ns.enter() and ns.exit() are for power users, and calling them is handled for you by ns.run() and ns.bind():

Namespace.prototype.run = function (fn) {
var context = this.createContext();
this.enter(context);
try {
fn(context);
return context;
}
catch (exception) {
exception[ERROR_SYMBOL] = context;
throw exception;
}
finally {
this.exit(context);
}};Namespace.prototype.bind = function (fn, context) {
if (!context) {
if (!this.active) {
context = Object.create(this.active);
}
else {
context = this.active;
}
}

var self = this;
return function () {
self.enter(context);
try {
return fn.apply(this, arguments);
}
catch (exception) {
exception[ERROR_SYMBOL] = context;
throw exception;
}
finally {
self.exit(context);
}
};};

(btw, while you shouldn't have to read the code to be able to use CLS, my hope is that it's pretty easy to follow if you do choose to take a look at it.)

You should never need to call ns.enter() and ns.exit() yourself.

should I add data to the namespace with ns.set('field', value) or context.field = value? The former seems to not work.

Always use ns.set() and ns.get(). If they're not working, you're not in an active CLS context (i.e. not in a function being run by ns.run() or ns.bind(). Working directly with context objects will notdo what you expect with asynchronous execution.

when I need to pull data out of the namespace, should I call cls.getNamespace() each time, or should I do it once at the top of each .js source file?

You only need to grab the namespace once per module where you're going to be using it.

Let me know if this still leaves you with question! Thanks for checking out CLS, and I hope it proves useful for you!

Reply to this email directly or view it on GitHubhttps://github.com//issues/15#issuecomment-31630057.

This e-mail is intended only for the use of the addressees. Any copying,
forwarding, printing or other use of this e-mail by persons other than the
addressees is not authorized. This e-mail may contain information that is
privileged, confidential and exempt from disclosure. If you are not the intended
recipient, please notify us immediately by return e-mail (including the original
message in your reply) and then delete and discard all copies of the e-mail.

Thank you.


@rick-kilgore
Copy link
Author

Hi Forrest,

I've tried your suggestions and I think I'm almost there. One problem I'm having is that when I call Cursor.toArray() on a mongoDB cursor in the native mongo library, it somehow loses my continuation chain. Immediately before the call, the data in my namespace looks good. But if I print out the contents of the namespace in my callback that I pass to Cursor.toArray(), namespace.active is null.

Do you all use Mongo at all? I think the mongo driver might be using emitters in places. Would I need to figure out how to make bindEmitter calls on these emitter objects for this to work?

[edit: I switched to using the Mongo CursorStream API, and then if I call ns.bindEmitter() on the CursorStream object before iterating over its items, everything seems to be working. I'm a little shaky on what is required, though. If you could elaborate on this and help me to understand when I can expect to run into this type of thing and how I should in general deal with it, I'd really appreciate it.]

Thank you,
- Rick

@othiym23
Copy link
Owner

othiym23 commented Jan 8, 2014

The short, sweet answer to working with database APIs that batch up or pipeline calls: use ns.bind() on the callback to .toArray(). Because the continuation chain is broken by the call out to the database, a small shim is required to bridge the gap. (This is something you also need to do with domains for async error-handling.)

The reason the CursorStream works is because then all the data events called on the stream are called within the CLS context because of how .bindEmitter() modifies the EE.

@rick-kilgore
Copy link
Author

Sounds good. Thanks!

@santimacia
Copy link

I'm having troubles with configuration under express4. I tried at the begining and in the end with same result. This is the code I tried:

app.use(function(req, res, next) {
  ns.bindEmitter(req);
  ns.bindEmitter(res);
  return ns.run(next);

});
app.use(function(req, res) {
  return ns.set('traceId', 'something');


});
app.get('/traced', function(req, res) {
   console.log("traceId = " + (ns.get('traceId')));
   res.send('ok')
});

And the stack error:

TypeError: Object object has no method 'toString'
    at node_modules/express/lib/application.js:140:28
    at node_modules/express/lib/router/index.js:140:5
    at node_modules/express/lib/router/index.js:265:10
    at next (node_modules/express/lib/router/index.js:165:14)
    at next (node_modules/express/lib/router/index.js:182:38)
    at next (node_modules/express/lib/router/index.js:182:38)
    at next (node_modules/express/lib/router/index.js:182:38)
    at next (node_modules/express/lib/router/index.js:182:38)
    at next (node_modules/express/lib/router/index.js:182:38)
    at next (node_modules/express/lib/router/index.js:182:38)

@othiym23
Copy link
Owner

@encaputxat Could you open this on a new issue, please? This one is closed, so it's a tough place to keep track of things. What version of Express are you using? It may be that I need to do something to make CLS work properly with Express 4, but an exact version will make it easier for me to reproduce the problem you're seeing.

@iliakan
Copy link

iliakan commented May 13, 2015

@rick-kilgore did you solve the issue w/ mongodb? Do you use this module now?

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