fix: guard against apm-clients encoding/truncation of trace objects throwing (#4359)

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

Author: trentm

CHANGELOG.asciidoc

@@ -46,6 +46,11 @@ See the <<upgrade-to-v4>> guide.
 [float]
 ===== Bug fixes
 
+* Guard against a possible encoding error of tracing data in the APM client,
+  before it is sent. It is *possible* this could wedge the APM client,
+  resulting in the APM agent no longer sending tracing data.
+  ({pull}4359[#4359])
+
 [float]
 ===== Chores
 

lib/apm-client/http-apm-client/index.js

@@ -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',
@@ -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(
@@ -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);
 
@@ -601,7 +619,7 @@ Client.prototype._writeBatch = function (objs, cb) {
     {
       encodeTimeMs,
       fullTimeMs,
-      numEvents: objs.length,
+      numEvents: chunks.length,
       numBytes: chunk.length,
     },
     '_writeBatch',
@@ -687,24 +705,54 @@ 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.warn(
+      {
+        err,
+        // Only log full problematic object at TRACE level to limit noise.
+        thing: this._log.isLevelEnabled('trace') ? thing : '[REDACTED]',
+        thing_id: thing?.id,
+        thing_name: thing?.name,
+      },
+      `could not encode "${outAttr}" object`,
+    );
+    return null;
+  }
+
   return ndjson.serialize(out);
 };
 
@@ -765,7 +813,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;
@@ -784,6 +831,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.