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

Logging configuration #116

Open
deinspanjer opened this issue Nov 6, 2019 · 8 comments
Open

Logging configuration #116

deinspanjer opened this issue Nov 6, 2019 · 8 comments

Comments

@deinspanjer
Copy link

What is the right way to handle configuration of the logger?
Based on notes in the source code, I tried bringing in log4js as a dependency and then changing the log level like this:

const log4js = require('log4js');
log4js.getlogger().level = 'warn';
log4js.getLogger('ArangoDBAdapter').level = 'warn';

const project = new Project({
  loggerProvider: log4js,
        sources: [
...

But even with that, I still get a log message from the ArangoDBAdapter:
ArangoDBAdapter: Executing AQL: @v_result1 = execute(

Is it right for me to be bringing in log4js as a dependency? it doesn't look like the built-in ConsoleLoggerProvider is exported, nor is it actually configurable, so I'm thinking that part is right.

@henkesn
Copy link
Collaborator

henkesn commented Nov 6, 2019

You can have a look at LoggerProvider. You can either have your own logger or you can pass log4js directly as you do in your code. Implementing your own provider also lets you add specific data like a request id or a username (delegator style).
Regarding the log level: I don't think you change the level like this. You only set the level for the logger object retrieved by getLogger() which remains unused in your code. Please have a look at the log4js manual how to properly configure log levels.

@henkesn
Copy link
Collaborator

henkesn commented Nov 6, 2019

Regarding the ConsoleLoggerProvider, this is only for internal test purposes. We don't want to have the log4js in the cruddl package.json. That's why there is an unexported minimalistic console logger implementation in the code.

@deinspanjer
Copy link
Author

So the way that I am setting the level works for most things inside cruddl, everything except for that one message that is logged inside ArangoDBAdapter. I believe it works because I am configuring the default logging provider that is returned whenever .getLogger() is called, so cruddl sees that new value.

I can't pass an instance to loggingProvider because the instances don't have the constructor or getLogger() function that the interface expects.

I'll keep looking in the log4js docs, but their minimalist configuration and usage just looks like this:

var logger = log4js.getLogger();
logger.level = 'debug'; // default level is OFF - which means no logs at all.
logger.debug("Some debug messages");```

@deinspanjer
Copy link
Author

deinspanjer commented Nov 6, 2019

Okay, I found the more appropriate way to configure the default logger, but I'm still getting the logging from ArangoDBAdapter. Here is my new code snippet:

        const loggerProvider = log4js.configure({
            appenders: { out: { type: 'stdout'}},
            categories: {default: {appenders: ['out'], level: env.logging.LEVEL}}
        });

        const project = new Project({
            loggerProvider,
            sources: [

@henkesn
Copy link
Collaborator

henkesn commented Nov 6, 2019

Hm that sounds strange. Is it just this Executing AQL... log which you get?
In the code there is even a condition around this log because the log generation is expensive.
Perhaps you can debug into that line and check if the log4js level is correct. Or do you perhaps redefine the default log levels?

@deinspanjer
Copy link
Author

Yes, it is only that one message I'm getting. I was wondering if maybe the fact that the adapter seems to try to set up its own logger was preventing the one I was passing in from being used.

@henkesn
Copy link
Collaborator

henkesn commented Nov 6, 2019

Yes, but you are not passing a logger but a LoggerProvider. The Logger itself is created when needed, specifically for a category. The ArangoDBAdapter still uses your provided LoggerProvider.

@deinspanjer
Copy link
Author

Could you see if you can replicate the behavior?

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

2 participants