Message events: use counters instead of hexdecimal value (#446)

* Message events: use counters instread of hexdecimal string * Change id to number instead of string to match specs This is mostly used in the internal APIs and it won't impact the end users.
census-instrumentation · Apr 3, 2019 · bd4f1b6 · bd4f1b6
1 parent 0c733be
commit bd4f1b6
Show file tree

Hide file tree

Showing 22 changed files with 69 additions and 103 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ All notable changes to this project will be documented in this file.
 - Add `TagMetadata` that defines the properties associated with a `Tag`.
 - Add HTTP text format serializer to Tag propagation component.
 - Enforce `--strictNullChecks` and `--noUnusedLocals` Compiler Options on [opencensus-core] package.
+- Please note that there is an API breaking change in methods `addMessageEvent()`. The field `id` is now number instead of string.
 
 ## 0.0.9 - 2019-02-12
 - Add Metrics API.

diff --git a/packages/opencensus-core/src/trace/model/no-record/no-record-span-base.ts b/packages/opencensus-core/src/trace/model/no-record/no-record-span-base.ts
@@ -129,7 +129,7 @@ export abstract class NoRecordSpanBase implements types.Span {
 
   /** No-op implementation of this method. */
   addMessageEvent(
-      type: types.MessageEventType, id: string, timestamp = 0,
+      type: types.MessageEventType, id: number, timestamp = 0,
       uncompressedSize?: number, compressedSize?: number) {}
 
   /** No-op implementation of this method. */

diff --git a/packages/opencensus-core/src/trace/model/span-base.ts b/packages/opencensus-core/src/trace/model/span-base.ts
@@ -218,7 +218,7 @@ export abstract class SpanBase implements types.Span {
    *     zero or undefined, assumed to be the same size as uncompressed.
    */
   addMessageEvent(
-      type: types.MessageEventType, id: string, timestamp = 0,
+      type: types.MessageEventType, id: number, timestamp = 0,
       uncompressedSize?: number, compressedSize?: number) {
     if (this.messageEvents.length >=
         this.activeTraceParams.numberOfMessageEventsPerSpan!) {

diff --git a/packages/opencensus-core/src/trace/model/types.ts b/packages/opencensus-core/src/trace/model/types.ts
@@ -191,13 +191,12 @@ export interface MessageEvent {
   /** Indicates whether the message was sent or received. */
   type: MessageEventType;
   /**
-   * An identifier for the MessageEvent's message. This should be a hexadecimal
-   * value that fits within 64-bits. Message event ids should start with 1 for
+   * An identifier for the MessageEvent's message that can be used to match
+   * SENT and RECEIVED MessageEvents. Message event ids should start with 1 for
    * both sent and received messages and increment by 1 for each message
-   * sent/received. See:
-   * https://github.com/census-instrumentation/opencensus-specs/blob/master/trace/gRPC.md#message-events
+   * sent/received.
    */
-  id: string;
+  id: number;
   /** The number of uncompressed bytes sent or received. */
   uncompressedSize?: number;
   /**
@@ -433,7 +432,7 @@ export interface Span {
    *     zero or undefined, assumed to be the same size as uncompressed.
    */
   addMessageEvent(
-      type: MessageEventType, id: string, timestamp?: number,
+      type: MessageEventType, id: number, timestamp?: number,
       uncompressedSize?: number, compressedSize?: number): void;
 
   /**

diff --git a/packages/opencensus-core/test/test-no-record-root-span.ts b/packages/opencensus-core/test/test-no-record-root-span.ts
@@ -28,8 +28,7 @@ describe('NoRecordRootSpan()', () => {
     noRecordRootSpan.addAnnotation(
         'MyAnnotation', {myString: 'bar', myNumber: 123, myBoolean: true});
     noRecordRootSpan.addLink('aaaaa', 'aaa', LinkType.CHILD_LINKED_SPAN);
-    noRecordRootSpan.addMessageEvent(
-        MessageEventType.RECEIVED, 'aaaa', 123456789);
+    noRecordRootSpan.addMessageEvent(MessageEventType.RECEIVED, 1, 123456789);
     noRecordRootSpan.addAttribute('my_first_attribute', 'foo');
     noRecordRootSpan.setStatus(CanonicalCode.OK);
     noRecordRootSpan.startChildSpan();

diff --git a/packages/opencensus-core/test/test-no-record-span.ts b/packages/opencensus-core/test/test-no-record-span.ts
@@ -25,7 +25,7 @@ describe('NoRecordSpan()', () => {
     noRecordSpan.addAnnotation(
         'MyAnnotation', {myString: 'bar', myNumber: 123, myBoolean: true});
     noRecordSpan.addLink('aaaaa', 'aaa', LinkType.CHILD_LINKED_SPAN);
-    noRecordSpan.addMessageEvent(MessageEventType.RECEIVED, 'aaaa', 123456789);
+    noRecordSpan.addMessageEvent(MessageEventType.RECEIVED, 1, 123456789);
     noRecordSpan.addAttribute('my_first_attribute', 'foo');
     noRecordSpan.setStatus(CanonicalCode.OK);
   });

diff --git a/packages/opencensus-core/test/test-root-span.ts b/packages/opencensus-core/test/test-root-span.ts
@@ -271,9 +271,7 @@ describe('RootSpan', () => {
 
       const rootSpan = new RootSpan(tracer, name, kind, traceId, parentSpanId);
       rootSpan.start();
-
-      rootSpan.addMessageEvent(
-          types.MessageEventType.UNSPECIFIED, 'message_event_test_id');
+      rootSpan.addMessageEvent(types.MessageEventType.UNSPECIFIED, 1);
 
       assert.ok(rootSpan.messageEvents.length > 0);
       assert.equal(rootSpan.droppedMessageEventsCount, 0);

diff --git a/packages/opencensus-core/test/test-span.ts b/packages/opencensus-core/test/test-span.ts
@@ -308,14 +308,14 @@ describe('Span', () => {
       span.start();
 
       span.addMessageEvent(
-          types.MessageEventType.UNSPECIFIED, /* id */ '1',
+          types.MessageEventType.UNSPECIFIED, /* id */ 1,
           /* timestamp */ 1550000000000, /* uncompressedSize */ 55,
           /** compressedSize */ 40);
 
       assert.ok(span.messageEvents.length > 0);
       assert.deepEqual(span.messageEvents, [{
                          type: types.MessageEventType.UNSPECIFIED,
-                         id: '1',
+                         id: 1,
                          timestamp: 1550000000000,
                          uncompressedSize: 55,
                          compressedSize: 40,
@@ -331,8 +331,7 @@ describe('Span', () => {
       const span = new Span(rootSpan);
       span.start();
       for (let i = 0; i < 35; i++) {
-        span.addMessageEvent(
-            types.MessageEventType.UNSPECIFIED, 'message_event_test_id');
+        span.addMessageEvent(types.MessageEventType.UNSPECIFIED, 1);
       }
 
       assert.equal(span.messageEvents.length, 32);

diff --git a/packages/opencensus-exporter-ocagent/src/adapters.ts b/packages/opencensus-exporter-ocagent/src/adapters.ts
@@ -179,8 +179,7 @@ const adaptTimeEvents =
           timeEvents.push({
             time: millisToTimestamp(messageEvent.timestamp),
             messageEvent: {
-              // tslint:disable-next-line:ban Needed to parse hexadecimal.
-              id: parseInt(messageEvent.id, 16),
+              id: messageEvent.id,
               type: adaptMessageEventType(messageEvent.type),
               uncompressedSize: messageEvent.uncompressedSize || 0,
               compressedSize: messageEvent.compressedSize || 0

diff --git a/packages/opencensus-exporter-ocagent/test/test-ocagent.ts b/packages/opencensus-exporter-ocagent/test/test-ocagent.ts
@@ -269,13 +269,12 @@ describe('OpenCensus Agent Exporter', () => {
 
       // Message Event
       const timeStamp = 123456789;
-      rootSpan.addMessageEvent(MessageEventType.SENT, 'aaaa', timeStamp);
-      rootSpan.addMessageEvent(
-          MessageEventType.SENT, 'ffff', timeStamp, 100, 12);
-      rootSpan.addMessageEvent(MessageEventType.RECEIVED, 'ffff', timeStamp);
+      rootSpan.addMessageEvent(MessageEventType.SENT, 1, timeStamp);
+      rootSpan.addMessageEvent(MessageEventType.SENT, 2, timeStamp, 100, 12);
+      rootSpan.addMessageEvent(MessageEventType.RECEIVED, 1, timeStamp);
       // Use of `null` is to force a `TYPE_UNSPECIFIED` value
       // tslint:disable-next-line:no-any
-      rootSpan.addMessageEvent(null as any, 'ffff', timeStamp);
+      rootSpan.addMessageEvent(null as any, 2, timeStamp);
 
       // Links
       rootSpan.addLink('aaaaa', 'aaa', LinkType.CHILD_LINKED_SPAN);
@@ -388,7 +387,7 @@ describe('OpenCensus Agent Exporter', () => {
                 {
                   messageEvent: {
                     compressedSize: '12',
-                    id: '65535',
+                    id: 2,
                     type: 'SENT',
                     uncompressedSize: '100'
                   },
@@ -399,7 +398,7 @@ describe('OpenCensus Agent Exporter', () => {
                   value: 'messageEvent',
                   messageEvent: {
                     compressedSize: '0',
-                    id: '65535',
+                    id: 1,
                     type: 'RECEIVED',
                     uncompressedSize: '0'
                   },
@@ -409,7 +408,7 @@ describe('OpenCensus Agent Exporter', () => {
                   value: 'messageEvent',
                   messageEvent: {
                     compressedSize: '0',
-                    id: '65535',
+                    id: 2,
                     type: 'TYPE_UNSPECIFIED',
                     uncompressedSize: '0'
                   },
@@ -493,8 +492,8 @@ describe('OpenCensus Agent Exporter', () => {
 
          // Message Event
          const timeStamp = 123456789;
-         rootSpan.addMessageEvent(MessageEventType.SENT, 'ffff', timeStamp);
-         rootSpan.addMessageEvent(MessageEventType.RECEIVED, 'ffff', timeStamp);
+         rootSpan.addMessageEvent(MessageEventType.SENT, 1, timeStamp);
+         rootSpan.addMessageEvent(MessageEventType.RECEIVED, 1, timeStamp);
 
          // Links
          rootSpan.addLink('ffff', 'ffff', LinkType.CHILD_LINKED_SPAN, {
@@ -577,7 +576,7 @@ describe('OpenCensus Agent Exporter', () => {
                    {
                      messageEvent: {
                        compressedSize: '0',
-                       id: '65535',
+                       id: 1,
                        type: 'SENT',
                        uncompressedSize: '0'
                      },
@@ -588,7 +587,7 @@ describe('OpenCensus Agent Exporter', () => {
                      value: 'messageEvent',
                      messageEvent: {
                        compressedSize: '0',
-                       id: '65535',
+                       id: 1,
                        type: 'RECEIVED',
                        uncompressedSize: '0'
                      },

diff --git a/packages/opencensus-exporter-stackdriver/src/stackdriver-cloudtrace-utils.ts b/packages/opencensus-exporter-stackdriver/src/stackdriver-cloudtrace-utils.ts
@@ -82,7 +82,7 @@ export function createTimeEvents(
         (messageEvent) => ({
           time: new Date(messageEvent.timestamp).toISOString(),
           messageEvent: {
-            id: messageEvent.id,
+            id: String(messageEvent.id),
             type: createMessageEventType(messageEvent.type),
             uncompressedSize: String(messageEvent.uncompressedSize || 0),
             compressedSize: String(messageEvent.compressedSize || 0)

diff --git a/packages/opencensus-exporter-stackdriver/test/test-stackdriver-cloudtrace-utils.ts b/packages/opencensus-exporter-stackdriver/test/test-stackdriver-cloudtrace-utils.ts
@@ -98,17 +98,14 @@ describe('Stackdriver CloudTrace Exporter Utils', () => {
     ];
     const messageEvents: coreTypes.MessageEvent[] = [
       {
-        id: 'aaaa',
+        id: 1,
         timestamp: ts,
         type: coreTypes.MessageEventType.SENT,
         compressedSize: 100,
         uncompressedSize: 12
       },
-      {id: 'ffff', timestamp: ts, type: coreTypes.MessageEventType.RECEIVED}, {
-        id: 'eeee',
-        timestamp: ts,
-        type: coreTypes.MessageEventType.UNSPECIFIED
-      }
+      {id: 1, timestamp: ts, type: coreTypes.MessageEventType.RECEIVED},
+      {id: 1, timestamp: ts, type: coreTypes.MessageEventType.UNSPECIFIED}
     ];
 
     const expectedTimeEvent = [
@@ -147,22 +144,18 @@ describe('Stackdriver CloudTrace Exporter Utils', () => {
         }
       },
       {
-        messageEvent: {
-          compressedSize: '100',
-          id: 'aaaa',
-          type: 1,
-          uncompressedSize: '12'
-        },
+        messageEvent:
+            {compressedSize: '100', id: '1', type: 1, uncompressedSize: '12'},
         time: '1970-01-02T10:17:36.789Z',
       },
       {
         messageEvent:
-            {compressedSize: '0', id: 'ffff', type: 2, uncompressedSize: '0'},
+            {compressedSize: '0', id: '1', type: 2, uncompressedSize: '0'},
         time: '1970-01-02T10:17:36.789Z',
       },
       {
         messageEvent:
-            {compressedSize: '0', id: 'eeee', type: 0, uncompressedSize: '0'},
+            {compressedSize: '0', id: '1', type: 0, uncompressedSize: '0'},
         time: '1970-01-02T10:17:36.789Z',
       }
     ];

diff --git a/packages/opencensus-exporter-zipkin/test/test-zipkin.ts b/packages/opencensus-exporter-zipkin/test/test-zipkin.ts
@@ -117,9 +117,9 @@ describe('Zipkin Exporter', function() {
         span.setStatus(CanonicalCode.RESOURCE_EXHAUSTED, 'RESOURCE_EXHAUSTED');
 
         span.addAnnotation('processing', {}, 1550213104708);
-        span.addMessageEvent(MessageEventType.SENT, '1', 1550213104708);
-        span.addMessageEvent(MessageEventType.RECEIVED, '2', 1550213104708);
-        span.addMessageEvent(MessageEventType.UNSPECIFIED, '3', 1550213104708);
+        span.addMessageEvent(MessageEventType.SENT, 1, 1550213104708);
+        span.addMessageEvent(MessageEventType.RECEIVED, 2, 1550213104708);
+        span.addMessageEvent(MessageEventType.UNSPECIFIED, 3, 1550213104708);
         span.addAnnotation('done', {}, 1550213104708);
         span.end();
         rootSpan.end();

diff --git a/packages/opencensus-instrumentation-grpc/src/grpc.ts b/packages/opencensus-instrumentation-grpc/src/grpc.ts
@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
-import {BasePlugin, CanonicalCode, deserializeBinary, PluginInternalFiles, RootSpan, serializeBinary, Span, SpanContext, SpanKind, TagMap, TagTtl, TraceOptions} from '@opencensus/core';
+import {BasePlugin, CanonicalCode, deserializeBinary, MessageEventType, PluginInternalFiles, RootSpan, serializeBinary, Span, SpanContext, SpanKind, TagMap, TagTtl, TraceOptions} from '@opencensus/core';
 import {deserializeSpanContext, serializeSpanContext} from '@opencensus/propagation-binaryformat';
 import {EventEmitter} from 'events';
 import * as grpcTypes from 'grpc';
 import * as lodash from 'lodash';
 import * as shimmer from 'shimmer';
+
 import * as clientStats from './grpc-stats/client-stats';
 import * as serverStats from './grpc-stats/server-stats';
 
@@ -228,6 +229,7 @@ export class GrpcPlugin extends BasePlugin {
             GrpcPlugin.ATTRIBUTE_GRPC_STATUS_CODE,
             grpcTypes.status.OK.toString());
       }
+      rootSpan.addMessageEvent(MessageEventType.RECEIVED, 1);
 
       // record stats
       const parentTagCtx =
@@ -378,6 +380,7 @@ export class GrpcPlugin extends BasePlugin {
               GrpcPlugin.ATTRIBUTE_GRPC_STATUS_CODE,
               grpcTypes.status.OK.toString());
         }
+        span.addMessageEvent(MessageEventType.SENT, 1);
 
         // record stats: new RPCs on client-side inherit the tag context from
         // the current Context.

diff --git a/packages/opencensus-instrumentation-http/package-lock.json b/packages/opencensus-instrumentation-http/package-lock.json
diff --git a/packages/opencensus-instrumentation-http/package.json b/packages/opencensus-instrumentation-http/package.json
@@ -54,7 +54,6 @@
     "@types/node": "^9.4.7",
     "@types/semver": "^6.0.0",
     "@types/shimmer": "^1.0.1",
-    "@types/uuid": "^3.4.3",
     "codecov": "^3.1.0",
     "gts": "^0.9.0",
     "mocha": "^6.0.0",
@@ -68,7 +67,6 @@
   "dependencies": {
     "@opencensus/core": "^0.0.9",
     "semver": "^6.0.0",
-    "shimmer": "^1.2.0",
-    "uuid": "^3.2.1"
+    "shimmer": "^1.2.0"
   }
 }