Set startTimestamp to creation time of cumulative view (#273)

* Set startTimestamp to creation time of cumulative view * fix review comments
census-instrumentation · Jan 11, 2019 · 90f1905 · 90f1905
1 parent 7af93bb
commit 90f1905
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 18 deletions.
diff --git a/packages/opencensus-core/src/common/time-util.ts b/packages/opencensus-core/src/common/time-util.ts
@@ -59,6 +59,20 @@ export function getTimestampWithProcessHRTime(): Timestamp {
   return {seconds, nanos};
 }
 
+/**
+ * Creates a new timestamp from the given milliseconds.
+ *
+ * @param {number} epochMilli the timestamp represented in milliseconds since
+ *  epoch.
+ * @returns {Timestamp} new timestamp with specified fields.
+ */
+export function timestampFromMillis(epochMilli: number): Timestamp {
+  return {
+    seconds: Math.floor(epochMilli / MILLIS_PER_SECOND),
+    nanos: (epochMilli % MILLIS_PER_SECOND) * NANOS_PER_MILLI
+  };
+}
+
 setHrtimeReference();
 
 export const TEST_ONLY = {

diff --git a/packages/opencensus-core/src/stats/stats.ts b/packages/opencensus-core/src/stats/stats.ts
@@ -145,7 +145,7 @@ export class Stats {
 
     for (const measureName of Object.keys(this.registeredViews)) {
       for (const view of this.registeredViews[measureName]) {
-        metrics.push(view.getMetric());
+        metrics.push(view.getMetric(view.startTime));
       }
     }
 

diff --git a/packages/opencensus-core/src/stats/types.ts b/packages/opencensus-core/src/stats/types.ts
@@ -118,7 +118,7 @@ export interface View {
   /** Gets the view's tag keys */
   getColumns(): string[];
   /** Gets view`s metric */
-  getMetric(): Metric;
+  getMetric(start: number): Metric;
 }
 
 /**

diff --git a/packages/opencensus-core/src/stats/view.ts b/packages/opencensus-core/src/stats/view.ts
@@ -15,7 +15,7 @@
  */
 
 import * as defaultLogger from '../common/console-logger';
-import {getTimestampWithProcessHRTime} from '../common/time-util';
+import {getTimestampWithProcessHRTime, timestampFromMillis} from '../common/time-util';
 import * as loggerTypes from '../common/types';
 import {DistributionValue, LabelValue, Metric, MetricDescriptor, MetricDescriptorType, Point, TimeSeries, Timestamp} from '../metrics/export/types';
 
@@ -200,9 +200,10 @@ export class BaseView implements View {
 
   /**
    * Gets view`s metric
+   * @param start The start timestamp in epoch milliseconds
    * @returns {Metric}
    */
-  getMetric(): Metric {
+  getMetric(start: number): Metric {
     const {type} = this.metricDescriptor;
     let startTimestamp: Timestamp;
 
@@ -215,8 +216,7 @@ export class BaseView implements View {
         startTimestamp = null;
         break;
       default:
-        // TODO (mayurkale): This should be set when create Cumulative view.
-        startTimestamp = getTimestampWithProcessHRTime();
+        startTimestamp = timestampFromMillis(start);
     }
 
     const timeseries: TimeSeries[] = [];

diff --git a/packages/opencensus-core/test/test-view.ts b/packages/opencensus-core/test/test-view.ts
@@ -199,6 +199,8 @@ describe('BaseView', () => {
     const realHrtimeFn = process.hrtime;
     const realNowFn = Date.now;
     const mockedTime: Timestamp = {seconds: 1450000100, nanos: 1e7};
+    const mockStartTime = 1546540757282;
+    const mockStartTimestamp: Timestamp = {seconds: 1546540757, nanos: 282e6};
     const measurementValues = [1.1, 2.3, 3.2, 4.3, 5.2];
     const buckets = [2, 4, 6];
     const tags: Tags = {testKey1: 'testValue', testKey2: 'testValue'};
@@ -227,7 +229,7 @@ describe('BaseView', () => {
         TEST_ONLY.resetHrtimeFunctionCache();
       });
 
-      const {descriptor, timeseries} = view.getMetric();
+      const {descriptor, timeseries} = view.getMetric(mockStartTime);
 
       describe(
           `Aggregation type: ${aggregationTestCase.aggregationType}`, () => {
@@ -263,8 +265,7 @@ describe('BaseView', () => {
                 assert.ok(startTimestamp);
                 assert.equal(typeof startTimestamp.nanos, 'number');
                 assert.equal(typeof startTimestamp.seconds, 'number');
-                assert.ok(startTimestamp.seconds > 0);
-                assert.ok(startTimestamp.nanos > 0);
+                assert.deepStrictEqual(startTimestamp, mockStartTimestamp);
               });
             }
 
@@ -288,8 +289,8 @@ describe('BaseView', () => {
       }
 
       it('should have point', () => {
-        const {timeseries} = view.getMetric();
-        const [{points}] = timeseries;
+        const {timeseries} = view.getMetric(mockStartTime);
+        const [{points, startTimestamp}] = timeseries;
         assert.ok(points);
         const [point] = points;
         const {timestamp, value} = point;
@@ -306,6 +307,8 @@ describe('BaseView', () => {
           sum: total,
           sumOfSquaredDeviation: 10.427999999999997
         });
+
+        assert.deepStrictEqual(startTimestamp, mockStartTimestamp);
       });
     });
 
@@ -325,7 +328,7 @@ describe('BaseView', () => {
           }
 
           it('should have points', () => {
-            const {timeseries} = view.getMetric();
+            const {timeseries} = view.getMetric(mockStartTime);
             assert.equal(timeseries.length, 2);
             const [{labelValues: labelValues1, points: points1}, {
               labelValues: labelValues2,
@@ -383,8 +386,8 @@ describe('BaseView', () => {
       }
 
       it('should have point', () => {
-        const {timeseries} = view.getMetric();
-        const [{points}] = timeseries;
+        const {timeseries} = view.getMetric(mockStartTime);
+        const [{points, startTimestamp}] = timeseries;
         assert.ok(points);
         const [point] = points;
         const {timestamp, value} = point;
@@ -395,6 +398,7 @@ describe('BaseView', () => {
         assert.equal(timestamp.nanos, mockedTime.nanos);
         assert.equal(typeof value, 'number');
         assert.strictEqual(value, 5);
+        assert.deepStrictEqual(startTimestamp, mockStartTimestamp);
       });
     });
 
@@ -410,8 +414,8 @@ describe('BaseView', () => {
       }
 
       it('should have point', () => {
-        const {timeseries} = view.getMetric();
-        const [{points}] = timeseries;
+        const {timeseries} = view.getMetric(mockStartTime);
+        const [{points, startTimestamp}] = timeseries;
         assert.ok(points);
         const [point] = points;
         const {timestamp, value} = point;
@@ -422,6 +426,7 @@ describe('BaseView', () => {
         assert.equal(timestamp.nanos, mockedTime.nanos);
         assert.equal(typeof value, 'number');
         assert.strictEqual(value, total);
+        assert.deepStrictEqual(startTimestamp, mockStartTimestamp);
       });
     });
 
@@ -435,8 +440,8 @@ describe('BaseView', () => {
       }
 
       it('should have point', () => {
-        const {timeseries} = view.getMetric();
-        const [{points}] = timeseries;
+        const {timeseries} = view.getMetric(mockStartTime);
+        const [{points, startTimestamp}] = timeseries;
         assert.ok(points);
         const [point] = points;
         const {timestamp, value} = point;
@@ -445,6 +450,7 @@ describe('BaseView', () => {
         assert.equal(typeof value, 'number');
         assert.strictEqual(
             value, measurementValues[measurementValues.length - 1]);
+        assert.strictEqual(startTimestamp, undefined);
       });
     });
   });