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

396 - fix message logs #399

Merged
merged 5 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## 0.65.3
Release 2020-03-dd

- Fix noisy message logs while checking analyses' limits.
- Fix CI setup, explicit use of PGPORT while creating the PostgreSQL cluster.

## 0.65.2
Expand Down
8 changes: 5 additions & 3 deletions lib/limits/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ function LimitsContext(databaseService, limits, logger) {
}

// Log analysis limits error information
LimitsContext.prototype.logError = function(err) {
if (this.logger) {
this.logger.logLimitsError(err);
LimitsContext.prototype.logError = function(err, node) {
if (!this.logger) {
return;
}

this.logger.logLimitsError(err, node);
};

// Execute SQL in the database
Expand Down
16 changes: 12 additions & 4 deletions lib/logging/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,18 @@ AnalysisLogger.prototype.log = function (analysis) {
});
};

AnalysisLogger.prototype.logLimitsError = function (err) {
if (err) {
this.logger.info({ 'error-message': err.message, 'error-code': err.code }, 'analysis:limits_error');
}
AnalysisLogger.prototype.logLimitsError = function (err, node) {
const msg = 'analysis:limits_error';
const info = {
'error-message': err.message,
'error-code': err.code,
node: node.id(),
type: node.getType(),
queued: node.didQueueWork(),
username: node.getOwner()
};

this.logger.info(info, msg);
};

AnalysisLogger.prototype.reopenFileStreams = function () {
Expand Down
20 changes: 10 additions & 10 deletions lib/node/limits.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ var estimates = require('../limits/estimates');

function limitNumberOfRows(node, context, limit, callback) {
if (!Number.isFinite(limit.value)) {
context.logError('Limit for number of rows is not defined');
context.logError(new Error('Limit for number of rows is not defined'), node);
return callback(null);
}
estimates.estimatedNumberOfRows(node, context, function(err, outputRows) {
if (err) {
// if estimation is not available don't abort the analysis
context.logError(err);
context.logError(err, node);
err = null;
} else {
if (outputRows > limit.value) {
Expand All @@ -26,12 +26,12 @@ function limitNumberOfRows(node, context, limit, callback) {

function limitInputRows(node, input, context, limit, callback) {
if (!Number.isFinite(limit.value)) {
context.logError('Limit for input rows is not defined');
context.logError(new Error('Limit for input rows is not defined'), node);
return callback(null);
}
estimates.estimatedNumberOfRows(node[input], context, function(err, numRows) {
if (err) {
context.logError(err);
context.logError(err, node);
err = null;
} else {
if (numRows > limit.value) {
Expand All @@ -48,10 +48,10 @@ function limitInputRowsAndAvgGroupedRows(node, input, categoriesData, context, l
var limitPresent = Number.isFinite(limit.value);
var groupedLimitPresent = Number.isFinite(groupedLimit.value);
if (!limitPresent && !groupedLimitPresent) {
context.logError('limitInputRowsAndAvgGroupedRows called without any passed limit');
context.logError(new Error('limitInputRowsAndAvgGroupedRows called without any passed limit'), node);
return callback(null);
} else if (groupedLimitPresent && !_verifyCategoryData(categoriesData)) {
context.logError('Categories data must contain source and column name keys');
context.logError(new Error('Categories data must contain source and column name keys'), node);
return callback(null);
}

Expand All @@ -73,10 +73,10 @@ function limitInputRowsAndAvgGroupedRows(node, input, categoriesData, context, l
function limitInputRowsMultipliedByCategories(node, input, categoryTarget, context, limit, callback) {
var limitPresent = Number.isFinite(limit.value);
if (!limitPresent) {
context.logError('Limit for input rows multiplied by categories is not defined');
context.logError(new Error('Limit for input rows multiplied by categories is not defined'), node);
return callback(null);
} else if (!_verifyCategoryData(categoryTarget)) {
context.logError('Categories data must contain source and column name keys');
context.logError(new Error('Categories data must contain source and column name keys'), node);
return callback(null);
}

Expand All @@ -96,7 +96,7 @@ function limitInputRowsMultipliedByCategories(node, input, categoryTarget, conte
function _limitInputRowsWithCategories(node, input, categoryTarget, context, checkFunction, callback) {
estimates.estimatedNumberOfRows(node[input], context, function(err, inputRows) {
if (err) {
context.logError(err);
context.logError(err, node);
// something went wrong... lack of stats? anyway, we'll let the analysis pass
return callback(null);
} else {
Expand All @@ -106,7 +106,7 @@ function _limitInputRowsWithCategories(node, input, categoryTarget, context, che
node[categoryTarget.source], context, categoryTarget.column,
function(err, num) {
if (err) {
context.logError(err);
context.logError(err, node);
} else {
numCategories = num;
}
Expand Down