Check the protected code in SDK Span methods that use self._lock

[ocelotl]

This started when discussing #358, @codeboten and @toumorokoshi raise a valid concern regarding what should and should not be protected by self._lock in several methods of the SDK Span class.

Take a particular good look into this.

[Oberon00]

You linked the is_recording_events method here, which just returns the literal True: https://github.com/open-telemetry/opentelemetry-python/pull/358/files#diff-2f7b55162ad705df55ee48eb71914737L280-L281

Was this intentional? If so, how is this related?

[ocelotl]

Yes, because of this. I am a bit surprised by the implementation of the is_recording_events method, why do we bother asking if the span is recording events if it always is?

[Oberon00]

Only a SDK.Span is always recording events. A DefaultSpan is not (the SDK returns a default span for spans that are sampled out, for example). Some 3rd party API-implementation may even have a more complex logic to determime if a span is recording events
(BTW, this method was renamed to just isRecording in the spec).

[ocelotl]

So, what's the idea behind having this in the SDK code of Span.start if it will always be True? Are we having it there so that subclasses that may override is_recording_events will also have that check in place for their corresponding start methods?

[Oberon00]

They currently don't really serve a purpose, I guess. In this case the blame view of the file would have helped you since I commented on that exact issue in the PR that added the ckeck (and is_recording_events): #141 (comment)

[lzchen]

Closing due to inactivity.