Skip to content

Commit

Permalink
Ensure enums are consistently declared
Browse files Browse the repository at this point in the history
Resolves #363

The rules are:
- Place enums outside the messages.
- Prefix the enum value names with enum name, using underscores with uppercase.

The changes in this PR should be backwards compatible (or allowed because
they are in experimental parts):
- For Binary Protobuf: the encoding does not use enum names anywhere on the wire.
  There are no changes on the wire at all.
- For JSON Protobuf:
  - DataPointFlags and LogRecordFlags are not visible on the wire since they are
    just helper enums to be used in the code.
  - Moving SpanKind and StatusCode out of the message should not result in any
    changes on the wire.
  - ConstantDecision is experimental and we are free to break it.

Note: all of these changes are breaking for the code that consumes the generated
proto files. We consider this acceptable since this repository is not yet declared
Stable.

I would like someone to independently confirm my analysis to make sure
this PR does not break anything on the wire (neither for binary nor for JSON).
  • Loading branch information
tigrannajaryan committed May 31, 2022
1 parent c31b2d8 commit be5cd5c
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 52 deletions.
4 changes: 2 additions & 2 deletions opentelemetry/proto/logs/v1/logs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ enum SeverityNumber {

// Masks for LogRecord.flags field.
enum LogRecordFlags {
LOG_RECORD_FLAG_UNSPECIFIED = 0;
LOG_RECORD_FLAG_TRACE_FLAGS_MASK = 0x000000FF;
LOG_RECORD_FLAGS_UNSPECIFIED = 0;
LOG_RECORD_FLAGS_TRACE_FLAGS_MASK = 0x000000FF;
}

// A log record according to OpenTelemetry Log Data Model:
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry/proto/metrics/v1/metrics.proto
Original file line number Diff line number Diff line change
Expand Up @@ -364,12 +364,12 @@ enum AggregationTemporality {
// (point.flags & FLAG_NO_RECORDED_VALUE) == FLAG_NO_RECORDED_VALUE
//
enum DataPointFlags {
FLAG_NONE = 0;
DATA_POINTS_FLAG_NONE = 0;

// This DataPoint is valid but has no recorded value. This value
// SHOULD be used to reflect explicitly missing data in a series, as
// for an equivalent to the Prometheus "staleness marker".
FLAG_NO_RECORDED_VALUE = 1;
DATA_POINTS_FLAG_NO_RECORDED_VALUE = 1;

// Bits 2-31 are reserved for future use.
}
Expand Down
84 changes: 42 additions & 42 deletions opentelemetry/proto/trace/v1/trace.proto
Original file line number Diff line number Diff line change
Expand Up @@ -171,36 +171,6 @@ message Span {
// This field is required.
string name = 5;

// SpanKind is the type of span. Can be used to specify additional relationships between spans
// in addition to a parent/child relationship.
enum SpanKind {
// Unspecified. Do NOT use as default.
// Implementations MAY assume SpanKind to be INTERNAL when receiving UNSPECIFIED.
SPAN_KIND_UNSPECIFIED = 0;

// Indicates that the span represents an internal operation within an application,
// as opposed to an operation happening at the boundaries. Default value.
SPAN_KIND_INTERNAL = 1;

// Indicates that the span covers server-side handling of an RPC or other
// remote network request.
SPAN_KIND_SERVER = 2;

// Indicates that the span describes a request to some remote service.
SPAN_KIND_CLIENT = 3;

// Indicates that the span describes a producer sending a message to a broker.
// Unlike CLIENT and SERVER, there is often no direct critical path latency relationship
// between producer and consumer spans. A PRODUCER span ends when the message was accepted
// by the broker while the logical processing of the message might span a much longer time.
SPAN_KIND_PRODUCER = 4;

// Indicates that the span describes consumer receiving a message from a broker.
// Like the PRODUCER kind, there is often no direct critical path latency relationship
// between producer and consumer spans.
SPAN_KIND_CONSUMER = 5;
}

// Distinguishes between spans generated in a particular context. For example,
// two spans with the same name may be distinguished using `CLIENT` (caller)
// and `SERVER` (callee) to identify queueing latency associated with the span.
Expand Down Expand Up @@ -306,6 +276,36 @@ message Span {
Status status = 15;
}

// SpanKind is the type of span. Can be used to specify additional relationships between spans
// in addition to a parent/child relationship.
enum SpanKind {
// Unspecified. Do NOT use as default.
// Implementations MAY assume SpanKind to be INTERNAL when receiving UNSPECIFIED.
SPAN_KIND_UNSPECIFIED = 0;

// Indicates that the span represents an internal operation within an application,
// as opposed to an operation happening at the boundaries. Default value.
SPAN_KIND_INTERNAL = 1;

// Indicates that the span covers server-side handling of an RPC or other
// remote network request.
SPAN_KIND_SERVER = 2;

// Indicates that the span describes a request to some remote service.
SPAN_KIND_CLIENT = 3;

// Indicates that the span describes a producer sending a message to a broker.
// Unlike CLIENT and SERVER, there is often no direct critical path latency relationship
// between producer and consumer spans. A PRODUCER span ends when the message was accepted
// by the broker while the logical processing of the message might span a much longer time.
SPAN_KIND_PRODUCER = 4;

// Indicates that the span describes consumer receiving a message from a broker.
// Like the PRODUCER kind, there is often no direct critical path latency relationship
// between producer and consumer spans.
SPAN_KIND_CONSUMER = 5;
}

// The Status type defines a logical error model that is suitable for different
// programming environments, including REST APIs and RPC APIs.
message Status {
Expand All @@ -314,18 +314,18 @@ message Status {
// A developer-facing human readable error message.
string message = 2;

// For the semantics of status codes see
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
enum StatusCode {
// The default status.
STATUS_CODE_UNSET = 0;
// The Span has been validated by an Application developers or Operator to have
// completed successfully.
STATUS_CODE_OK = 1;
// The Span contains an error.
STATUS_CODE_ERROR = 2;
};

// The status code.
StatusCode code = 3;
}

// For the semantics of status codes see
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
enum StatusCode {
// The default status.
STATUS_CODE_UNSET = 0;
// The Span has been validated by an Application developers or Operator to have
// completed successfully.
STATUS_CODE_OK = 1;
// The Span contains an error.
STATUS_CODE_ERROR = 2;
};
13 changes: 7 additions & 6 deletions opentelemetry/proto/trace/v1/trace_config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ message ConstantSampler {
// - Always off
// - Always on
// - Always follow the parent Span's decision (off if no parent).
enum ConstantDecision {
ALWAYS_OFF = 0;
ALWAYS_ON = 1;
ALWAYS_PARENT = 2;
}
ConstantDecision decision = 1;
ConstantSamplerDecision decision = 1;
}

enum ConstantSamplerDecision {
CONSTANT_SAMPLER_DECISION_ALWAYS_OFF = 0;
CONSTANT_SAMPLER_DECISION_ALWAYS_ON = 1;
CONSTANT_SAMPLER_DECISION_ALWAYS_PARENT = 2;
}

// Sampler that tries to uniformly sample traces with a given ratio.
Expand Down

0 comments on commit be5cd5c

Please sign in to comment.