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

Provide context local/promise local logging configs with cls-hooked #71

Open
koraa opened this issue Nov 21, 2019 · 1 comment
Open
Labels
question Further information is requested

Comments

@koraa
Copy link
Contributor

koraa commented Nov 21, 2019

https://www.npmjs.com/package/cls-hooked can be used to provide variables local to the current promise/async function context.

Current/Possible use case:

  • Providing the activationId field in openwhisk for logging (multiple activations can run at the same time) – this is already possible with a custom filter
  • As a better foundation to implement recordLogs and magically make it work like expected. (This would be a really nice feature)

"Async awareness" could serve as another really nice unique selling point compared to other logging frameworks if we get this right!

Multiple API designs spring to my mind:

  • Making rootLogger context dependent (or making it a getter so it can be customized by the library user)
  • Making filters/formatters context dependant

To be honest I don't really like any of those ideas…

@koraa koraa added the question Further information is requested label Nov 21, 2019
@tripodsan
Copy link
Contributor

tripodsan commented Nov 22, 2019

I don't think that helix-log needs to provide continuation context (thread local) support out of the box.

Making rootLogger context dependent (or making it a getter so it can be customized by the library user)

it's not the logger that needs to be context dependent, but invoking the logger needs to be. eg:

// doesn't work!
const contextLogger = rootLoger.getContextAware();
contextLogger.log()

// works:
const contextLogger = rootLoger.getContextAware();
contextLogger.run(() => {
  contextLogger.log()
});

another approach could be to make the rootLogger a cls-context, eg:

// ---- helix-log
const rootLogger = createContext('helix-root-logger');
rootLogger.log = ....

// ---- test.js
rootLogger.run(() => {
   rootLogger.set('id', 42);
   info('should work!');
});

also see the writer example in cls-hooked: https://github.com/jeff-lewis/cls-hooked


all in all, I think the complexity of using cls-hooked is too big and might be too opinionated to provide out of the box. maybe adding some examples of how this could be achieved is enough.

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

No branches or pull requests

2 participants