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

Jump to bottom

Add W3C-specified trace flags to v1 Span proto #503

Merged
merged 13 commits into from
Sep 14, 2023
Merged

Add W3C-specified trace flags to v1 Span proto #503

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2df5ad1
Add W3C-specified trace flags to v1 Span proto
jmacd Aug 4, 2023
3beb053
draft changelog
jmacd Aug 4, 2023
d26fab1
Link context is for the link
jmacd Aug 10, 2023
493d1a3
use 16
jmacd Aug 10, 2023
05e412f
feedback applied
jmacd Aug 21, 2023
5fc27d9
comment on new field
jmacd Aug 21, 2023
703950f
new spans should zero 24 bits; use tag 18
jmacd Aug 24, 2023
8505a5a
refine comment
jmacd Sep 5, 2023
8329601
Merge branch 'main' into jmacd/trace_flags
carlosalberto Sep 5, 2023
abb6ff9
Merge branch 'jmacd/trace_flags' of github.com:jmacd/opentelemetry-pr…
jmacd Sep 5, 2023
4251604
merge
jmacd Sep 5, 2023
7b33da2
revise
jmacd Sep 5, 2023
de0b005
Update opentelemetry/proto/trace/v1/trace.proto
jmacd Sep 13, 2023
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

Full list of differences found in [this compare](https://github.com/open-telemetry/opentelemetry-proto/compare/v1.0.0...main).

### Added

* Add a field for W3C-specified Trace Context flags to the `Span` and `Link`.
[#503](https://github.com/open-telemetry/opentelemetry-proto/pull/503)

## 1.0.0 - 2023-07-03

Full list of differences found in [this compare](https://github.com/open-telemetry/opentelemetry-proto/compare/v0.20.0...v1.0.0).
Expand Down
8 changes: 5 additions & 3 deletions opentelemetry/proto/logs/v1/logs.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ enum SeverityNumber {
SEVERITY_NUMBER_FATAL4 = 24;
}

// LogRecordFlags is defined as a protobuf 'uint32' type and is to be used as
// bit-fields. Each non-zero value defined in this enum is a bit-mask.
// To extract the bit-field, for example, use an expression like:
// LogRecordFlags represents constants used to interpret the
// LogRecord.flags field, which is protobuf 'fixed32' type and is to
// be used as bit-fields. Each non-zero value defined in this enum is
// a bit-mask. To extract the bit-field, for example, use an
// expression like:
//
// (logRecord.flags & LOG_RECORD_FLAGS_TRACE_FLAGS_MASK)
//
Expand Down
45 changes: 45 additions & 0 deletions opentelemetry/proto/trace/v1/trace.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ message Span {
// field must be empty. The ID is an 8-byte array.
bytes parent_span_id = 4;

// Flags, a bit field. 8 least significant bits are the trace
// flags as defined in W3C Trace Context specification. Readers
// MUST not assume that 24 most significant bits will be zero.
// When creating new spans, the most-significant 24-bits MUST be
// zero. To read the 8-bit W3C trace flag (use flags &
// SPAN_FLAGS_TRACE_FLAGS_MASK). [Optional].
Copy link
Member

@Oberon00 Oberon00 Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think "when creating new spans" may still be confusing in the context of the protocol spec / a proto comment as it could be understood as "creating new span messages", especially since the remainder seems to assume that "new spans" have a 32 bit flags field. I could not find a shorter unambiguous way than this:

Suggested change
// MUST not assume that 24 most significant bits will be zero.
// When creating new spans, the most-significant 24-bits MUST be
// zero. To read the 8-bit W3C trace flag (use flags &
// SPAN_FLAGS_TRACE_FLAGS_MASK). [Optional].
// MUST not assume that 24 most significant bits will be zero.
// To read the 8-bit W3C trace flag, use `flags & SPAN_FLAGS_TRACE_FLAGS_MASK`.
//
// When creating span messages, if the message is logically forwarded from another source
// with an equivalent flags fields (i.e., usually another OTLP span message), the field SHOULD
// be copied as-is. If creating from a source that does not have an equivalent flags field
// (such as a runtime representation of an OpenTelemetry span), the high 24 bits MUST
// be set to zero.
//
// [Optional].

Copy link
Member

Choose a reason for hiding this comment

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

Also, there seems to be a typo in:

To read the 8-bit W3C trace flag (use flags & SPAN_FLAGS_TRACE_FLAGS_MASK).

I assumed it should mean:

To read the 8-bit W3C trace flag, use flags & SPAN_FLAGS_TRACE_FLAGS_MASK.

Copy link
Member

@Oberon00 Oberon00 Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@carlosalberto @jmacd I think we forgot to apply an analogous change to the flags field in the link object.

//
// See https://www.w3.org/TR/trace-context-2/#trace-flags for the flag definitions.
fixed32 flags = 16;
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved

// A description of the span's operation.
//
// For example, the name can be a qualified method name or a file name
Expand Down Expand Up @@ -242,6 +252,16 @@ message Span {
// dropped_attributes_count is the number of dropped attributes. If the value is 0,
// then no attributes were dropped.
uint32 dropped_attributes_count = 5;

// Flags, a bit field. 8 least significant bits are the trace
// flags as defined in W3C Trace Context specification. Readers
// MUST not assume that 24 most significant bits will be zero.
// When creating new spans, the most-significant 24-bits MUST be
// zero. To read the 8-bit W3C trace flag (use flags &
// SPAN_FLAGS_TRACE_FLAGS_MASK). [Optional].
//
// See https://www.w3.org/TR/trace-context-2/#trace-flags for the flag definitions.
fixed32 flags = 6;
}

// links is a collection of Links, which are references from this span to a span
Expand Down Expand Up @@ -280,3 +300,28 @@ message Status {
// The status code.
StatusCode code = 3;
}

// SpanFlags represents constants used to interpret the
// Span.flags field, which is protobuf 'fixed32' type and is to
// be used as bit-fields. Each non-zero value defined in this enum is
// a bit-mask. To extract the bit-field, for example, use an
// expression like:
//
// (span.flags & SPAN_FLAGS_TRACE_FLAGS_MASK)
//
// See https://www.w3.org/TR/trace-context-2/#trace-flags for the flag definitions.
//
// Note that Span flags were introduced in version 1.1 of the
// OpenTelemetry protocol. Older Span producers do not set this
// field, consequently consumers should not rely on the absence of a
// particular flag bit to indicate the presence of a particular feature.
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
enum SpanFlags {
// The zero value for the enum. Should not be used for comparisons.
// Instead use bitwise "and" with the appropriate mask as shown above.
SPAN_FLAGS_DO_NOT_USE = 0;

// Bits 0-7 are used for trace flags.
SPAN_FLAGS_TRACE_FLAGS_MASK = 0x000000FF;

// Bits 8-31 are reserved for future use.
}
Loading