From 608c358741b94f5688cf45524cd7d537be10d737 Mon Sep 17 00:00:00 2001
From: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
Date: Wed, 30 Oct 2019 16:26:02 -0400
Subject: [PATCH] Change timestamp data type to unixnano in Metrics Protobuf
 definitions (#33)

* Change timestamp data type to unixnano in Metrics Protobuf definitions

This change applies the refinement approach that is already performed on Traces
Protobuf definitions as part of https://github.com/open-telemetry/oteps/pull/59 and
which proved to yield significant performance improvements.

I replaced google.protobuf.Timestamp by int64 time in unix epoch nanoseconds.

Simple benchmark in Go demonstrates the following improvement of encoding and decoding
compared to the current state:

```
===== Encoded sizes
Encoding                       Uncompressed  Improved        Compressed  Improved
Baseline/MetricOne              20000 bytes  [1.000], gziped 1506 bytes  [1.000]
Proposed/MetricOne              18250 bytes  [1.096], gziped 1433 bytes  [1.051]

Encoding                       Uncompressed  Improved        Compressed  Improved
Baseline/MetricSeries           51797 bytes  [1.000], gziped 6455 bytes  [1.000]
Proposed/MetricSeries           43047 bytes  [1.203], gziped 6093 bytes  [1.059]

goos: darwin
goarch: amd64
pkg: github.com/tigrannajaryan/exp-otelproto/encodings
BenchmarkEncode/Baseline/MetricOne-8         	      30	 186998840 ns/op
BenchmarkEncode/Proposed/MetricOne-8         	      36	 166668705 ns/op

BenchmarkEncode/Baseline/MetricSeries-8      	       8	 632391842 ns/op
BenchmarkEncode/Proposed/MetricSeries-8      	      10	 537384515 ns/op

BenchmarkDecode/Baseline/MetricOne-8         	      16	 348156010 ns/op	171896049 B/op	 4974000 allocs/op
BenchmarkDecode/Proposed/MetricOne-8         	      19	 314727259 ns/op	155096036 B/op	 4624000 allocs/op

BenchmarkDecode/Baseline/MetricSeries-8      	       5	1013035422 ns/op	440696048 B/op	11874000 allocs/op
BenchmarkDecode/Proposed/MetricSeries-8      	       6	 846887981 ns/op	356696040 B/op	10124000 allocs/op
```

It is 10-15% faster and is 10-20% smaller on the wire and in memory.

Benchmarks encode and decode 500 batches of 2 metrics: one int64 Gauge with 5 time series
and one Histogram of doubles with 1 time series and single bucket. Each time series for
both metrics contains either 1 data point (MetricOne) or 5 data points (MetricSeries).
Both metrics have 2 labels.

Benchmark source code is available at:
https://github.com/tigrannajaryan/exp-otelproto/blob/master/encodings/encoding_test.go

* Change timestamp from int64 to sfixed64

* Change timestamp interval from closed to open
---
 opentelemetry/proto/metrics/v1/metrics.proto | 33 +++++++++++++-------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/opentelemetry/proto/metrics/v1/metrics.proto b/opentelemetry/proto/metrics/v1/metrics.proto
index 78f806dc9..1a12371b8 100644
--- a/opentelemetry/proto/metrics/v1/metrics.proto
+++ b/opentelemetry/proto/metrics/v1/metrics.proto
@@ -16,7 +16,6 @@ syntax = "proto3";
 
 package opentelemetry.proto.metrics.v1;
 
-import "google/protobuf/timestamp.proto";
 import "google/protobuf/wrappers.proto";
 import "opentelemetry/proto/resource/v1/resource.proto";
 
@@ -124,15 +123,24 @@ message LabelValue {
 
 // A timestamped measurement.
 message Point {
-  // Must be present for counter/cumulative metrics. The time when the
-  // cumulative value was reset to zero. The cumulative value is over the time
-  // interval (start_timestamp, timestamp]. If not specified, the backend can
-  // use the previous recorded value.
-  google.protobuf.Timestamp start_timestamp = 1;
-
-  // The moment when this point was recorded.
-  // If not specified, the timestamp will be decided by the backend.
-  google.protobuf.Timestamp timestamp = 2;
+  // start_time_unixnano is the time when the cumulative value was reset to zero.
+  // This is used for Counter type only. For Gauge the value is not specified and
+  // defaults to 0.
+  //
+  // The cumulative value is over the time interval (start_time_unixnano, timestamp_unixnano].
+  // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970.
+  //
+  // Value of 0 indicates that the start_time is the same as that of the previous
+  // data point in this timeseries. When creating timeseries of this type it is recommended
+  // to omit this value if the start_time does not change, since it results in more
+  // compact encoding on the wire.
+  // If the value of 0 occurs for the first data point in the timeseries it means that
+  // the timestamp is unspecified. In that case the timestamp may be decided by the backend.
+  sfixed64 start_time_unixnano = 1;
+
+  // timestamp_unixnano is the moment when this value was recorded.
+  // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970.
+  sfixed64 timestamp_unixnano = 2;
 
   // The actual point value.
   oneof value {
@@ -204,8 +212,9 @@ message HistogramValue {
       // belongs to.
       double value = 1;
 
-      // The observation (sampling) time of the above value.
-      google.protobuf.Timestamp timestamp = 2;
+      // timestamp_unixnano is the moment when this exemplar was recorded.
+      // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970.
+      sfixed64 timestamp_unixnano = 2;
 
       // Contextual information about the example value.
       map<string, string> attachments = 3;