Skip to content

Commit

Permalink
fix: guard against apm-clients encoding/truncation of trace objects t…
Browse files Browse the repository at this point in the history
…hrowing

We have observed an exception from 'truncate.transaction()' twice,
to my knowledge. I have been unable to reproduce it so far.
A possible side-effect of client._encode throwing in the
apm-client is that it effectively gets wedged ('corked()').

Refs: #1966
  • Loading branch information
trentm committed Dec 10, 2024
1 parent 4d8f0af commit d26a9b8
Showing 1 changed file with 54 additions and 10 deletions.
64 changes: 54 additions & 10 deletions lib/apm-client/http-apm-client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,15 @@ Client.prototype._resetEncodedMetadata = function () {

// This is the only code path that should set `_encodedMetadata`.
this._encodedMetadata = this._encode({ metadata }, Client.encoding.METADATA);
if (!this._encodedMetadata) {
// The APM client cannot function without encoded metadata. Handling this
// could be improved (e.g. log details and disable the APM agent). However,
// this suffices for now as we have never observed a metadata encoding
// failure.
throw new Error(
'could not encode metadata (trace-level logging will include details)',
);
}
this._log.trace(
{ _encodedMetadata: this._encodedMetadata },
'_resetEncodedMetadata',
Expand Down Expand Up @@ -504,6 +513,9 @@ Client.prototype._write = function (obj, enc, cb) {
} else {
const t = process.hrtime();
const chunk = this._encode(obj, enc);
if (!chunk) {
return;
}
this._numEventsEnqueued++;
this._chopper.write(chunk, cb);
this._log.trace(
Expand Down Expand Up @@ -579,12 +591,18 @@ Client.prototype._writeBatch = function (objs, cb) {
const chunks = [];
for (var i = 0; i < objs.length; i++) {
const obj = objs[i];
chunks.push(this._encode(obj.chunk, obj.encoding));
const encoded = this._encode(obj.chunk, obj.encoding);
if (encoded) {
chunks.push(encoded);
}
}
if (chunks.length === 0) {
return;
}
const chunk = chunks.join('');
const encodeTimeMs = deltaMs(t);

this._numEventsEnqueued += objs.length;
this._numEventsEnqueued += chunks.length;
this._chopper.write(chunk, cb);
const fullTimeMs = deltaMs(t);

Expand All @@ -601,7 +619,7 @@ Client.prototype._writeBatch = function (objs, cb) {
{
encodeTimeMs,
fullTimeMs,
numEvents: objs.length,
numEvents: chunks.length,
numBytes: chunk.length,
},
'_writeBatch',
Expand Down Expand Up @@ -687,24 +705,45 @@ Client.prototype._maybeUncork = function () {
};

Client.prototype._encode = function (obj, enc) {
const out = {};
let thing;
let truncFunc;
let outAttr;
switch (enc) {
case Client.encoding.SPAN:
out.span = truncate.span(obj.span, this._conf);
thing = obj.span;
truncFunc = truncate.span;
outAttr = 'span';
break;
case Client.encoding.TRANSACTION:
out.transaction = truncate.transaction(obj.transaction, this._conf);
thing = obj.transaction;
truncFunc = truncate.transaction;
outAttr = 'transaction';
break;
case Client.encoding.METADATA:
out.metadata = truncate.metadata(obj.metadata, this._conf);
thing = obj.metadata;
truncFunc = truncate.metadata;
outAttr = 'metadata';
break;
case Client.encoding.ERROR:
out.error = truncate.error(obj.error, this._conf);
thing = obj.error;
truncFunc = truncate.error;
outAttr = 'error';
break;
case Client.encoding.METRICSET:
out.metricset = truncate.metricset(obj.metricset, this._conf);
thing = obj.metricset;
truncFunc = truncate.metricset;
outAttr = 'metricset';
break;
}

const out = {};
try {
out[outAttr] = truncFunc(thing, this._conf);
} catch (err) {
this._log.trace({ err, thing }, `error encoding "${outAttr}" object`);
return null;
}

return ndjson.serialize(out);
};

Expand Down Expand Up @@ -765,7 +804,6 @@ Client.prototype.lambdaRegisterTransaction = function (trans, awsRequestId) {
{ awsRequestId, traceId: trans.trace_id, transId: trans.id },
'lambdaRegisterTransaction start',
);
var out = this._encode({ transaction: trans }, Client.encoding.TRANSACTION);

const finish = (errOrErrMsg) => {
const durationMs = performance.now() - startTime;
Expand All @@ -784,6 +822,12 @@ Client.prototype.lambdaRegisterTransaction = function (trans, awsRequestId) {
resolve(); // always resolve, never reject
};

var out = this._encode({ transaction: trans }, Client.encoding.TRANSACTION);
if (!out) {
finish('could not encode transaction');
return;
}

// Every `POST /register/transaction` request must set the
// `x-elastic-aws-request-id` header. Instead of creating a new options obj
// each time, we just modify in-place.
Expand Down

0 comments on commit d26a9b8

Please sign in to comment.