logger function

[osahyoun]

Here's a possible solution to logging the important stuff when functions are invoked. Thoughts?

I'm publishing errors to an SNS topic that'll in turn update us via email and slack.

[rodrei]

Hey @osahyoun, thanks for starting this!

About the approach, IMO since logging is something we need to figure out, ideally papertrail since we're already using that for our other apps. I think it's easier to use something like Papertrail alerts instead of implementing our own alerts system.

I did some research starting on the docs that @eyko shared, and I think a good approach is to have a separate logging lambda triggered by the logs of the core app lambdas and API Gateway. From that logging lambda we can push the logs to whatever system we want.

[vincemtnz]

I'd also export this for unit testing without the wrapper.

[osahyoun]

@rodrei where we stream/store our logs is a separate problem. This code is trying to address the problem of what do we actually want to log. Does that make sense?

[osahyoun]

What we want to avoid is littering our lambdas with console.log statements.

[rodrei]

@osahyoun You're suggesting to trigger alarms by manually creating an event on an SNS topic. What I'm suggesting is that we solve the logging issue first, and then use Papertrail alarms to trigger alerts from the logs.

[vincemtnz]

@osahyoun I also thought of going down a similar route: a wrapper function that logged every request (I guess in a format we're familiar with, or even using bunyan). For the wrapper function, however, I'd have written something a bit clearer.

function wrapper(handler) {
  return function (event, context, callback) {
    // log request
    console.log(...);
    // wrap the callback as well
    const cb = (...args) => {
      // perhaps log the response, in some cases (APIGWProxy)
      console.log(...args);
      // call the real callback
      callback(...args)
    }
    return handler(event, context, cb);
  }
};

[osahyoun]

Thanks, @eyko. That does look clearer.

[coveralls]

Coverage decreased (-0.8%) to 81.34% when pulling 996fa73 on feature.logging into 22d6872 on development.

[coveralls]

Coverage decreased (-0.8%) to 81.34% when pulling 996fa73 on feature.logging into 22d6872 on development.

[coveralls]

Coverage decreased (-0.8%) to 81.384% when pulling 9bc4c29 on feature.logging into 22d6872 on development.

[vincemtnz]

We can get name from context.functionName so it might be unnecessary to pass it a name?

[osahyoun]

@eyko 👍 thanks

[coveralls]

Coverage decreased (-0.6%) to 81.585% when pulling c0c1968 on feature.logging into 22d6872 on development.

[osahyoun]

@eyko ready for another review

[vincemtnz]

👍 let's go for it!