Add experimental support for AddLinks()

[austindrenski]

This commit introduces a new ExperimentalSpan interface which aims to allow the API and SDK to support experimental features in a way that clearly communicates to users that they are opting into an API surface that is still considered experimental by the spec and thus may break, change, or be outright removed with minimal notice.

In the case of AddLinks(), users can explicitly type-check for the new interface, making it obvious that they are interacting with an API surface that is considered experimental:

if s, ok := span.(trace.ExperimentalSpan); ok {
        s.AddLinks(links...)
}

See: #3919

---

Note

Go isn't my usual language, so if you see something unconventional/antipattern-esque, please assume its ignorance vs opinion, and call it out. More interested in the behind idea of this PR than the specific impl. seen here, so happy to adjust as needed.

Topics for review

• Does ExperimentalSpan sound sufficiently foreboding as to communicate that you're interacting with unstable code? Or is there a better way to communicate this?
• If the premise for this PR is accepted, should we also update the OpenCensus bridge to use it, thus removing one of the documented incompatibilities?

  • opentelemetry-go/bridge/opencensus/doc.go

    Lines 47 to 52 in e3eb3f7

    	// # Limitations
    	//
    	// There are known limitations to the trace bridge:
    	//
    	// - The AddLink method for OpenCensus Spans is ignored, and an error is sent
    	// to the OpenTelemetry ErrorHandler.

  • opentelemetry-go/bridge/opencensus/internal/span.go

    Lines 123 to 126 in e3eb3f7

    	// AddLink adds a link to this span.
    	func (s *Span) AddLink(l octrace.Link) {
    	Handle(fmt.Errorf("ignoring OpenCensus link %+v for span %q because OpenTelemetry doesn't support setting links after creation", l, s.String()))
    	}

[MrAlias]

Thank you for your interest in the project. However, we cannot accept these changes. This is making changes to APIs and interfaces in stable modules. The type and method added are not stable. We do not plan to support these additions indefinitely and therefore cannot accept something like this.

Please feel free to make these changes in your own fork.

If you would like to propose another design to add these (as mentioned here) please do so in the mentioned issue prior to submitting a PR.

[pellared]

Something like below would not require making any changes in the API.

Suggested change

	if s, ok := span.(trace.ExperimentalSpan); ok {
	s.AddLinks(links...)
	}
	if s, ok := span.(interface {
	AddLinks(links ...Link)
	}); ok {
	s.AddLinks(links...)
	}

[MrAlias]

It will still, however, require the AddLinks method to be exported on an implementation. Something that would immediately need to be stable given the state of the SDK version.

We cannot accept a method that we know will conflict with the eventual method needed to be added to support the API when it is extended to include an AddLinks method.

[pellared]

recordingSpan type (implementation) is not exported so it will not be an "ABI change".

If the signature will change then simply ok will be false.

EDIT:

The only way people would learn about such experimental functionality is via SDK docs.

However, I do not see a lot of demand for this issue so I would rather wait for the spec to mark this stable.

[austindrenski]

I considered that, and think it could be a viable alternative to a formal interface, but the tradeoff is how easy it is for users who have opted in to realize we've dropped/changed support.

With type-checking for ExperimentalSpan, users would get a compilation error, whereas type-checking for interface { AddLinks(links ...Link) }, it could go unnoticed that we've changed or removed this signature altogether.

But, if the project policies won't allow for this sort of non-stable interface in a stable module, as @MrAlias alluded to in #4889 (comment), then perhaps it would just be an added requirement for users trying to opt into these surfaces to log their own warnings for if/when they go missing:

	if s, ok := span.(interface {
		AddLinks(links ...Link)
	}); ok {
		s.AddLinks(links...)
-	}
+	} else {
+		// user is responsible for logging a warning that the type-check failed 
+	}

[MrAlias]

So someone who is using a v1 version of a Go package and does what you propose above in their code then upgrades that package to the next minor or patch version but has their code fail to compile because that method had changed or been removed wouldn't have a valid argument to make that we violated our versioning and stability guarantees(!)?

I'm sorry, but no. That is not acceptable and we can talk more about it in the next SIG meeting if it needs clarification.

[pellared]

but has their code fail to compile because that method had changed or been removed

For clarification, with anonymous interface in type assertion it will continue to compile.

[austindrenski]

So someone who is using a v1 version of a Go package and does what you propose above in their code then upgrades that package to the next minor or patch version but has their code fail to compile because that method had changed or been removed wouldn't have a valid argument to make that we violated our versioning and stability guarantees(!)?

Well, that's kind of the entire idea I was trying to navigate with this PR: can this project ship opt-in features where part of the contract for opting in is knowing that the opt-in features are not subject to the same stability guarantees.

Fully respect that you disagree with the objective, but this^ is explicitly what this PR was proposing, providing a mechanism for users to opt-into features that might break their build next time they upgrade.

I'm sorry, but no. That is not acceptable and we can talk more about it in the next SIG meeting if it needs clarification.

Sure, though, I think you might be coming at this just a little hot, given that this PR (should've hit the draft button, I suppose) was just meant for discussion. Was in no way expecting this PR to actually land in main as-is, or necessarily at all, was just looking to discuss some ideas.

Something that would be useful as a happy-and-grateful community member would be for some SIG clarification on why some language SDKs are more or less willing to support experimental features (e.g. we've been able to AddLinks() from the dotnet SDK for a while now).

[austindrenski]

Ope, sorry @MrAlias, was just trying to open this for discussion in conjunction with the discussion over in #3919. Figured it would be easier to look at the diffs in context than just tossing them into comments, but noted for the future.