Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comments about StartTimeUnixNano #295

Merged
merged 12 commits into from
May 11, 2021
Merged

Add comments about StartTimeUnixNano #295

Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eda7485
Add comments about StartTimeUnixNano
Apr 22, 2021
7e9029b
Apply suggestions from code review
jmacd Apr 23, 2021
5b6abcb
From review feedback
Apr 26, 2021
03b2d36
more text about unbroken sequence of observations
Apr 26, 2021
3dfeba0
Gauge overlap: delta or cumulative
Apr 27, 2021
fd4d374
Update for gauges
May 3, 2021
bbe3959
Fixes
May 6, 2021
351cfa1
Merge branch 'main' into jmacd/start_time
jmacd May 7, 2021
ad79ff1
Remove detail text as being for the data model spec
May 10, 2021
a8761df
Merge branch 'jmacd/start_time' of github.com:jmacd/opentelemetry-pro…
May 10, 2021
55eb417
Remove 134-154
May 10, 2021
5951df4
Merge branch 'main' into jmacd/start_time
bogdandrutu May 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 68 additions & 76 deletions opentelemetry/proto/metrics/v1/metrics.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ message InstrumentationLibraryMetrics {
repeated Metric metrics = 2;
}

// Defines a Metric which has one or more timeseries.
// Defines a Metric which has one or more timeseries. The following is a
// brief summary of the Metric data model. For more details, see:
//
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/datamodel.md
//
//
// The data model and relation between entities is shown in the
// diagram below. Here, "DataPoint" is the term used to refer to any
Expand All @@ -54,8 +58,8 @@ message InstrumentationLibraryMetrics {
//
// - Metric is composed of a metadata and data.
// - Metadata part contains a name, description, unit.
// - Data is one of the possible types (Gauge, Sum, Histogram, etc.).
// - DataPoint contains timestamps, labels, and one of the possible value type
// - Data is one of the possible types (Sum, Gauge, Histogram, Summary).
// - DataPoint contains timestamps, attributes, and one of the possible value type
// fields.
//
// Metric
Expand Down Expand Up @@ -95,14 +99,37 @@ message InstrumentationLibraryMetrics {
// |+-----+ |
// +---------------------------+
//
// Each distinct type of DataPoint represents the output of a specific
// aggregation function, the result of applying the DataPoint's
// associated function of to one or more measurements.
//
// All DataPoint types have three common fields:
// - Labels zero or more key-value pairs associated with the data point.
// - StartTimeUnixNano MUST be set to the start of the interval when the data's
// type includes an AggregationTemporality. This field is not set otherwise.
// - TimeUnixNano MUST be set to:
// - the moment when an aggregation is reported (independent of the
// aggregation temporality).
// - the instantaneous time of the event.
// - Attributes includes key-value pairs associated with the data point
// - TimeUnixNano is required, set to the end time of the aggregation
// - StartTimeUnixNano is optional, but strongly encouraged for DataPoints
// having an AggregationTemporality field, as discussed below.
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
//
// Both TimeUnixNano and StartTimeUnixNano values are expressed as
// UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970.
//
// # TimeUnixNano
//
// This field is required, having consistent interpretation across
// DataPoint types. TimeUnixNano is the moment corresponding to when
// the data point's aggregate value was captured.
//
// Data points with the 0 value for TimeUnixNano SHOULD be rejected
// by consumers.
//
// # StartTimeUnixNano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interest of both getting this PR in, and ensuring information is easily found, Would It make sense to move the depiction of unbroken sequences and start time synthesis into the DataModel specificaiton (with a link from here) rather than in the proto file?

a. We may evolve/improve the description.
b. This isn't a great medium for delivering "why" content.

My mantra for this file is "What, not why", and for the DataModel specification is "Why then What" (i.e. Justification, then Specification semantics).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK; will do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 @jsuereth also we make sure we don't duplicate specification and get one of them outdated

//
// StartTimeUnixNano in general allows detecting when a sequence of
// observations is unbroken. This field indicates to consumers the
// start time for points with cumulative and delta
// AggregationTemporality, and it should be included whenever possible
// to support correct rate calculation. Although it may be omitted
// when the start time is truly unknown, setting StartTimeUnixNano is
// strongly encouraged.
message Metric {
// name of the metric, including its DNS name prefix. It must be unique.
string name = 1;
Expand Down Expand Up @@ -150,7 +177,7 @@ message Metric {
// IntGauge represents the type of a int scalar metric that always exports the
// "current value" for every data point. It should be used for an "unknown"
// aggregation.
//
//
// A Gauge does not support different aggregation temporalities. Given the
// aggregation is unknown, points cannot be combined using the same
// aggregation, regardless of aggregation temporalities. Therefore,
Expand All @@ -165,7 +192,7 @@ message IntGauge {
// Gauge represents the type of a double scalar metric that always exports the
// "current value" for every data point. It should be used for an "unknown"
// aggregation.
//
//
// A Gauge does not support different aggregation temporalities. Given the
// aggregation is unknown, points cannot be combined using the same
// aggregation, regardless of aggregation temporalities. Therefore,
Expand Down Expand Up @@ -196,7 +223,7 @@ message IntSum {
// as a sum of all reported measurements over a time interval.
message Sum {
repeated NumberDataPoint data_points = 1;

// aggregation_temporality describes if the aggregator reports delta changes
// since last report time, or cumulative changes since a fixed start time.
AggregationTemporality aggregation_temporality = 2;
Expand Down Expand Up @@ -304,7 +331,7 @@ enum AggregationTemporality {
// number of requests received over the interval of time t_1 to
// t_0+1 with a value of 1.
//
// Note: Even though, when reporting changes since last report time, using
// Note: Even though, when reporting changes since last report time, using
// CUMULATIVE is valid, it is not recommended. This may cause problems for
// systems that do not use start_time to determine when the aggregation
// value was reset (e.g. Prometheus).
Expand All @@ -319,22 +346,15 @@ message IntDataPoint {
// The set of labels that uniquely identify this timeseries.
repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1;

// start_time_unix_nano is the last time when the aggregation value was reset
// to "zero". For some metric types this is ignored, see data types for more
// details.
// StartTimeUnixNano is optional but strongly encouraged, see the
// the detiled comments above Metric.
//
// The aggregation value is over the time interval (start_time_unix_nano,
// time_unix_nano].
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
//
// Value of 0 indicates that the timestamp is unspecified. In that case the
// timestamp may be decided by the backend.
fixed64 start_time_unix_nano = 2;

// time_unix_nano is the moment when this aggregation value was reported.
//
// TimeUnixNano is required, see the detailed comments above Metric.
jmacd marked this conversation as resolved.
Show resolved Hide resolved
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
fixed64 time_unix_nano = 3;
Expand All @@ -350,7 +370,7 @@ message IntDataPoint {
// NumberDataPoint is a single data point in a timeseries that describes the
// time-varying value of a double metric.
message NumberDataPoint {
// The set of key/value pairs that uniquely identify the timeseries from
// The set of key/value pairs that uniquely identify the timeseries from
// where this point belongs. The list may be empty (may contain 0 elements).
repeated opentelemetry.proto.common.v1.KeyValue attributes = 7;

Expand All @@ -364,22 +384,15 @@ message NumberDataPoint {
// This field will be removed in ~3 months, on July 1, 2021.
repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1 [deprecated = true];

// start_time_unix_nano is the last time when the aggregation value was reset
// to "zero". For some metric types this is ignored, see data types for more
// details.
// StartTimeUnixNano is optional but strongly encouraged, see the
// the detiled comments above Metric.
//
// The aggregation value is over the time interval (start_time_unix_nano,
// time_unix_nano].
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
//
// Value of 0 indicates that the timestamp is unspecified. In that case the
// timestamp may be decided by the backend.
fixed64 start_time_unix_nano = 2;

// time_unix_nano is the moment when this aggregation value was reported.
//
// TimeUnixNano is required, see the detailed comments above Metric.
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
fixed64 time_unix_nano = 3;
Expand All @@ -388,7 +401,7 @@ message NumberDataPoint {
// value fields is not present inside this oneof.
oneof value {
double as_double = 4;
sfixed64 as_int = 6;
sfixed64 as_int = 6;
}

// (Optional) List of exemplars collected from
Expand All @@ -404,7 +417,7 @@ message NumberDataPoint {
// the distribution of those values across a set of buckets.
//
// If the histogram contains the distribution of values, then both
// "explicit_bounds" and "bucket counts" fields must be defined.
// "explicit_bounds" and "bucket counts" fields must be defined.
// If the histogram does not contain the distribution of values, then both
// "explicit_bounds" and "bucket_counts" must be omitted and only "count" and
// "sum" are known.
Expand All @@ -414,22 +427,15 @@ message IntHistogramDataPoint {
// The set of labels that uniquely identify this timeseries.
repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1;

// start_time_unix_nano is the last time when the aggregation value was reset
// to "zero". For some metric types this is ignored, see data types for more
// details.
// StartTimeUnixNano is optional but strongly encouraged, see the
// the detiled comments above Metric.
//
// The aggregation value is over the time interval (start_time_unix_nano,
// time_unix_nano].
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
//
// Value of 0 indicates that the timestamp is unspecified. In that case the
// timestamp may be decided by the backend.
fixed64 start_time_unix_nano = 2;

// time_unix_nano is the moment when this aggregation value was reported.
//
// TimeUnixNano is required, see the detailed comments above Metric.
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
fixed64 time_unix_nano = 3;
Expand Down Expand Up @@ -461,7 +467,7 @@ message IntHistogramDataPoint {
// (-infinity, explicit_bounds[i]] for i == 0
// (explicit_bounds[i-1], explicit_bounds[i]] for 0 < i < N-1
// (explicit_bounds[i], +infinity) for i == N-1
//
//
// The values in the explicit_bounds array must be strictly increasing.
//
// Histogram buckets are inclusive of their upper boundary, except the last
Expand All @@ -480,12 +486,12 @@ message IntHistogramDataPoint {
// distribution of those values across a set of buckets.
//
// If the histogram contains the distribution of values, then both
// "explicit_bounds" and "bucket counts" fields must be defined.
// "explicit_bounds" and "bucket counts" fields must be defined.
// If the histogram does not contain the distribution of values, then both
// "explicit_bounds" and "bucket_counts" must be omitted and only "count" and
// "sum" are known.
message HistogramDataPoint {
// The set of key/value pairs that uniquely identify the timeseries from
// The set of key/value pairs that uniquely identify the timeseries from
// where this point belongs. The list may be empty (may contain 0 elements).
repeated opentelemetry.proto.common.v1.KeyValue attributes = 9;

Expand All @@ -499,22 +505,15 @@ message HistogramDataPoint {
// This field will be removed in ~3 months, on July 1, 2021.
repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1 [deprecated = true];

// start_time_unix_nano is the last time when the aggregation value was reset
// to "zero". For some metric types this is ignored, see data types for more
// details.
// StartTimeUnixNano is optional but strongly encouraged, see the
// the detiled comments above Metric.
//
// The aggregation value is over the time interval (start_time_unix_nano,
// time_unix_nano].
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
//
// Value of 0 indicates that the timestamp is unspecified. In that case the
// timestamp may be decided by the backend.
fixed64 start_time_unix_nano = 2;

// time_unix_nano is the moment when this aggregation value was reported.
//
// TimeUnixNano is required, see the detailed comments above Metric.
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
fixed64 time_unix_nano = 3;
Expand Down Expand Up @@ -546,7 +545,7 @@ message HistogramDataPoint {
// (-infinity, explicit_bounds[i]] for i == 0
// (explicit_bounds[i-1], explicit_bounds[i]] for 0 < i < N-1
// (explicit_bounds[i], +infinity) for i == N-1
//
//
// The values in the explicit_bounds array must be strictly increasing.
//
// Histogram buckets are inclusive of their upper boundary, except the last
Expand All @@ -562,7 +561,7 @@ message HistogramDataPoint {
// SummaryDataPoint is a single data point in a timeseries that describes the
// time-varying values of a Summary metric.
message SummaryDataPoint {
// The set of key/value pairs that uniquely identify the timeseries from
// The set of key/value pairs that uniquely identify the timeseries from
// where this point belongs. The list may be empty (may contain 0 elements).
repeated opentelemetry.proto.common.v1.KeyValue attributes = 7;

Expand All @@ -576,21 +575,14 @@ message SummaryDataPoint {
// This field will be removed in ~3 months, on July 1, 2021.
repeated opentelemetry.proto.common.v1.StringKeyValue labels = 1 [deprecated = true];

// start_time_unix_nano is the last time when the aggregation value was reset
// to "zero". For some metric types this is ignored, see data types for more
// details.
//
// The aggregation value is over the time interval (start_time_unix_nano,
// time_unix_nano].
// StartTimeUnixNano is optional but strongly encouraged, see the
// the detiled comments above Metric.
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
//
// Value of 0 indicates that the timestamp is unspecified. In that case the
// timestamp may be decided by the backend.
fixed64 start_time_unix_nano = 2;

// time_unix_nano is the moment when this aggregation value was reported.
// TimeUnixNano is required, see the detailed comments above Metric.
//
// Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January
// 1970.
Expand Down