Merge pull request #5 from sigmacomputing/wes/optimize-override-tags

Optimize overrideTags for objects

Author: wesmoncrief

.github/workflows/node.js.yml

@@ -15,7 +15,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, windows-latest]
-        node-version: [10.x, 12.x, 14.x, 16.x, 18.x]
+        node-version: [18.x]
         # See supported Node.js release schedule at https://nodejs.org/en/about/releases/
     runs-on: ${{ matrix.os }}
     steps:

lib/helpers.js

@@ -27,8 +27,10 @@ function formatTags(tags, telegraf) {
 /**
  * Overrides tags in parent with tags from child with the same name (case sensitive) and return the result as new
  * array. parent and child are not mutated.
+ *
+ * OverrideTags2 is faster, but it requires that the parent/child are both objects, not arrays.
  */
-function overrideTags (parent, child, telegraf) {
+function overrideTagsUnoptimized (parent, child, telegraf) {
   const seenKeys = {};
   const toAppend = [];
 
@@ -61,6 +63,58 @@ function overrideTags (parent, child, telegraf) {
   return toAppend.length > 0 ? tags.concat(toAppend) : tags;
 }
 
+/**
+ * Overrides tags in parent with tags from child with the same name (case sensitive) and return the result as a string.
+ */
+function overrideTagsToStringUnoptimized(parent, child, telegraf, separator) {
+  const tags = overrideTagsUnoptimized(parent, child, telegraf);
+  return tags.join(separator);
+}
+
+/**
+ * Overrides tags in parent with tags from child with the same name (case sensitive) and return the result as a string.
+ *
+ * More performant than overrideTagsUnoptimized. But it requires that globalTags and child are both objects, not arrays.
+ */
+function overrideTags2 (globalTags, globalTagsFullMemo, child, telegraf, separator) {
+  let result = '';
+  const usedGlobalKeys = new Set();
+  // eslint-disable-next-line require-jsdoc
+  function addToResult(tag, addToFront) {
+    if (result === '') {
+      result += tag;
+    }
+    // this is just here to match the behavior of overrideTagsUnoptimized
+    else if (addToFront) {
+        result = tag + separator + result;
+      }
+      else {
+        result += separator + tag;
+      }
+  }
+  for (const c of Object.keys(child)) {
+    // there is a global tag with the same name as the child tag - use child
+    const formattedChildKey = sanitizeTags(c);
+    if (Object.hasOwn(globalTags, formattedChildKey)) {
+      usedGlobalKeys.add(formattedChildKey);
+    }
+    const serializedTagWithValue = `${formattedChildKey}:${sanitizeTags(child[c], telegraf)}`;
+    addToResult(serializedTagWithValue);
+  }
+  if (usedGlobalKeys.size === 0) {
+    addToResult(globalTagsFullMemo, true);
+  }
+  else {
+    for (const g of Object.keys(globalTags)) {
+      if (!usedGlobalKeys.has(g)) {
+        const serializedTagWithValue = `${g}:${sanitizeTags(globalTags[g], telegraf)}`;
+        addToResult(serializedTagWithValue, true);
+      }
+    }
+  }
+  return result;
+}
+
 /**
  * Formats a date for use with DataDog
  */
@@ -112,7 +166,9 @@ function getDefaultRoute() {
 
 module.exports = {
   formatTags: formatTags,
-  overrideTags: overrideTags,
+  overrideTagsUnoptimized: overrideTagsUnoptimized,
+  overrideTagsToStringUnoptimized: overrideTagsToStringUnoptimized,
+  overrideTags2: overrideTags2,
   formatDate: formatDate,
   getDefaultRoute: getDefaultRoute,
   sanitizeTags: sanitizeTags

lib/statsFunctions.js

@@ -250,7 +250,7 @@ function applyStatsFns (Client) {
 
     let mergedTags = this.globalTags;
     if (tags && typeof(tags) === 'object') {
-      mergedTags = helpers.overrideTags(mergedTags, tags, this.telegraf);
+      mergedTags = helpers.overrideTagsUnoptimized(mergedTags, tags, this.telegraf);
     }
     if (mergedTags.length > 0) {
       check.push(`#${mergedTags.join(',')}`);

lib/statsd.js

@@ -65,8 +65,12 @@ const Client = function (host, port, prefix, suffix, globalize, cacheDns, mock,
   this.prefix = options.prefix || '';
   this.suffix = options.suffix || '';
   this.tagPrefix = options.tagPrefix || '#';
-  this.tagSeparator = options.tagSeparator || ',';
+  this.telegraf = options.telegraf || false;
+  this.tagSeparator = this.telegraf ? ',' : options.tagSeparator || ',';
   this.mock        = options.mock;
+  /**
+   * this is always an array. It's always populated.
+   */
   this.globalTags  = typeof options.globalTags === 'object' ?
       helpers.formatTags(options.globalTags, options.telegraf) : [];
   const availableDDEnvs = Object.keys(DD_ENV_GLOBAL_TAGS_MAPPING).filter(key => process.env[key]);
@@ -75,7 +79,21 @@ const Client = function (host, port, prefix, suffix, globalize, cacheDns, mock,
       filter((item) => !availableDDEnvs.some(env => item.startsWith(`${DD_ENV_GLOBAL_TAGS_MAPPING[env]}:`))).
       concat(availableDDEnvs.map(env => `${DD_ENV_GLOBAL_TAGS_MAPPING[env]}:${helpers.sanitizeTags(process.env[env])}`));
   }
-  this.telegraf = options.telegraf || false;
+  /**
+   * This is an object or `null`. It's populated if the original input is an object.
+   * We can't necessarily convert an array to an object, since array supports keys without values
+   */
+  this.globalTagsObjectSanitized = null;
+  if (typeof options.globalTags === 'object' && !Array.isArray(options.globalTags)) {
+    this.globalTagsObjectSanitized = {};
+    for (const key of Object.keys(options.globalTags)) {
+     const value =  options.globalTags[key];
+     const sanitizedKey = helpers.sanitizeTags(key);
+     const sanitizedVal = helpers.sanitizeTags(value);
+     this.globalTagsObjectSanitized[sanitizedKey] = sanitizedVal;
+    }
+  }
+  this.globalTagsMemo = helpers.overrideTagsToStringUnoptimized(this.globalTags, [], this.telegraf, this.tagSeparator);
   this.maxBufferSize = options.maxBufferSize || 0;
   this.sampleRate = options.sampleRate || 1;
   this.bufferFlushInterval = options.bufferFlushInterval || 1000;
@@ -242,22 +260,30 @@ Client.prototype.sendStat = function (stat, value, type, sampleRate, tags, callb
  * @param callback {Function=} Callback when message is done being delivered (only if maxBufferSize == 0). Optional.
  */
 Client.prototype.send = function (message, tags, callback) {
-  let mergedTags = this.globalTags;
-  if (tags && typeof tags === 'object') {
-    mergedTags = helpers.overrideTags(mergedTags, tags, this.telegraf);
-  }
-  if (mergedTags.length > 0) {
+  const tagsString = getTagString(this.globalTags, this.globalTagsObjectSanitized, this.globalTagsMemo, tags, this.telegraf, this.tagSeparator);
+  if (tagsString.length > 0) {
     if (this.telegraf) {
       message = message.split(':');
-      message = `${message[0]},${mergedTags.join(',').replace(/:/g, '=')}:${message.slice(1).join(':')}`;
+      message = `${message[0]},${tagsString.replace(/:/g, '=')}:${message.slice(1).join(':')}`;
     } else {
-      message += `|${this.tagPrefix}${mergedTags.join(this.tagSeparator)}`;
+      message += `|${this.tagPrefix}${tagsString}`;
     }
   }
 
   this._send(message, callback);
 };
 
+// eslint-disable-next-line require-jsdoc
+function getTagString(globalTags, globalTagsObject, globalTagsMemo, tags, telegraf, tagSeparator) {
+  if (!(tags && typeof tags === 'object')) {
+    return globalTagsMemo;
+  }
+  if (globalTagsObject !== null && tags && !Array.isArray(tags)) {
+    return helpers.overrideTags2(globalTagsObject, globalTagsMemo, tags, telegraf, tagSeparator);
+  }
+  return helpers.overrideTagsToStringUnoptimized(globalTags, tags, telegraf, tagSeparator);
+}
+
 /**
  * Send a stat or event across the wire
  * @param message {String} The constructed message without tags
@@ -505,7 +531,7 @@ const ChildClient = function (parent, options) {
     mock        : parent.mock,
     // Append child's tags to parent's tags
     globalTags  : typeof options.globalTags === 'object' ?
-        helpers.overrideTags(parent.globalTags, options.globalTags, parent.telegraf) : parent.globalTags,
+      helpers.overrideTagsUnoptimized(parent.globalTags, options.globalTags, parent.telegraf) : parent.globalTags,
     maxBufferSize : parent.maxBufferSize,
     bufferFlushInterval: parent.bufferFlushInterval,
     telegraf    : parent.telegraf,