dev-mastery / comments-api

MIT License
1.92k stars 649 forks source link

Implementing logging #7

Open rush-levint opened 5 years ago

rush-levint commented 5 years ago

Hi, how do we go about implementing logging? Do we just import a logger function anywhere in our use cases or pass it down from the frameworks & drivers layer?

arcdev1 commented 5 years ago

Create an adapter that wraps some framework and then inject it into your use case.

On a brand new project I usually start by wrapping the console API and introduce some logging framework later as we get closer to launch. Something like:

// logger/logger.js
export default function makeLogger(spec) {
  return Object.freeze({
    debug: console.debug,
    error: console.error,
    info: console.info,
    warn: console.warn
  })
}
// logger/index.js
import logger from './logger'
const logger = makeLogger()
export default logger
Ryuno-Ki commented 5 years ago

Hm, I'd pass some kind of argument to allow a prefix. From experience, I can tell, that this helps to identify, from where logging happened. Plus, if you log an error, the stack trace might be confusing …

You could use loglevel or winston for example. The other parts of the application shouldn't care, what solution is used in the end …

rush-levint commented 5 years ago

Looks clearer now, thanks :D

Considering @Ryuno-Ki 's opinion, would it still be valid if in logger/index.js we expose the logger as a function accepting a prefix that returns a (child) logger where the prefix is taken into account?

something similar to loglevel's .getLogger(name)

arcdev1 commented 5 years ago

There are many ways to extend/improve on the basic example I provided. It really depends on the needs of your application.

The main point is, use an adapter and injection to avoid a direct dependency on a specific logging library. Logging is something that will get used everywhere so you have to be very careful about directly referencing a particular 3rd party library all over your code.

PixsaOJ commented 3 years ago

Could you provide an example with singleton logger that uses 3rd party library like Bunyan?

arcdev1 commented 3 years ago

@PixsaOJ it might look something like this...

// /logger/logger.js
export function makeLogger({thirdPartyLogger}) {
  return Object.freeze({
    info: (msg) => thirdPartyLogger.info(msg) // you should do more here (e.g. wrap proprietary errors).
   // more method wrappers like .warn or .error
  })
}
// /logger/index.js
import ThirdPartyLogger from "third-party-logger" // this could be Bunyan
import {makeLogger} from "./logger"

const thirdPartyLogger = new ThirdPartyLogger({/* config */})
export const Logger = makeLogger({thirdPartyLogger}) // here's your singleton

Now the rest of your app uses the logger exported from /logger/index.js and if you ever want or need to switch logging libraries you have one place to do it.

PixsaOJ commented 3 years ago

Thanks for the response @arcdev1. What do you think, would it be a good idea to use our generated logger as dependency injection? Or just require/import it whenever we need to log.

E.g. when we are writing tests, it could be easier to inject as dependency. However, I do not know how pretty it would be to that in actual code. I would have to inject in routes first, then in controllers, then in use-cases...

EDIT: I currently have it as a global variable global.logger = config.logger()

arcdev1 commented 3 years ago

@PixsaOJ Logging tends to have side effects (e.g. writing to a file) so it's important to use dependency injection to avoid brittle tests.

Reading between the lines of your question, it seems like you are logging in lots of places. If you find that you get a lot of value from doing that then it's all good.

In my case, I've noticed that limiting logs to an exception handler seems to be enough. So, at the entry point(s) of my app, I have a single try/catch that sends Exceptions to an Exception Handler which logs the exception and makes some decisions about what else to do with the Exception based on its type. This means I only really have to inject the Logger in my Exception Handler.

I've worked for clients that like to log a lot more than that and in those cases, we just accepted that you have to inject your logger everywhere.

PixsaOJ commented 3 years ago

@arcdev1 Makes sense, thank you.