is_valid is currently a method on the SpanContext, the spec defines it as a bool flag

[codeboten]

To be compliant with the tracing spec, we need a is_valid to be a boolean flag rather than a method on the SpanContext

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#spancontext

[ffe4]

In the linked document it is simply described as "an API that returns a boolean value [...]". What would be the difference between a flag and a method here? Should it simply be defined as an attribute during instantiation, as is the case with is_remote?

[codeboten]

@ffe4 right, the discrepancy between is_remote and is_valid is what I meant to address with this issue, as they're defined in the spec as the same thing, but they're implemented differently.

[lzchen]

@codeboten
@ffe4

Actually I would think "returns" indicates that it is a method. So instead of changing is_valid into a property, I would think we should change is_remote to a function. Java seems to be taking this approach.

[ffe4]

@lzchen In Java they also use getter methods, so using a method might be more natural. However, Go and Ruby also seem to use methods.

We can also have them as properties, with the same semantics but an actual "return". In that sense, I think having them as data attributes / properties would be more flexible.

Another thought might be to have the same semantics everywhere in the codebase, but while Status.is_ok is a property, Span.is_recording_events is a method. Also see specification for IsRecording and GetIsOk.

I feel like properties are more idiomatic for boolean flags, but that might just be because of the libraries I'm using (e.g. requests)

[ffe4]

Another note while we are on the topic of wording. The specification explicitly uses flag for TraceFlags.sampled, SpanContext.IsRemote and Span.IsRecording, but not for SpanContext.IsValid. The former two have similar semantics, being a property and a data attribute respectively, while the latter two are both methods.