Skip to content

Commit

Permalink
fix: Correctly log Exceptions
Browse files Browse the repository at this point in the history
Adds bunyan Error serializer (to log {err:err} correctly with stack trace etc.
Fix log messages to include error message in log message
  • Loading branch information
nielm committed Apr 3, 2024
1 parent 3db7ce5 commit 6f5d6a6
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 83 deletions.
4 changes: 2 additions & 2 deletions src/autoscaler-common/counters_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,8 @@ async function tryFlush() {
EXPORTER_PARAMETERS[exporterMode].FLUSH_MAX_ATTEMPTS
} attempts - see OpenTelemetry logging`);
}
} catch (e) {
logger.error('Error while flushing counters', e);
} catch (err) {
logger.error({err: err, message: `Error while flushing counters: ${err}`});
} finally {
// Release any waiters...
resolveFlushInProgress();
Expand Down
1 change: 1 addition & 0 deletions src/autoscaler-common/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const logger = bunyan.createLogger({
streams: [
loggingBunyan.stream(getLogLevel()),
],
serializers: bunyan.stdSerializers,
});

module.exports = {
Expand Down
17 changes: 9 additions & 8 deletions src/forwarder/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ async function forwardFromHTTP(req, res) {
res.status(200).end();
} catch (err) {
logger.error({
message: `An error occurred in the Autoscaler forwarder (HTTP)`,
err: err});
logger.error({message: `JSON payload`, payload: payloadString});
res.status(500).end(err.toString());
message: `An error occurred in the Autoscaler forwarder (HTTP): ${err}`,
err: err,
payload: payloadString,
});
res.status(500).end('An exception occurred');
}
}

Expand All @@ -87,10 +88,10 @@ async function forwardFromPubSub(pubSubEvent, context) {
});
} catch (err) {
logger.error({
message:
`An error occurred in the Autoscaler forwarder (PubSub)`,
err: err});
logger.error({message: `JSON payload`, payload: payload});
message: `An error occurred in the Autoscaler forwarder (PubSub): ${err}`,
err: err,
payload: payload,
});
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ async function main() {
}
} catch (err) {
logger.error({
message: 'Error in unified poller/scaler wrapper:',
err: err});
message: 'Error in unified poller/scaler wrapper: ${err}',
err: err,
});
} finally {
CountersBase.setTryFlushEnabled(true);
await CountersBase.tryFlush();
Expand Down
5 changes: 3 additions & 2 deletions src/poller/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ async function main() {
JSON.stringify(yaml.load(config)));
} catch (err) {
logger.error({
message: 'Error in Poller wrapper:',
err: err});
message: `Error in Poller: ${err}`,
err: err,
});
}
}

Expand Down
67 changes: 40 additions & 27 deletions src/poller/poller-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,10 @@ async function postPubSubMessage(spanner, metrics) {
.catch((err) => {
logger.error({
message: `An error occurred when publishing the message to ${
spanner.scalerPubSubTopic}`,
spanner.scalerPubSubTopic}: $[err}]`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: err,
payload: spanner,
err: err,
});
});
Expand Down Expand Up @@ -337,7 +337,7 @@ async function callScalerHTTP(spanner, metrics) {
})
.catch((err) => {
logger.error({
message: `An error occurred when calling the scaler`,
message: `An error occurred when calling the scaler: ${err}`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: err,
Expand Down Expand Up @@ -469,11 +469,13 @@ async function parseAndEnrichPayload(payload) {
spannersFound.push(spanners[sIdx]);
} catch (err) {
logger.error({
message: `Unable to retrieve Spanner metadata for ${
spanners[sIdx].projectId}/${spanners[sIdx].instanceId}`,
message:
`Unable to retrieve Spanner metadata for ${
spanners[sIdx].projectId}/${spanners[sIdx].instanceId}: ${err}`,
projectId: spanners[sIdx].projectId,
instanceId: spanners[sIdx].instanceId,
payload: err,
err: err,
payload: spanners[sIdx],
});
}
}
Expand Down Expand Up @@ -550,10 +552,11 @@ async function forwardMetrics(forwarderFunction, spanners) {
await Counters.incPollingSuccessCounter(spanner);
} catch (err) {
logger.error({
message: `Unable to retrieve metrics for ${spanner.projectId}/${
spanner.instanceId}`,
message: `Unable to retrieve metrics for ${
spanner.projectId}/${spanner.instanceId}: ${err}`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: spanner,
err: err,
});
await Counters.incPollingFailedCounter(spanner);
Expand All @@ -576,10 +579,11 @@ async function aggregateMetrics(spanners) {
await Counters.incPollingSuccessCounter(spanner);
} catch (err) {
logger.error({
message: `Unable to retrieve metrics for ${spanner.projectId}/${
spanner.instanceId}`,
message: `Unable to retrieve metrics for ${
spanner.projectId}/${spanner.instanceId}: ${err}`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
spanner: spanner,
err: err,
});
await Counters.incPollingFailedCounter(spanner);
Expand Down Expand Up @@ -607,16 +611,20 @@ async function checkSpannerScaleMetricsPubSub(pubSubEvent, context) {
await Counters.incRequestsSuccessCounter();
} catch (err) {
logger.error({
message: `An error occurred in the Autoscaler poller function (PubSub)`,
err: err});
logger.error({message: `JSON payload`, payload: payload});
message:
`An error occurred in the Autoscaler poller function (PubSub): ${
err}`,

Check warning

Code scanning / CodeQL

Improper code sanitization Medium

Code construction depends on an
improperly sanitized value
.
payload: payload,
err: err,
});
await Counters.incRequestsFailedCounter();
}
} catch (err) {
logger.error({
message: `An error occurred in the Autoscaler poller function (PubSub)`,
err: err});
logger.error({message: `Pubsub data`, payload: pubSubEvent.data});
message: `An error occurred parsing pubsub payload: ${err}`,
payload: pubSubEvent.data,
err: err,
});
await Counters.incRequestsFailedCounter();
} finally {
await Counters.tryFlush();
Expand Down Expand Up @@ -646,11 +654,12 @@ async function checkSpannerScaleMetricsHTTP(req, res) {
await Counters.incRequestsSuccessCounter();
} catch (err) {
logger.error({
message: `An error occurred in the Autoscaler poller function (HTTP)`,
err: err});
logger.error({message: `JSON payload`, payload: payload});
res.status(500).end(err.toString());
res.end(err.toString());
message: `An error occurred in the Autoscaler poller function (HTTP): ${
err}`,

Check warning

Code scanning / CodeQL

Improper code sanitization Medium

Code construction depends on an
improperly sanitized value
.
payload: payload,
err: err,
});
res.status(500).contentType('text/plain').end('An Exception occurred');
await Counters.incRequestsFailedCounter();
} finally {
await Counters.tryFlush();
Expand All @@ -673,9 +682,11 @@ async function checkSpannerScaleMetricsJSON(payload) {
} catch (err) {
logger.error({
message:
`An error occurred in the Autoscaler poller function (JSON/HTTP)`,
err: err});
logger.error({message: `JSON payload`, payload: payload});
`An error occurred in the Autoscaler poller function (JSON/HTTP): ${
err}`,

Check warning

Code scanning / CodeQL

Improper code sanitization Medium

Code construction depends on an
improperly sanitized value
.
Code construction depends on an
improperly sanitized value
.
payload: payload,
err: err,
});
await Counters.incRequestsFailedCounter();
} finally {
await Counters.tryFlush();
Expand All @@ -700,9 +711,11 @@ async function checkSpannerScaleMetricsLocal(payload) {
} catch (err) {
logger.error({
message:
`An error occurred in the Autoscaler poller function (JSON/Local)`,
err: err});
logger.error({message: `JSON payload`, payload: payload});
`An error occurred in the Autoscaler poller function (JSON/Local): ${
err}`,

Check warning

Code scanning / CodeQL

Improper code sanitization Medium

Code construction depends on an
improperly sanitized value
.
Code construction depends on an
improperly sanitized value
.
payload: payload,
err: err,
});
await Counters.incRequestsFailedCounter();
} finally {
await Counters.tryFlush();
Expand Down
5 changes: 3 additions & 2 deletions src/scaler/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ function main() {
app.listen(port);
} catch (err) {
logger.error({
message: 'Error in Scaler wrapper:',
err: err});
message: 'Error startting Scaler: ${err}',
err: err,
});
}
}

Expand Down
46 changes: 13 additions & 33 deletions src/scaler/scaler-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,18 +319,11 @@ async function processScalingRequest(spanner, autoscalerState) {
} catch (err) {
logger.error({
message: `----- ${spanner.projectId}/${
spanner.instanceId}: Unsuccessful scaling attempt.`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: err,
err: err,
});
logger.error({
message:
`----- ${spanner.projectId}/${spanner.instanceId}: Spanner payload:`,
spanner.instanceId}: Unsuccessful scaling attempt: ${err}.`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: spanner,
err: err,
});
eventType = 'SCALING_FAILURE';
await Counters.incScalingFailedCounter(
Expand Down Expand Up @@ -372,22 +365,17 @@ async function scaleSpannerInstancePubSub(pubSubEvent, context) {
await Counters.incRequestsSuccessCounter();
} catch (err) {
logger.error({
message: `Failed to process scaling request\n`,
message: `Failed to process scaling request: ${err}`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: spanner,
});
logger.error({
message: `Exception\n`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
err: err,
});
await Counters.incRequestsFailedCounter();
}
} catch (err) {
logger.error({
message: `Failed to parse pubSub scaling request\n`,
message: `Failed to parse pubSub scaling request: ${err}`,
payload: pubSubEvent.data,
err: err,
});
Expand Down Expand Up @@ -416,13 +404,17 @@ async function scaleSpannerInstanceHTTP(req, res) {
res.status(200).end();
await Counters.incRequestsSuccessCounter();
} catch (err) {
console.error(err);
res.status(500).end(err.toString());
logger.error({
message: `Failed to process scaling request: ${err}`,
payload: payload,
err: err,
});
res.status(500).contentType('text/plain').end('An Exception occurred');
await Counters.incRequestsFailedCounter();
}
} catch (err) {
logger.error({
message: `Failed to parse http scaling request\n`,
message: `Failed to parse http scaling request: ${err}`,
err: err,
});
await Counters.incRequestsFailedCounter();
Expand All @@ -449,16 +441,10 @@ async function scaleSpannerInstanceJSON(req, res) {
await Counters.incRequestsSuccessCounter();
} catch (err) {
logger.error({
message: `Failed to process scaling request\n`,
message: `Failed to process scaling request: ${err}`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: spanner,
});
logger.error({
message: `Exception\n`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: err,
err: err,
});
res.writeHead(500, {
Expand All @@ -484,16 +470,10 @@ async function scaleSpannerInstanceLocal(spanner) {
await Counters.incRequestsSuccessCounter();
} catch (err) {
logger.error({
message: `Failed to process scaling request\n`,
message: `Failed to process scaling request: ${err}`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: spanner,
});
logger.error({
message: `Exception\n`,
projectId: spanner.projectId,
instanceId: spanner.instanceId,
payload: err,
err: err,
});
} finally {
Expand Down
7 changes: 5 additions & 2 deletions src/scaler/scaler-core/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,11 @@ class StateFirestore extends State {
await oldDocRef.delete();
}
return snapshot;
} catch (e) {
logger.error(e, `Failed to migrate docpaths`);
} catch (err) {
logger.error({
message: `Failed to migrate docpaths: ${err}`,
err: err,
});
}
return null;
}
Expand Down
15 changes: 10 additions & 5 deletions src/scaler/scaler-core/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,19 @@ async function publishProtoMsgDownstream(eventName, jsonData, topicId) {
const data = Buffer.from(JSON.stringify(message.toJSON()));
const attributes = {event: eventName};

return topic.publishMessage({data: data, attributes: attributes})
.then( () => logger.info(
`Published ${eventName} message downstream to topic: ${topicId}`))
return topic
.publishMessage({data: data, attributes: attributes})
.then(() =>
logger.info(
`Published ${eventName} message downstream to topic: ${topicId}`,
),
)
.catch((err) => {
logger.error({
message: `An error occurred publishing ${
eventName} message downstream to topic: ${topicId}`,
err: err});
eventName} message downstream to topic: ${topicId}: ${err}`,
err: err,
});
});
}

Expand Down

0 comments on commit 6f5d6a6

Please sign in to comment.