Merge pull request #7 from appoptics/6.1.0

6.1.0

package.json

@@ -1,6 +1,6 @@
 {
   "name": "appoptics-bindings",
-  "version": "6.0.0",
+  "version": "6.1.0",
   "description": "Bindings to liboboe for the AppOptics APM agent",
   "author": "Bruce A. MacNaughton <[email protected]>",
   "contributors": [
@@ -31,7 +31,6 @@
     "node-gyp": "^3.8.0"
   },
   "devDependencies": {
-    "chai": "^4.2.0",
-    "mocha": "^3.5.3"
+    "chai": "^4.2.0"
   }
 }

src/bindings.h

@@ -44,19 +44,34 @@ class Metadata : public Napi::ObjectWrap<Metadata> {
 
   // work with existing metadata
   static Napi::Value sampleFlagIsSet(const Napi::CallbackInfo& info);
-  static bool format(oboe_metadata_t*, size_t, char*);
+  static bool format(oboe_metadata_t* md, size_t len, char* buffer, uint flags);
+  const static int ff_none = 0;
+  const static int ff_header = 1;
+  const static int ff_task = 2;
+  const static int ff_op = 4;
+  const static int ff_flags = 8;
+  const static int ff_sample = 16;
+  const static int ff_separators = 32;
+  const static int ff_lowercase = 64;
+
+  // predefined formats
+  const static int fmtHuman = ff_header | ff_task | ff_op | ff_flags | ff_separators | ff_lowercase;
+  const static int fmtLog = ff_task | ff_sample | ff_separators;
+
+  // size needed to format is the size needed + 3 for delimiters to split the parts.
+  const static size_t fmtBufferSize = OBOE_MAX_METADATA_PACK_LEN + 3;
 
   // C++ callable for creating an instance of Metadata
   static Napi::Object NewInstance(Napi::Env, Napi::Value);
 
 private:
   static Napi::FunctionReference constructor;
-  static char* PutHex(uint8_t, char*, char = 'a');
 
 public:
   static Napi::Object Init(Napi::Env, Napi::Object);
 };
 
+
 //
 // Event - work with oboe's oboe_event_t structure.
 //

src/context.cc

@@ -67,14 +67,18 @@ Napi::Value setDefaultSampleRate(const Napi::CallbackInfo& info) {
 // Stringify the current context's metadata structure.
 //
 Napi::Value toString(const Napi::CallbackInfo& info) {
-  char buf[OBOE_MAX_METADATA_PACK_LEN];
+  char buf[Metadata::fmtBufferSize];
   oboe_metadata_t* md = oboe_context_get();
 
   int rc;
 
-  // for now any argument means use our format. maybe accept options/flags?
-  if (info.Length() == 1) {
-    rc = Metadata::format(md, sizeof(buf), buf) ? 0 : -1;
+  if (info.Length() == 1 && info[0].IsNumber()) {
+    int fmt = info[0].As<Napi::Number>().Int64Value();
+    // default 1 to a human readable form for historical reasons.
+    if (fmt == 1) {
+      fmt = Metadata::fmtHuman;
+    }
+    rc = Metadata::format(md, sizeof(buf), buf, fmt);
   } else {
     rc = oboe_metadata_tostr(md, buf, sizeof(buf) - 1);
   }

src/event.cc

@@ -118,12 +118,17 @@ Napi::Object Event::NewInstance(Napi::Env env, oboe_metadata_t* omd, bool edge)
 //
 Napi::Value Event::toString(const Napi::CallbackInfo& info) {
     oboe_metadata_t* md = &this->event.metadata;
-    char buf[OBOE_MAX_METADATA_PACK_LEN];
+    char buf[Metadata::fmtBufferSize];
 
     int rc;
-    // TODO consider accepting an argument to select upper or lower case.
-    if (info.Length() == 1 && info[0].ToBoolean().Value()) {
-      rc = Metadata::format(md, sizeof(buf), buf) ? 0 : -1;
+
+    if (info.Length() == 1 && info[0].IsNumber()) {
+      int fmt = info[0].As<Napi::Number>().Int64Value();
+      // default 1 to a human readable form for historical reasons.
+      if (fmt == 1) {
+        fmt = Metadata::fmtHuman;
+      }
+      rc = Metadata::format(md, sizeof(buf), buf, fmt) ? 0 : -1;
     } else {
       rc = oboe_metadata_tostr(md, buf, sizeof(buf) - 1);
     }

src/metadata.cc

@@ -176,15 +176,25 @@ Napi::Value Metadata::sampleFlagIsSet(const Napi::CallbackInfo& info) {
 //
 Napi::Value Metadata::toString(const Napi::CallbackInfo& info) {
   Napi::Env env = info.Env();
-  char buf[OBOE_MAX_METADATA_PACK_LEN];
+  char buf[Metadata::fmtBufferSize];
 
   int rc;
-  // for now any argument means use our delimited, lowercase format.
-  // TODO BAM consider making the args control upper/lower, other?
-  if (info.Length() >= 1) {
-    rc = format(&this->metadata, sizeof(buf), buf) ? 0 : -1;
-  } else {
+  // no args is non-human-readable form - no delimiters, uppercase
+  // if arg == 1 it's the original human readable form.
+  // otherwise arg is a bitmask of options.
+  if (info.Length() == 0) {
     rc = oboe_metadata_tostr(&this->metadata, buf, sizeof(buf) - 1);
+  } else {
+    int style = info[0].ToNumber().Int64Value();
+    int flags;
+    // make style 1 the previous default because ff_header alone is not very useful.
+    if (style == 1) {
+      flags = Metadata::ff_header | Metadata::ff_task | Metadata::ff_op | Metadata::ff_flags | Metadata::ff_separators | Metadata::ff_lowercase;
+    } else {
+      // the style are the flags and the separator is a dash.
+      flags = style;
+    }
+    rc = format(&this->metadata, sizeof(buf), buf, flags) ? 0 : -1;
   }
 
   return Napi::String::New(env, rc == 0 ? buf : "");
@@ -247,11 +257,15 @@ Napi::Object Metadata::Init(Napi::Env env, Napi::Object exports) {
   Napi::HandleScope scope(env);
 
   Napi::Function ctor = DefineClass(env, "Metadata", {
-    // Statics
+    // Static methods
     StaticMethod("fromString", &Metadata::fromString),
     StaticMethod("fromContext", &Metadata::fromContext),
     StaticMethod("makeRandom", &Metadata::makeRandom),
     StaticMethod("sampleFlagIsSet", &Metadata::sampleFlagIsSet),
+    // Static values
+    StaticValue("fmtHuman", Napi::Number::New(env, Metadata::fmtHuman)),
+    StaticValue("fmtLog", Napi::Number::New(env, Metadata::fmtLog)),
+
     // Instance methods (prototype)
     InstanceMethod("isValid", &Metadata::isValid),
     InstanceMethod("getSampleFlag", &Metadata::getSampleFlag),
@@ -267,50 +281,86 @@ Napi::Object Metadata::Init(Napi::Env env, Napi::Object exports) {
 }
 
 //
-// function to format an x-trace with components split by ':'
+// function to format an x-trace with components split by sep.
 //
-bool Metadata::format(oboe_metadata_t* md, size_t len, char* buffer) {
+bool Metadata::format(oboe_metadata_t* md, size_t len, char* buffer, uint flags) {
     char* b = buffer;
-
-    // length counts prefix chars, flag chars, 3 colons and null termination
+    char base = (flags & Metadata::ff_lowercase ? 'a' : 'A') - 10;
+    const char sep = '-';
+
+    auto puthex = [&b, base](uint8_t byte) {
+      int digit = (byte >> 4);
+      digit += digit <= 9 ? '0' : base;
+      *b++ = (char) digit;
+      digit = byte & 0xF;
+      digit += digit <= 9 ? '0' : base;
+      *b++ = (char) digit;
+
+      return b;
+    };
+
+    // make sure there is enough room in the buffer.
+    // prefix + task + op + flags + 3 colons + null terminator.
     if (2 + md->task_len + md->op_len + 2 + 4 > len) return false;
 
-    // fix up the first byte (because it's not 2B in memory)
-    int task_bits = (md->task_len >> 2) - 1;
-    if (task_bits == 4) task_bits = 3;
-    uint8_t packed = md->version << 4 | ((md->op_len >> 2) - 1) << 3 | task_bits;
-    b = PutHex(packed, b);
-    *b++ = ':';
+    const bool separators = flags & Metadata::ff_separators;
+
+    if (flags & Metadata::ff_header) {
+      if (md->version == 2) {
+        // the encoding of the task and op lengths is kind of silly so
+        // skip it if version two. it seems like using the whole byte for
+        // a major/minor version then tying the length to the version makes
+        // more sense. what's the point of the version if it's not used to
+        // make decisions in the code?
+        b = puthex(0x2b);
+      } else {
+        // fix up the first byte using arcane length encoding rules.
+        uint task_bits = (md->task_len >> 2) - 1;
+        if (task_bits == 4) task_bits = 3;
+        uint8_t packed = md->version << 4 | ((md->op_len >> 2) - 1) << 3 | task_bits;
+        b = puthex(packed);
+      }
+      // only add a separator if more fields will be output.
+      const uint more = Metadata::ff_task | Metadata::ff_op | Metadata::ff_flags | Metadata::ff_sample;
+      if (flags & more && separators) {
+        *b++ = sep;
+      }
+    }
+
+    // place to hold pointer to each byte of task and/or op ids.
+    uint8_t* mdp;
 
     // put the task ID
-    uint8_t* mdp = (uint8_t *) &md->ids.task_id;
-    for(unsigned int i = 0; i < md->task_len; i++) {
-        b = PutHex(*mdp++, b);
+    if (flags & Metadata::ff_task) {
+      mdp = (uint8_t *) &md->ids.task_id;
+      for(unsigned int i = 0; i < md->task_len; i++) {
+          b = puthex(*mdp++);
+      }
+      if (flags & (Metadata::ff_op | Metadata::ff_flags | Metadata::ff_sample) && separators) {
+        *b++ = sep;
+      }
     }
-    *b++ = ':';
 
     // put the op ID
-    mdp = (uint8_t *) &md->ids.op_id;
-    for (unsigned int i = 0; i < md->op_len; i++) {
-        b = PutHex(*mdp++, b);
+    if (flags & Metadata::ff_op) {
+      mdp = (uint8_t *) &md->ids.op_id;
+      for (unsigned int i = 0; i < md->op_len; i++) {
+          b = puthex(*mdp++);
+      }
+      if (flags & (Metadata::ff_flags | Metadata::ff_sample) && separators) {
+        *b++ = sep;
+      }
     }
-    *b++ = ':';
 
-    // put the flags byte and null terminate the string.
-    b = PutHex(md->flags, b);
+    // put the flags byte or sample flag only. flags, if specified, takes precedence over sample.
+    if (flags & Metadata::ff_flags) {
+      b = puthex(md->flags);
+    } else if (flags & Metadata::ff_sample) {
+      *b++ = (md->flags & 1) + '0';
+    }
+
+    // null terminate the string
     *b = '\0';
 
     return true;
 }
-
-char* Metadata::PutHex(uint8_t byte, char *p, char base) {
-    base -= 10;
-    int digit = (byte >> 4);
-    digit += digit <= 9 ? '0' : base;
-    *p++ = (char) digit;
-    digit = byte & 0xF;
-    digit += digit <= 9 ? '0' : base;
-    *p++ = (char) digit;
-
-    return p;
-}