Declare OTLP/JSON Stable

[tigrannajaryan]

See also open-telemetry/opentelemetry-proto#436

[tigrannajaryan]

We need a lot of eyes on this before we merge this.

[bogdandrutu]

Would like to clarify the behavior for invalid (all 0) trace/span IDs. In other words we have 3 cases for trace/logs/metrics exemplar:

• "traceId" key is not present
• "traceId" key is present and has an empty value
• "traceId" key is present and value is all 0s

Can we consider these 3 cases the same? Is there a difference between them, if yes, what is the semantic?

[jmacd]

I asked @bogdandrutu because I didn't understand immediately how #2930 (review) was supposed to block stabilizing OTLP/JSON.

There are more cases than the three listed above. We expect that TraceIDs should be exactly 32 hex digits and SpanIDs should be exactly 16 hex digits. This follows from the conditions for a "valid" TraceID and SpanID here: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spancontext

We already have language about rejecting invalid data, which makes me think no more specification is needed. We should reject IDs with all zeros, whether it is because they are empty or missing, because valid identifiers require at least one non-zero byte.

[tigrannajaryan]

We already have language about rejecting invalid data, which makes me think no more specification is needed. We should reject IDs with all zeros, whether it is because they are empty or missing, because valid identifiers require at least one non-zero byte.

This is true for Spans but not for LogRecords, where traceid/spanid are optional fields.

[jmacd]

This is true for Spans but not for LogRecords, where traceid/spanid are optional fields.

Bogdan's remark was clear about presence and absence, though #2930 (review).

If the "traceId" is absent, then the field is absent, and if the "traceId" is present then the field is present. We only expect valid data when a field is present.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[bogdandrutu]

If the "traceId" is absent, then the field is absent, and if the "traceId" is present then the field is present. We only expect valid data when a field is present.

Does that mean absent is equivalent with present and invalid?

[Aneurysm9]

Does that mean absent is equivalent with present and invalid?

If the field is required, then I would say yes. If the field is not required, then no.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[carlosalberto]

Hey @tigrannajaryan

Adding a small note after (briefly) discussing this: the question regarding trace_id being missing/empty/all-zeroes is specially important as we are deviating from the standard protbuf/JSON handling for trace_id, hence having this super clear would be important for components such as the Collector.

As discussed: we should at least mention some of the mentioned cases (missing/empty trace_id) is undefined behavior.

cc @bogdandrutu @jmacd

[abitrolly]

We need a lot of eyes on this before we merge this.

I can not read the spec. I need to read examples, but there are no examples as long as open-telemetry/opentelemetry-proto#462 is open.

[abitrolly]

Another concern is that spec doesn't cover how to deal with incomplete and missing spans, which should occur quite often in case of errors and exceptions. Basically in cases where the proper trace would be most useful.

#2773 (comment)
#373 (comment)

[tigrannajaryan]

Would like to clarify the behavior for invalid (all 0) trace/span IDs. In other words we have 3 cases for trace/logs/metrics exemplar:

• "traceId" key is not present
• "traceId" key is present and has an empty value
• "traceId" key is present and value is all 0s

Can we consider these 3 cases the same? Is there a difference between them, if yes, what is the semantic?

Here is a PR to clarify this: open-telemetry/opentelemetry-proto#442

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[tigrannajaryan]

Not stale. Blocked by open-telemetry/opentelemetry-proto#442

[carlosalberto]

@tigrannajaryan the last blocking issue has been resolved - I think it's time to merge it (and release it as part of the February release).

[tigrannajaryan]

This is ready to be merged after 1.18 release.

[abitrolly]

IWithout making end of span optional you will have to break the stable implementation, or people will resort to hacks for debugging spans in real-time.

[carlosalberto]

@abitrolly Probably a comment for a different PR/issue? Please follow up otherwise with a new issue ;)

[carlosalberto]

Merging as 1.18.0 was released.

[jonatan-ivanov]

Is there any roadblock in front of marking the whole OTLP spec stable (right now it is mixed but its components are stable)?