Drop Tracer() method from Span API

[YuriiBarninets]

Having Tracer() method within Span API introduces a kind of confusion in terms of delegating calls to a new Tracer once TracerProvider was configured.
For better understanding let's take a look at the following code:

package main

import (
	"context"
	"go.opentelemetry.io/otel"
	sdk "go.opentelemetry.io/otel/sdk/trace"
)

func main() {
	proxyTracer := otel.Tracer("Default Tracer")
	_, span := proxyTracer.Start(context.Background(), "Span A")
	spanTracer := span.Tracer();

	otel.SetTracerProvider(sdk.NewTracerProvider()) // set SDK TracerProvider

	proxyTracer.Start(context.Background(), "Span B") // SDK Span will be created
	spanTracer.Start(context.Background(), "Span C") // noopSpan will be created, even though SDK TracerProvider is installed
}

The problem is that spanTracer := span.Tracer(); will return an instance of noopTracer instead of an instance of internal tracer that supports a possibility to delegate calls to an installed one when TracerProvider was configured.
As a result, after SDK TracerProvider was installed we still get a noopSpan upon this call spanTracer.Start(context.Background(), "Span C").

I have checked Span interface of other techs and have not found Tracer() methods.
Here are links to other techs: JavaScript Span API, Java Span API, PHP Span API, .NET Span API.

My proposal is to remove Tracer() method from Span API or return the internal tracer upon span.Tracer(); call for noopSpan

[MrAlias]

This is a bug. The no-op span needs to return the Tracer that created it instead of a generic no-op Tracer implementation.

opentelemetry-go/trace/noop.go

Line 78 in d20e722

func (noopSpan) Tracer() Tracer { return noopTracer{} }

[YuriiBarninets]

You know, it depends on from which point of view you are looking at the Span creation process.
If you are looking from the point of the internal tracer implementation then the Span was created by noopTracer, since internal Tracer redirects call to noopTracer.
If you are looking from the perspective of API consumers then, of course, it is a bug, since Client does not know anything about the internal Tracer placeholder.

Let me formalize it, so for this code

...
_, noopSpan := internalTracer.Start(context.Background(), "Noop Span")
tracerFromSpan := noopSpan.Tracer();
assert(internalTracer == tracerFromSpan);

Client is expecting to get that internalTracer object is equal to tracerFromSpan object since noopSpan was created by internalTracer.

[MrAlias]

Tracking with #1892

[Aneurysm9]

I'm not sure that trying to fix this is the right approach. I've always felt wary of the pattern I've seen some people use where they obtain a span from an incoming context and use that span's tracer to start another span. Since a tracer should contain a resource identifying the instrumentation library and version it is likely that re-using a tracer from a span created by a different library will result in incorrect identification of the source of the span. Removing the Tracer() method from the span will force instrumentation authors to obtain and utilize their own tracer, which should at the least have not incorrect information.

[MrAlias]

For some reason I though the Tracer method was a part of the specification, it is not. Without this requirement from the specification, I agree we should go in the direction of removing this method.

[PeterFeicht]

I would argue that the current implementation is the right thing to do. The Tracer method's documentation says it returns the tracer that created the span, which it does. As a client I wouldn't expect the Tracer method to suddenly return a different tracer just because I configured a tracer provider, because that doesn't change the tracer that created the span.

I don't know the use case that prompted adding the Tracer method to the Span interface. But one use case I can imagine is that of tracing a transaction through multiple spans from beginning to end. In that case, changing the tracer from a noop tracer to a non-noop tracer in the middle of the transaction seems surprising, because spans for the transaction will start somewhere in the middle.

I can understand someone wanting to switch to the newly configured tracer, and that's possible by using a tracer obtained via otel.Tracer. I also realize that this is probably a corner case that rarely anybody will encounter in real life, just wanted to add my two cents.