Skip to content

[Common] Spec inconsistency with proto definition of Attributes #2581

@MSNev

Description

@MSNev

The proto definition used for attributes uses "KeyValue" which can have a value of AnyValue, however, the spec definition of "Attribute" only allows for primitives or an Array of primitives, this is inconsistent.

What did you expect to see?

  • They should be defined in the same way or have their own distinct definitions

Additional context.

The Spec Attribute definition

Trace proto attribute definition

  // attributes is 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"
  //     "/http/server_latency": 300
  //     "abc.com/myattribute": true
  //     "abc.com/score": 10.239
  //
  // The OpenTelemetry API specification further restricts the allowed value types:
  // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/common.md#attributes
  // Attribute keys MUST be unique (it is not allowed to have more than one
  // attribute with the same key).
  repeated opentelemetry.proto.common.v1.KeyValue attributes = 9;

And the Logs spec also uses Attributes

Logs proto attribute definition

  // Additional attributes that describe the specific event occurrence. [Optional].
  // Attribute keys MUST be unique (it is not allowed to have more than one
  // attribute with the same key).
  repeated opentelemetry.proto.common.v1.KeyValue attributes = 6;

With Proto KeyValue defined as taking AnyValue

// KeyValue is a key-value pair that is used to store Span attributes, Link
// attributes, etc.
message KeyValue {
  string key = 1;
  AnyValue value = 2;
}

And Proto AnyValue being

// AnyValue is used to represent 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 {
  // The value is one of the listed fields. It is valid for all values to be unspecified
  // in which case this AnyValue is considered to be "empty".
  oneof value {
    string string_value = 1;
    bool bool_value = 2;
    int64 int_value = 3;
    double double_value = 4;
    ArrayValue array_value = 5;
    KeyValueList kvlist_value = 6;
    bytes bytes_value = 7;
  }
}

And Proto KeyValueList being

// KeyValueList is a list of KeyValue messages. We need KeyValueList as a message
// since `oneof` in AnyValue does not allow repeated fields. Everywhere else where we need
// a list of KeyValue messages (e.g. in Span) we use `repeated KeyValue` directly to
// avoid unnecessary extra wrapping (which slows down the protocol). The 2 approaches
// are semantically equivalent.
message KeyValueList {
  // A collection of key/value pairs of key-value pairs. The list may be empty (may
  // contain 0 elements).
  // The keys MUST be unique (it is not allowed to have more than one
  // value with the same key).
  repeated KeyValue values = 1;
}

Metadata

Metadata

Assignees

Labels

spec:traceRelated to the specification/trace directory

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions