Specify IsValid and IsRemote as required

[arminru]

Fixes #908.

I explicitly specified both APIs as required, which I think was the initial intention.

According to the spec-compliance-matrix, only JS is known to not have implemented IsValid.
--> implemented in the meantime

Apart from that, I slightly reworded IsRemote to not have the API spec dictate how it should be implemented.

[Oberon00]

Please fix this more generally. Right now the spec also says

The OpenTelemetry SpanContext representation conforms to the W3C TraceContext specification.

Is this a MUST?

It contains two identifiers - a TraceId and a SpanId - along with a set of common TraceFlags and system-specific TraceState values.

Are both identifiers a MUST? Are TraceFlags and TraceState a MUST?

etc

[arminru]

@Oberon00 In contrast to the two mentioned APIs, which I could interpret as "suggesting names and behavior", if implemented, the other parts can only be interpreted as given IMHO. If it would say "The OpenTelemetry SpanContext representation MUST conform to the W3C TraceContext specification.", this would impose a MUST on the spec itself (which wouldn't make any sense), rather than implementations, wouldn't it?

[Oberon00]

You could simplify+clarify this at once by just saying "SpanContext MUST conform to the W3C TraceContext specification". The sentence you suggested does indeed sound odd.

[Oberon00]

Approving this, because while incomplete it is an improvement over the current situation.

[arminru]

@Oberon00 Personally I think the current wording is fine. "SpanContext conforms to the W3C TraceContext spec" implies that anyone implementing it will also have to conform to it. When the spec says "SpanContext contains two identifiers", an implementation that only implements one is not implementing SpanContext. The section is about defining SpanContext itself and not the requirements for how SpanContext should be defined by the spec.
I could, however, rephrase it to "SpanContext implementations MUST ..." but I'm not sure if that adds value.

[Oberon00]

Can we just add the sentence "The API MUST provide all operations specified in the following subsections" instead?

[Oberon00]

When the spec says "SpanContext contains two identifiers", an implementation that only implements one is not implementing SpanContext.

Does it say anywhere that SpanContext MUST be implemented?

[arminru]

@Oberon00

Does it say anywhere that SpanContext MUST be implemented?

It says:

The Span interface MUST provide:

• An API that returns the SpanContext for the given Span.

[carlosalberto]

Overall LGTM. Maybe you want to apply @Oberon00's suggestion in order to not have to use MUST for every operation ;)

[carlosalberto]

@arminru Please rebase too ;)

[arminru]

@carlosalberto I think the current wording is quite fine. If I put that sentence in the section above, I would still have to write something like "There MUST be an API ..." or "An API ... MUST be provided" or at least "The API ... is defined as ...". Otherwise it will just be an odd sounding mention of some behavior, so we wouldn't save any spare words here.

The original wording does not really make up proper sentences:

"An API that returns a boolean value, which is true if the SpanContext has a non-zero TraceID and a non-zero SpanID."

"An API that returns a boolean value, which is true if the SpanContext was propagated from a remote parent."

[obecny]

lgtm

[Oberon00]

Do we really need a changelog entry for such clarifications?

[arminru]

I didn't add one at first but then thought it couldn't hurt to make SIG maintainers aware of it, just in case they interpreted it differently in the past. Not all have filled out the compliance matrix yet and adding the entry was easier than inspecting the missing repos.

[carlosalberto]

@arminru Please rebase so we can merge it.