Fixes #207 - Add Style guide for proto comments. (#717)

* Fixes #207 - Add Style guide for proto comments.

* Update comments to match style guide.

* Fixes from review.

* Update opentelemetry/proto/metrics/v1/metrics.proto

Co-authored-by: Tigran Najaryan <[email protected]>

---------

Co-authored-by: Tigran Najaryan <[email protected]>

Author: jsuereth

CONTRIBUTING.md

@@ -21,6 +21,20 @@ for general information about the project.
 After making any changes to .proto files make sure to generate all
 implementation by running `make gen-all`.
 
+## Style-Guide
+
+OpenTelemetry follows the [protocol buffer style guide](https://protobuf.dev/programming-guides/style/) with the following clarifications:
+
+- All Messages and fields should be documented via comments.
+- Field comments should document purpose or behavior with active verbs, or
+  a simple definition noun phrase (similar to a dictionary entry).
+  - valid: "Represents ..."
+    valid: "Additional attributes that describe the scope."
+  - not-valid: "used to represent..."
+- Message and field comments may reference the field or message by name.
+  - valid: "AnyValue ..."
+  - valid: "The value ..."
+
 ## Further Help
 
 If you have any questions or need assistance while contributing, feel free to reach out to the [`#otel-specification`](https://cloud-native.slack.com/archives/C01N7PP1THC) Slack channel.  

opentelemetry/proto/common/v1/common.proto

@@ -22,7 +22,7 @@ option java_package = "io.opentelemetry.proto.common.v1";
 option java_outer_classname = "CommonProto";
 option go_package = "go.opentelemetry.io/proto/otlp/common/v1";
 
-// AnyValue is used to represent any type of attribute value. AnyValue may contain a
+// Represents any type of attribute value. AnyValue may contain a
 // primitive value such as a string or integer or it may contain an arbitrary nested
 // object containing arrays, key-value lists and primitives.
 message AnyValue {
@@ -59,24 +59,35 @@ message KeyValueList {
   repeated KeyValue values = 1;
 }
 
-// KeyValue is a key-value pair that is used to store Span attributes, Link
+// Represents a key-value pair that is used to store Span attributes, Link
 // attributes, etc.
 message KeyValue {
+  // The key name of the pair.
   string key = 1;
+
+  // The value of the pair.
   AnyValue value = 2;
 }
 
 // InstrumentationScope is a message representing the instrumentation scope information
 // such as the fully qualified name and version. 
 message InstrumentationScope {
+  // A name denoting the Instrumentation scope.
   // An empty instrumentation scope name means the name is unknown.
   string name = 1;
+
+  // Defines the version of the instrumentation scope.
+  // An empty instrumentation scope version means the version is unknown.
   string version = 2;
 
   // Additional attributes that describe the scope. [Optional].
   // Attribute keys MUST be unique (it is not allowed to have more than one
   // attribute with the same key).
   repeated KeyValue attributes = 3;
+
+  // The number of attributes that were discarded. Attributes
+  // can be discarded because their keys are too long or because there are too many
+  // attributes. If this value is 0, then no attributes were dropped.
   uint32 dropped_attributes_count = 4;
 }
 

opentelemetry/proto/metrics/v1/metrics.proto

@@ -187,13 +187,13 @@ message ScopeMetrics {
 message Metric {
   reserved 4, 6, 8;
 
-  // name of the metric.
+  // The name of the metric.
   string name = 1;
 
-  // description of the metric, which can be used in documentation.
+  // A description of the metric, which can be used in documentation.
   string description = 2;
 
-  // unit in which the metric value is reported. Follows the format
+  // The unit in which the metric value is reported. Follows the format
   // described by https://unitsofmeasure.org/ucum.html.
   string unit = 3;
 
@@ -228,25 +228,31 @@ message Metric {
 // AggregationTemporality is not included. Consequently, this also means
 // "StartTimeUnixNano" is ignored for all data points.
 message Gauge {
+  // The time series data points.
+  // Note: Multiple time series may be included (same timestamp, different attributes).
   repeated NumberDataPoint data_points = 1;
 }
 
 // Sum represents the type of a scalar metric that is calculated as a sum of all
 // reported measurements over a time interval.
 message Sum {
+  // The time series data points.
+  // Note: Multiple time series may be included (same timestamp, different attributes).
   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;
 
-  // If "true" means that the sum is monotonic.
+  // Represents whether the sum is monotonic.
   bool is_monotonic = 3;
 }
 
 // Histogram represents the type of a metric that is calculated by aggregating
 // as a Histogram of all reported measurements over a time interval.
 message Histogram {
+  // The time series data points.
+  // Note: Multiple time series may be included (same timestamp, different attributes).
   repeated HistogramDataPoint data_points = 1;
 
   // aggregation_temporality describes if the aggregator reports delta changes
@@ -257,6 +263,8 @@ message Histogram {
 // ExponentialHistogram represents the type of a metric that is calculated by aggregating
 // as a ExponentialHistogram of all reported double measurements over a time interval.
 message ExponentialHistogram {
+  // The time series data points.
+  // Note: Multiple time series may be included (same timestamp, different attributes).
   repeated ExponentialHistogramDataPoint data_points = 1;
 
   // aggregation_temporality describes if the aggregator reports delta changes
@@ -274,6 +282,8 @@ message ExponentialHistogram {
 // because the count and sum fields of a SummaryDataPoint are assumed to be
 // cumulative values.
 message Summary {
+  // The time series data points.
+  // Note: Multiple time series may be included (same timestamp, different attributes).
   repeated SummaryDataPoint data_points = 1;
 }
 
@@ -554,12 +564,12 @@ message ExponentialHistogramDataPoint {
   // 1970.
   fixed64 time_unix_nano = 3;
 
-  // count is the number of values in the population. Must be
+  // The number of values in the population. Must be
   // non-negative. This value must be equal to the sum of the "bucket_counts"
   // values in the positive and negative Buckets plus the "zero_count" field.
   fixed64 count = 4;
 
-  // sum of the values in the population. If count is zero then this field
+  // The sum of the values in the population. If count is zero then this field
   // must be zero.
   //
   // Note: Sum should only be filled out when measuring non-negative discrete
@@ -586,7 +596,7 @@ message ExponentialHistogramDataPoint {
   // values depend on the range of the data.
   sint32 scale = 6;
 
-  // zero_count is the count of values that are either exactly zero or
+  // The count of values that are either exactly zero or
   // within the region considered zero by the instrumentation at the
   // tolerated degree of precision.  This bucket stores values that
   // cannot be expressed using the standard exponential formula as
@@ -605,12 +615,12 @@ message ExponentialHistogramDataPoint {
   // Buckets are a set of bucket counts, encoded in a contiguous array
   // of counts.
   message Buckets {
-    // Offset is the bucket index of the first entry in the bucket_counts array.
+    // The bucket index of the first entry in the bucket_counts array.
     //
     // Note: This uses a varint encoding as a simple form of compression.
     sint32 offset = 1;
 
-    // bucket_counts is an array of count values, where bucket_counts[i] carries
+    // An array of count values, where bucket_counts[i] carries
     // the count of the bucket at index (offset+i). bucket_counts[i] is the count
     // of values greater than base^(offset+i) and less than or equal to
     // base^(offset+i+1).
@@ -630,10 +640,10 @@ message ExponentialHistogramDataPoint {
   // measurements that were used to form the data point
   repeated Exemplar exemplars = 11;
 
-  // min is the minimum value over (start_time, end_time].
+  // The minimum value over (start_time, end_time].
   optional double min = 12;
 
-  // max is the maximum value over (start_time, end_time].
+  // The maximum value over (start_time, end_time].
   optional double max = 13;
 
   // ZeroThreshold may be optionally set to convey the width of the zero

opentelemetry/proto/profiles/v1development/profiles.proto

@@ -300,7 +300,7 @@ message Profile {
   // This field is optional; an ID may be assigned to an ID-less profile in a later step.
   bytes profile_id = 8;
 
-  // dropped_attributes_count is the number of attributes that were discarded. Attributes
+  // The number of attributes that were discarded. Attributes
   // can be discarded because their keys are too long or because there are too many
   // attributes. If this value is 0, then no attributes were dropped.
   uint32 dropped_attributes_count = 9;
@@ -333,8 +333,11 @@ message Link {
 
 // ValueType describes the type and units of a value.
 message ValueType {
-  int32 type_strindex = 1; // Index into ProfilesDictionary.string_table.
-  int32 unit_strindex = 2; // Index into ProfilesDictionary.string_table.
+  // Index into ProfilesDictionary.string_table.
+  int32 type_strindex = 1;
+
+  // Index into ProfilesDictionary.string_table.
+  int32 unit_strindex = 2;
 }
 
 // Each Sample records values encountered in some program context. The program
@@ -437,7 +440,7 @@ message Line {
 // Describes a function, including its human-readable name, system name,
 // source file, and starting line number in the source.
 message Function {
-  // Function name. Empty string if not available.
+  // The function name. Empty string if not available.
   int32 name_strindex = 1;
   // Function name, as identified by the system. For instance,
   // it can be a C++ mangled name. Empty string if not available.
@@ -452,7 +455,11 @@ message Function {
 // for profiles than opentelemetry.proto.common.v1.KeyValue
 // Specifically, uses the string table for keys and allows optional unit information.
 message KeyValueAndUnit {
+  // The index into the string table for the attribute's key.
   int32 key_strindex  = 1;
+  // The value of the attribute.
   opentelemetry.proto.common.v1.AnyValue value = 2;
-  int32 unit_strindex = 3; // zero indicates implicit (by semconv) or non-defined unit.
+  // The index into the string table for the attribute's unit.
+  // zero indicates implicit (by semconv) or non-defined unit.
+  int32 unit_strindex = 3;
 }

opentelemetry/proto/resource/v1/resource.proto

@@ -41,7 +41,7 @@ message Resource {
   // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.47.0/specification/common/README.md#attribute.
   repeated opentelemetry.proto.common.v1.KeyValue attributes = 1;
 
-  // dropped_attributes_count is the number of dropped attributes. If the value is 0, then
+  // The number of dropped attributes. If the value is 0, then
   // no attributes were dropped.
   uint32 dropped_attributes_count = 2;
 

opentelemetry/proto/trace/v1/trace.proto

@@ -182,23 +182,23 @@ message Span {
   // and `SERVER` (callee) to identify queueing latency associated with the span.
   SpanKind kind = 6;
 
-  // start_time_unix_nano is the start time of the span. On the client side, this is the time
+  // The start time of the span. On the client side, this is the time
   // kept by the local machine where the span execution starts. On the server side, this
   // is the time when the server's application handler starts running.
   // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970.
   //
   // This field is semantically required and it is expected that end_time >= start_time.
   fixed64 start_time_unix_nano = 7;
 
-  // end_time_unix_nano is the end time of the span. On the client side, this is the time
+  // The end time of the span. On the client side, this is the time
   // kept by the local machine where the span execution ends. On the server side, this
   // is the time when the server application handler stops running.
   // Value is UNIX Epoch time in nanoseconds since 00:00:00 UTC on 1 January 1970.
   //
   // This field is semantically required and it is expected that end_time >= start_time.
   fixed64 end_time_unix_nano = 8;
 
-  // attributes is a collection of key/value pairs. Note, global attributes
+  // A collection of key/value pairs. Note, global attributes
   // like server name can be set using the resource API. Examples of attributes:
   //
   //     "/http/user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36"
@@ -220,22 +220,22 @@ message Span {
   // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.47.0/specification/common/README.md#attribute.
   repeated opentelemetry.proto.common.v1.KeyValue attributes = 9;
 
-  // dropped_attributes_count is the number of attributes that were discarded. Attributes
+  // The number of attributes that were discarded. Attributes
   // can be discarded because their keys are too long or because there are too many
   // attributes. If this value is 0, then no attributes were dropped.
   uint32 dropped_attributes_count = 10;
 
   // Event is a time-stamped annotation of the span, consisting of user-supplied
   // text description and key-value pairs.
   message Event {
-    // time_unix_nano is the time the event occurred.
+    // The time the event occurred.
     fixed64 time_unix_nano = 1;
 
-    // name of the event.
+    // The name of the event.
     // This field is semantically required to be set to non-empty string.
     string name = 2;
 
-    // attributes is a collection of attribute key/value pairs on the event.
+    // A collection of attribute key/value pairs on the event.
     // Attribute keys MUST be unique (it is not allowed to have more than one
     // attribute with the same key).
     //
@@ -250,15 +250,15 @@ message Span {
     // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.47.0/specification/common/README.md#attribute.
     repeated opentelemetry.proto.common.v1.KeyValue attributes = 3;
 
-    // dropped_attributes_count is the number of dropped attributes. If the value is 0,
+    // The number of dropped attributes. If the value is 0,
     // then no attributes were dropped.
     uint32 dropped_attributes_count = 4;
   }
 
-  // events is a collection of Event items.
+  // A collection of Event items.
   repeated Event events = 11;
 
-  // dropped_events_count is the number of dropped events. If the value is 0, then no
+  // The number of dropped events. If the value is 0, then no
   // events were dropped.
   uint32 dropped_events_count = 12;
 
@@ -277,7 +277,7 @@ message Span {
     // The trace_state associated with the link.
     string trace_state = 3;
 
-    // attributes is a collection of attribute key/value pairs on the link.
+    // A collection of attribute key/value pairs on the link.
     // Attribute keys MUST be unique (it is not allowed to have more than one
     // attribute with the same key).
     //
@@ -292,7 +292,7 @@ message Span {
     // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.47.0/specification/common/README.md#attribute.
     repeated opentelemetry.proto.common.v1.KeyValue attributes = 4;
 
-    // dropped_attributes_count is the number of dropped attributes. If the value is 0,
+    // The number of dropped attributes. If the value is 0,
     // then no attributes were dropped.
     uint32 dropped_attributes_count = 5;
 
@@ -316,11 +316,11 @@ message Span {
     fixed32 flags = 6;
   }
 
-  // links is a collection of Links, which are references from this span to a span
+  // A collection of Links, which are references from this span to a span
   // in the same or different trace.
   repeated Link links = 13;
 
-  // dropped_links_count is the number of dropped links after the maximum size was
+  // The number of dropped links after the maximum size was
   // enforced. If this value is 0, then no links were dropped.
   uint32 dropped_links_count = 14;