Logging

[iskandersierra]

Hello @ncthbrt I would like to work on this project. The logging issue seem good enought to allow me to familiarize with all the code before taking more important stuff. If you have some ideas on this particular issue (logging abstractions or dependency on other lib, log levels, what to log, etc.) let me know and I will be working ASAP on this.

[ncthbrt]

@iskandersierra Excited :)

My thoughts around this issue was perhaps providing log methods in the actor's ctx object, then defining a message protocol which sends all logs to a logging actor which is defined at startup.

This is quite similar to what akka is doing, and seems quite a sensible approach to things.

Example usage would be:

ctx.log.error('A terrible thing has happened!', { details: 'some object containing information about the issue' })

We'd probably want to also include information about the actor in the logging protocol such as its path.

Let me know what you think

[iskandersierra]

Nice, I get it. Let me ask a couple of questions:

• How would you see a user pick the concrete implementation of logging?
• What is the API of ActorSystem an extension can use to extend/configure the system's behavior?
• The concept of telemetry is broader than logging, in my opinion. We could add to the ctx, as you say, the possibility to access a telemetry API or a simpler log API. Like so:

  // Using the simpler log API
  ctx.log.error('A terrible thing has happened!', { details: 'some object containing information about the issue' })
  // Using the telemetry API
  ctx.telemetry.trace.error('A terrible thing has happened!', { details: 'some object containing information about the issue' })
  ctx.telemetry.event('actor-created', { /* JS object with event details */ }, { /* JS object with event metrics */ })
  ctx.telemetry.metric('death-count', { /* JS object with metric details */ })

  This would allow a richer API to express system performance and functioning.

• Would a sensible default be console? That's all for now. :) I'll start working an a first approach ASAP in a fork.

[ncthbrt]

I like your telemetry idea.

I was thinking of picking the logging provider in a manner similar to how the persistence engine is injected into the system object:

const { configureTelemetry } = require('nact');
const system  = start(configureTelemetry(spawnLogstashActor("connection-string")));

Right now there is really only one extension point. For reference here is how the persistence engine is installed into the system:

const configurePersistence = (engine) => (system) => {
  if (!engine) {
    throw new Error('Persistence engine should not be undefined');
  }
  return Object.assign(system, { persistenceEngine: engine });
};

[iskandersierra]

Thanks. I'll start playing with the code to see where it gets me.

[iskandersierra]

Hello @ncthbrt, Happy New Year I was wondering if there is a way to access the ActorSystem from the Actor, at least from the internals. Or alternativelly the system could have some precense beside it reference, in the actor. I would like the Actor class to be able to do something like this:

  createContext (sender) {
    return this.systemExtensions.createContext({
      parent: this.parent.reference,
      path: this.path,
      self: this.reference,
      name: this.name,
      children: new Map(this.childReferences),
      sender
    });
  }

In particular, but could be generalized to other 'system hooks'.

The idea behind systemExtensions is allowing extensions to influence or determine the way the actors work. So an extension could do something like:

const configureLogging => (engine) => (system) => {
  system.registerHook(SystemHooks.CREATE_CONTEXT, (ctx) => Object.assign(ctx, { logger: getLogger(ctx) }))
}

The syntax is only speudo-code so any suggestions is open.

This allow any extension (including supervision, persistence, etc.) to introduce changes into the system without affecting (most of the time) the Actor/ActorSystem code. Some aspects like supervision, could be kept from being extensions, because they are central to the actor model, but persistence and logging are not, in my opinion.

What do you think about it?

Best regards.

[ncthbrt]

Happy New Year @iskandersierra 🎆 🎊

Note: The Actor can access the system object using this.system.

Rather than:

const configureLogging => (engine) => (system) => {
  system.registerHook(SystemHooks.CREATE_CONTEXT, (ctx) => Object.assign(ctx, { logger: getLogger(ctx) }))
}

I'd prefer something along the lines of:

const configureLogging => (engine) => (system) => {
  system.use(async (state, msg, ctx, next) =>  {  
        let nextCtx = { ... ctx, logger: getLogger(ctx) };
        let result = await next(state, msg, nextCtx);
        return result;
  });
}

This is somewhat analogous to Koa and .NET core's middleware APIs.

That being said, I'm a little nervous of creating any sort of fully fledged extension/middleware API. I'm working to create a clustering/remoting layer right now and it could fundamentally change how actors work internally. If we make a mistake with how the extension API is designed, it might make it a lot more difficult to evolve nact without breaking our user's workflows.

An alternative approach (which I would be a lot more comfortable with) would be to implement logging naïvely for now by adding logging functions directly to the Actor class's createContext() method, and using configureLogging simply to specify the logging engine. Later on once the internal implementation has stabilized, we could inject the logging methods using configureLogging without breaking userland.

Let me know what you think.

[iskandersierra]

I see your point and its fair enough. Ok lets do it the naive way for now. I'll let you know when I have something done and tested.

[ncthbrt]

@iskandersierra I've just created PR #37, this may affect your implementation of logging. Shouldn't be too severe but you may get a merge conflict or two. Let me know if I can help in any way

[iskandersierra]

Hi @ncthbrt Can I use library Sinon to mock/spy on functions to test whether the functions are being called with expected arguments, expected number of times, etc.?

[ncthbrt]

Sure 👍

[iskandersierra]

@ncthbrt I have a first draft to be reviewed here. How do you prefer to go about the review/PR process?

[ncthbrt]

Feel free to open a standard PR. :) Mind just adding the line "Fixes #21" to the opening statement of the PR so that it correctly links in https://waffle.io/ncthbrt/nact