Skip to content

Commit

Permalink
SNOW-1016523 log to console by default as it was before (#779)
Browse files Browse the repository at this point in the history
SNOW-1016523 log to console by default as it was before
  • Loading branch information
sfc-gh-knozderko authored Feb 27, 2024
1 parent 7efa640 commit c8ef0b0
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 8 deletions.
5 changes: 4 additions & 1 deletion lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,14 @@ function Core(options) {
configure: function (options) {
const logLevel = extractLogLevel(options);
const logFilePath = options.logFilePath;
const additionalLogToConsole = options.additionalLogToConsole;
if (logLevel != null || logFilePath) {
Logger.getInstance().debug(`Configuring logger with level: ${logLevel}, filePath: ${logFilePath}, additionalLogToConsole: ${additionalLogToConsole}`);
Logger.getInstance().configure(
{
level: logLevel,
filePath: logFilePath
filePath: logFilePath,
additionalLogToConsole: additionalLogToConsole
});
}

Expand Down
4 changes: 3 additions & 1 deletion lib/logger/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ exports.createLogger = function (options, logMessage, reconfigureOperation) {
let localMessageMaxLength;
let localLevel;
let localFilePath;
let localAdditionalLogToConsole;

// if an options argument is specified
if (Util.exists(options)) {
Expand All @@ -143,6 +144,7 @@ exports.createLogger = function (options, logMessage, reconfigureOperation) {
localMessageMaxLength = options.messageMaxLength;
localLevel = options.level;
localFilePath = options.filePath;
localAdditionalLogToConsole = options.additionalLogToConsole;
}

// if an includeTimestamp options is specified, convert it to a boolean
Expand Down Expand Up @@ -184,7 +186,7 @@ exports.createLogger = function (options, logMessage, reconfigureOperation) {
}

if (Util.isFunction(reconfigureOperation)) {
reconfigureOperation(localFilePath);
reconfigureOperation(localFilePath, localAdditionalLogToConsole);
}
},

Expand Down
3 changes: 2 additions & 1 deletion lib/logger/easy_logging_starter.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ exports.init = async function (configFilePathFromConnectionString) {
logger.info('Initializing Easy Logging with logPath=%s and logLevel=%s from file: %s', logPath, config.loggingConfig.logLevel, config.configPath);
logger.configure({
level: logLevel,
filePath: path.join(logPath, 'snowflake.log')
filePath: path.join(logPath, 'snowflake.log'),
additionalLogToConsole: false
});
logger.easyLoggingConfigureCounter = (logger.easyLoggingConfigureCounter ?? 0) + 1;
initTrialParameters = {
Expand Down
31 changes: 30 additions & 1 deletion lib/logger/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const Core = require('./core');
const Util = require('../util');
const Errors = require('../errors');

const DEFAULT_ADDITIONAL_LOG_TO_CONSOLE = true;

/**
* Creates a new Logger instance for when we're running in node.
*
Expand All @@ -18,6 +20,8 @@ function Logger(options) {
let winstonLogger;
const defaultFilePath = 'snowflake.log';
let filePath = getFilePath(options);
let additionalLogToConsole = DEFAULT_ADDITIONAL_LOG_TO_CONSOLE;
let transportLabels = [];

this.setLogger = function (logger) {
winstonLogger = logger;
Expand Down Expand Up @@ -52,15 +56,28 @@ function Logger(options) {
transport.close();
}

function reconfigureWinstonLogger(filePathInput) {
function reconfigureWinstonLogger(filePathInput, additionalLogToConsoleInput) {
const currentWinstonLogger = winstonLogger;
filePath = filePathInput ?? filePath;
if (Util.isBoolean(additionalLogToConsoleInput)) {
additionalLogToConsole = additionalLogToConsoleInput;
} else {
additionalLogToConsole = DEFAULT_ADDITIONAL_LOG_TO_CONSOLE;
}
winstonLogger = null; // it will be created for the first log operation
if (currentWinstonLogger) {
currentWinstonLogger.close();
}
}

function setTransportLabels(transportLabelsInput) {
transportLabels = transportLabelsInput;
}

this.getTransportLabels = function () {
return transportLabels;
};

/**
* Logs a message at a given level.
*
Expand All @@ -72,11 +89,17 @@ function Logger(options) {
// initialize the winston logger if needed
if (!winstonLogger) {
let transports;
let transportLabels;

if ('STDOUT' === filePath.toUpperCase()) {
transports = [new (winston.transports.Console)()];
transportLabels = ['Console'];
} else if (additionalLogToConsole === true) {
transports = [new (winston.transports.Console)(), new (winston.transports.File)({ filename: filePath })];
transportLabels = ['Console', 'File'];
} else {
transports = [new (winston.transports.File)({ filename: filePath })];
transportLabels = ['File'];
}

winstonLogger = new winston.createLogger(
Expand All @@ -85,6 +108,7 @@ function Logger(options) {
level: common.getLevelTag(),
levels: common.getLevelTagsMap()
});
setTransportLabels(transportLabels);
}

// get the appropriate logging method using the level tag and use this
Expand Down Expand Up @@ -117,6 +141,11 @@ function Logger(options) {
* @param {Object} options
*/
this.configure = function (options) {
if (Util.isBoolean(options.additionalLogToConsole)) {
additionalLogToConsole = options.additionalLogToConsole;
} else {
additionalLogToConsole = DEFAULT_ADDITIONAL_LOG_TO_CONSOLE;
}
common.configure(options);
};

Expand Down
2 changes: 2 additions & 0 deletions test/integration/testEasyLoggingOnConnecting.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ describe('Easy logging tests', function () {
if (err) {
done(err);
} else {
Logger.getInstance().info('Logging something');
assert.strictEqual(Logger.getInstance().getLevelTag(), logLevel);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());
done();
}
});
Expand Down
53 changes: 49 additions & 4 deletions test/unit/logger/easy_logging_starter_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const os = require('os');
const Logger = require('../../../lib/logger');
require('../../../lib/snowflake'); // import of it sets up node logger
const { exists } = require('../../../lib/util');
const snowflake = require('../../../lib/snowflake');
const defaultConfigName = 'sf_client_config.json';
const logLevelBefore = Logger.getInstance().getLevel();
let tempDir = null;
Expand All @@ -19,14 +20,14 @@ before(async function () {
});

after(async function () {
Logger.getInstance().configure({
level: logLevelBefore,
filePath: 'snowflake.log'
});
await fsPromises.rm(tempDir, { recursive: true, force: true });
});

afterEach(async function () {
Logger.getInstance().configure({
level: logLevelBefore,
filePath: 'snowflake.log'
});
await fsPromises.rm(path.join(os.homedir(), defaultConfigName), { force: true });
resetEasyLoggingModule();
});
Expand All @@ -41,10 +42,12 @@ describe('Easy logging starter tests', function () {

// when
await init(configFilePath);
Logger.getInstance().error('Logging something'); // we need to log anything to make the logger being recreated

// then
assert.strictEqual(Logger.getInstance().getLevelTag(), logLevel);
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, 1);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());

// when
await init(null);
Expand All @@ -54,6 +57,7 @@ describe('Easy logging starter tests', function () {
// then
assert.strictEqual(Logger.getInstance().getLevelTag(), logLevel);
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, 1);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());
});

it('should configure easy logging only once when initialized without config file path', async function () {
Expand All @@ -64,10 +68,12 @@ describe('Easy logging starter tests', function () {
// when
await init(null);
await init(null);
Logger.getInstance().error('Logging something'); // we need to log anything to make the logger being recreated

// then
assert.strictEqual(Logger.getInstance().getLevelTag(), logLevel);
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, 1);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());
});

it('should reconfigure easy logging if config file path is not given for the first time', async function () {
Expand All @@ -79,17 +85,21 @@ describe('Easy logging starter tests', function () {

// when
await init(null);
Logger.getInstance().error('Logging something'); // we need to log anything to make the logger being recreated

// then
assert.strictEqual(Logger.getInstance().getLevelTag(), homeDirLogLevel);
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, 1);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());

// when
await init(customConfigFilePath);
Logger.getInstance().error('Logging something'); // we need to log anything to make the logger being recreated

// then
assert.strictEqual(Logger.getInstance().getLevelTag(), customLogLevel);
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, 2);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());
});

it('should fail for unknown log level', async function () {
Expand Down Expand Up @@ -124,6 +134,41 @@ describe('Easy logging starter tests', function () {
});
});

it('should create console and file transports by default when not using client configuration', function () {
// when
Logger.getInstance().error('Logging something');

// then
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, undefined);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['Console', 'File'].toString());
});

it('should configure logger with file and console', function () {
// given
const logLevel = 'ERROR';

// when
snowflake.configure({ logLevel });
Logger.getInstance().error('Logging something');

// then
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, undefined);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['Console', 'File'].toString());
});

it('should configure logger for file without console', function () {
// given
const logLevel = 'ERROR';

// when
snowflake.configure({ logLevel: logLevel, additionalLogToConsole: false });
Logger.getInstance().error('Logging something');

// then
assert.strictEqual(Logger.getInstance().easyLoggingConfigureCounter, undefined);
assert.strictEqual(Logger.getInstance().getTransportLabels().toString(), ['File'].toString());
});

async function createConfigFile(logLevel, configDirectory, configFileName, logPath) {
const configFilePath = path.join(configDirectory, configFileName);
const configContent = `{
Expand Down

0 comments on commit c8ef0b0

Please sign in to comment.