Span Parent should be determined from Context (and take this as a parameter.

[jsuereth]

The specification api states:

The parent Context or an indication that the new Span should be a root Span. The API MAY also have an option for implicitly using the current Context as parent as a default behavior. This API MUST NOT accept a Span or SpanContext as parent, only a full Context.

The semantic parent of the Span MUST be determined according to the rules described in Determining the Parent Span from a Context.

Here's the current API: https://github.com/open-telemetry/opentelemetry-cpp/blob/main/api/include/opentelemetry/trace/span.h#L67

[lalitb]

Good point. This was also observed as part of initial review for compliance matrix ( #717 ), but then few other languages ( at-least opentelemetry-dotnet ) also accepts SpanContext as parent, so we didn't do any change.
This would be a breaking change for existing customers, so need to plan it better.

[lalitb]

As this is API breaking change, and other language(s) also have similar implementation, would like to discuss this in next community meeting before any changes.

[lalitb]

@jsuereth We would like to part these changes for next major release of otel-cpp after 1.0.0:

• It's a major API change, and we would like to avoid any such change while in Release Candidate.
• There would be quite a few internal and external customers using the current RC in production, with expectation of no critical breaking change being added in 1.0.0.
• There hasn't been any complaint from the existing customers regarding this deviation from otel-specs. Which means current api doesn't make things hard for them.

Unless we are missing any feature by passing parent's SpanContext, or making it difficult for applications to use the API, we would like to stick with current implementation, and park this change for next major otel-cpp release.

Would like to know if you see any issue here.

[jsuereth]

There's some open discussions going on around Baggage and Span.attributes that not having this feature will prevent us fixing going forward.

I understand not wanting to break customers. Are you able to provide this fix in a non-breaking fashion? I believe you should be able to. I think it's ok to have a legacy APi to avoid breakage, but given 1.0.0 is NOT out nor is their assumed compatibility prior to 1.0, you should be able to fix this now. The reason we do this review now is to finalize the 1.0 API prior to stabilizing.

[lalitb]

Are you able to provide this fix in a non-breaking fashion? I believe you should be able to.

Yes, that is doable with small restructuring of code, I think we are just slightly non-compliant this way :)
Have raised PR for this : #969