chore: add type definitions for exporter-jaeger (#93)

The JaegerTraceExporterOptions accepts `tags` as a Tag array, but was
processing those tags for the process information as if they were an object /
Record instead of a Tag array. This was leading to the process key-value
inside Jaeger to be the array index as key and the value as the key-value
object. We now use the proper key-value information from the input to place
into the process tags.

There were two approaches we could have taken here: 1. To leave the input
as a Tag array and process the input properly in the code or 2. To change
the input to a key-value record / object (Record<string, TagValue), which
would be handled properly by the existing code. We have chosen approach #1
because a Jaeger process tag CAN support multiple tags with the same key
name, which would not be supported in approach #2. There is a chance that
(because we were not previously exporting typescript types) users were
passing in a Record format instead of Array, and thus this might be seen
as a breaking change.

A test has been added to validate the process information to ensure proper
format.

Also included are sticter typing for all thrift types, removing all `any`
references and replacing with valid types.

Author: justindsmith

packages/opencensus-exporter-jaeger/src/jaeger-driver/index.ts

@@ -30,32 +30,77 @@ export const Utils = requireJaegerClientModule('util');
 // tslint:disable-next-line:variable-name
 export const ThriftUtils = requireJaegerClientModule('thrift');
 
+export type TagValue = string|number|boolean;
+
 export type Tag = {
   key: string,
-  // tslint:disable-next-line:no-any
-  value: any
+  value: TagValue
 };
 
-export type Process = {
-  serviceName: string,
-  tags: Tag[]
+export type Log = {
+  timestamp: number,
+  fields: Tag[]
 };
 
 export type SenderCallback = (numSpans: number, err?: string) => void;
 
+export type ThriftProcess = {
+  serviceName: string,
+  tags: ThriftTag[]
+};
+
+export type ThriftTag = {
+  key: string,
+  vType: string,
+  vStr: string,
+  vDouble: number,
+  vBool: boolean
+};
+
+export type ThriftLog = {
+  timestamp: number,
+  fields: ThriftTag[]
+};
+
+export enum ThriftReferenceType {
+  CHILD_OF = 'CHILD_OF',
+  FOLLOWS_FROM = 'FOLLOWS_FROM'
+}
+
+export type ThriftReference = {
+  traceIdLow: Buffer,
+  traceIdHigh: Buffer,
+  spanId: Buffer,
+  refType: ThriftReferenceType
+};
+
+export type ThriftSpan = {
+  traceIdLow: Buffer,
+  traceIdHigh: Buffer,
+  spanId: Buffer,
+  parentSpanId: string|Buffer,
+  operationName: string,
+  references: ThriftReference[],
+  flags: number,
+  startTime: number,  // milliseconds
+  duration: number,   // milliseconds
+  tags: ThriftTag[],
+  logs: ThriftLog[],
+};
+
 /**
  * Translate opencensus Span to Jeager Thrift Span
  * @param span
  */
-export function spanToThrift(span: Span) {
-  const tags = [];
+export function spanToThrift(span: Span): ThriftSpan {
+  const tags: Tag[] = [];
   if (span.attributes) {
     Object.keys(span.attributes).forEach(key => {
       tags.push({'key': key, 'value': span.attributes[key]});
     });
   }
 
-  const logs = [];
+  const logs: Log[] = [];
   if (span.messageEvents) {
     span.messageEvents.forEach(msg => {
       logs.push({
@@ -70,7 +115,7 @@ export function spanToThrift(span: Span) {
 
   if (span.annotations) {
     span.annotations.forEach(ann => {
-      const tags = [];
+      const tags: Tag[] = [];
       Object.keys(ann.attributes).forEach(key => {
         tags.push({'key': key, 'value': ann.attributes[key]});
       });
@@ -83,16 +128,17 @@ export function spanToThrift(span: Span) {
   }
 
   const unsigned = true;
-  const parentSpan = span.parentSpanId ? Utils.encodeInt64(span.parentSpanId) :
-                                         ThriftUtils.emptyBuffer;
+  const parentSpan: string|Buffer = span.parentSpanId ?
+      Utils.encodeInt64(span.parentSpanId) :
+      ThriftUtils.emptyBuffer;
 
   const traceId =
       `00000000000000000000000000000000${span.spanContext.traceId}`.slice(-32);
 
   const high = traceId.slice(0, 16);
   const low = traceId.slice(16);
-  const spanTags = ThriftUtils.getThriftTags(tags);
-  const spanLogs = ThriftUtils.getThriftLogs(logs);
+  const spanTags: ThriftTag[] = ThriftUtils.getThriftTags(tags);
+  const spanLogs: ThriftLog[] = ThriftUtils.getThriftLogs(logs);
 
   return {
     traceIdLow: Utils.encodeInt64(low),

packages/opencensus-exporter-jaeger/src/jaeger.ts

@@ -18,7 +18,7 @@ import {Exporter, ExporterBuffer, ExporterConfig, RootSpan, Span} from '@opencen
 import {logger, Logger} from '@opencensus/core';
 import * as os from 'os';
 
-import {Process, spanToThrift, Tag, ThriftUtils, UDPSender, Utils} from './jaeger-driver';
+import {spanToThrift, Tag, TagValue, ThriftProcess, ThriftUtils, UDPSender, Utils} from './jaeger-driver';
 
 /**
  * Options for Jaeger configuration
@@ -43,7 +43,7 @@ export class JaegerTraceExporter implements Exporter {
   //  ip of the process.
   static readonly PROCESS_IP = 'ip';
 
-  private process: Process;
+  private process: ThriftProcess;
   private logger: Logger;
   sender: typeof UDPSender;
   queue: Span[] = [];
@@ -65,13 +65,17 @@ export class JaegerTraceExporter implements Exporter {
     this.bufferTimeout = options.bufferTimeout;
     this.bufferSize = options.bufferSize;
     this.sender = new UDPSender(options);
-    const tags = options.tags || [];
-    tags[JaegerTraceExporter.JAEGER_OPENCENSUS_EXPORTER_VERSION_TAG_KEY] =
+    const tags: Tag[] = options.tags || [];
+
+    const defaultTags: Record<string, TagValue> = {};
+    defaultTags[JaegerTraceExporter
+                    .JAEGER_OPENCENSUS_EXPORTER_VERSION_TAG_KEY] =
         `opencensus-exporter-jaeger-${pjson.version}`;
-    tags[JaegerTraceExporter.TRACER_HOSTNAME_TAG_KEY] = os.hostname();
-    tags[JaegerTraceExporter.PROCESS_IP] = Utils.ipToInt(Utils.myIp());
+    defaultTags[JaegerTraceExporter.TRACER_HOSTNAME_TAG_KEY] = os.hostname();
+    defaultTags[JaegerTraceExporter.PROCESS_IP] = Utils.ipToInt(Utils.myIp());
 
-    const _tags = Utils.convertObjectToTags(tags);
+    // Merge the user given tags and the default tags
+    const _tags: Tag[] = [...tags, ...Utils.convertObjectToTags(defaultTags)];
     this.process = {
       serviceName: options.serviceName,
       tags: ThriftUtils.getThriftTags(_tags),

packages/opencensus-exporter-jaeger/test/test-jaeger.ts

@@ -27,7 +27,7 @@ import {spanToThrift, ThriftUtils, UDPSender} from '../src/jaeger-driver';
 
 const DEFAULT_BUFFER_TIMEOUT = 10;  // time in milliseconds
 
-import {Process} from '../src/jaeger-driver';
+import {ThriftProcess} from '../src/jaeger-driver';
 import {SenderCallback} from '../src/jaeger-driver';
 
 
@@ -75,6 +75,44 @@ describe('Jaeger Exporter', () => {
     exporter.close();
   });
 
+  describe('exporter configuration', () => {
+    it('should contain process information', () => {
+      const process: ThriftProcess = exporter.sender._process;
+
+      // Service name
+      assert.strictEqual(process.serviceName, 'opencensus-exporter-jaeger');
+
+      // Tags. Validate that both the user-given and default tags are present.
+      let testVersionSeen = false;
+      let testExporterVersionSeen = false;
+      let testHostnameSeen = false;
+      let testProcessIpSeen = false;
+      process.tags.forEach((tag) => {
+        if (tag.key === 'opencensus-exporter-jeager' &&
+            tag.vType === 'STRING' && tag.vStr === '0.0.1') {
+          testVersionSeen = true;
+          return;
+        }
+        if (tag.key ===
+            JaegerTraceExporter.JAEGER_OPENCENSUS_EXPORTER_VERSION_TAG_KEY) {
+          testExporterVersionSeen = true;
+          return;
+        }
+        if (tag.key === JaegerTraceExporter.TRACER_HOSTNAME_TAG_KEY) {
+          testHostnameSeen = true;
+          return;
+        }
+        if (tag.key === JaegerTraceExporter.PROCESS_IP) {
+          testProcessIpSeen = true;
+          return;
+        }
+      });
+      assert.ok(
+          testVersionSeen && testExporterVersionSeen && testHostnameSeen &&
+          testProcessIpSeen);
+    });
+  });
+
   /* Should export spans to Jeager */
   describe('test spans are valid', () => {
     it('should encode as thrift', () => {
@@ -114,7 +152,7 @@ describe('Jaeger Exporter', () => {
           }
         });
 
-        assert.strictEqual(true, testBoolSeen && testStringSeen && testNumSeen);
+        assert.ok(testBoolSeen && testStringSeen && testNumSeen);
 
         assert.strictEqual(thriftSpan.logs.length, 1);
         thriftSpan.logs.forEach((log) => {
@@ -131,7 +169,7 @@ describe('Jaeger Exporter', () => {
               errorSeen = true;
               return;
             }
-            assert.strictEqual(true, descriptionSeen && errorSeen);
+            assert.ok(descriptionSeen && errorSeen);
           });
         });
       });
@@ -174,14 +212,27 @@ describe('Jaeger Exporter', () => {
 
 
 function mockUDPSender(exporter: JaegerTraceExporter) {
+  // Get the process of the current sender and pass to the mock sender. The
+  // process is constructed and attached to the sender at exporter construction
+  // time at initialization time, so there is no way to intercept the process.
+  const process: ThriftProcess = exporter.sender._process;
+
   exporter.sender = new MockedUDPSender();
+  exporter.sender.setProcess(process);
 }
 
 
 class MockedUDPSender extends UDPSender {
-  queue = [];
+  // tslint:disable-next-line:no-any
+  queue: any = [];
 
-  setProcess(process: Process): void {}
+  // Holds the initialized process information. Name matches the associated
+  // UDPSender property.
+  _process: ThriftProcess;
+
+  setProcess(process: ThriftProcess): void {
+    this._process = process;
+  }
 
   // tslint:disable-next-line:no-any
   append(span: any, callback?: SenderCallback): void {

packages/opencensus-exporter-jaeger/tsconfig.json

@@ -1,4 +1,5 @@
 {
+  "extends": "./node_modules/gts/tsconfig-google.json",
   "compilerOptions": {
     "rootDir": ".",
     "outDir": "build",
@@ -14,5 +15,4 @@
     "src/**/*.ts",
     "test/**/*.ts"
   ]
-
 }