Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add embedded package to trace API #4545

Closed
MrAlias opened this issue Sep 22, 2023 · 6 comments · Fixed by #4620
Closed

Add embedded package to trace API #4545

MrAlias opened this issue Sep 22, 2023 · 6 comments · Fixed by #4620
Assignees
Labels
area:trace Part of OpenTelemetry tracing pkg:API Related to an API package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Sep 22, 2023

Similar to the metric embedded package, add one for the trace API exported interfaces.

This will, similarly, force SDK developers to make a choice on how they want unimplemented behavior to be handled.

@MrAlias MrAlias added pkg:API Related to an API package area:trace Part of OpenTelemetry tracing labels Sep 22, 2023
@MrAlias MrAlias added this to the v1.20.0 milestone Sep 22, 2023
@MrAlias MrAlias self-assigned this Oct 11, 2023
@MrAlias
Copy link
Contributor Author

MrAlias commented Oct 12, 2023

The current no-op trace implementation lives in go.opentelemetry.io/otel/trace and is not fully exported. The only way to access it is via NewNoopTracerProvider which returns a trace.TracerProvider. This means a user will not be able to directly embed a no-op implementation with the current layout.

I'm working on a solution to this that will deprecate the NewNoopTracerProvider function in favor of an added go.opentelemetry.io/otel/trace/noop package. That package will contain an TracerProvider, Tracer, and Span no-op structs.

@MrAlias
Copy link
Contributor Author

MrAlias commented Oct 12, 2023

There is still an open question about the no-op package design in how the Tracer.Start method will work. Currently, the noop implementation matches the global default implementation in that it will propagate a span context for a non-zero context in a non-recording span (though, that span needs to explicitly be a nonRecordingSpan type currently which is likely not what the spec intended). However, given the spec only defines the default API behavior when no SDK is set (i.e. the default global implementation) to be this behavior I don't know if we should continue providing this in the noop package.

It should be easy enough for users who want this propagation behavior to wrap the noop implementation with one that does it.

@pellared
Copy link
Member

Is this not related to the comment above #4027?

@MrAlias
Copy link
Contributor Author

MrAlias commented Oct 12, 2023

From SIG meeting:

  • We want the default no-op behavior to propagate context for spans with span context
  • If a user wants to disable this feature they can wrap the no-op implementation and disable it with method overrides of their own

@EdSchouten
Copy link

EdSchouten commented Nov 11, 2023

I had some code of mine where I used gomock to stub out implementations of TracerProvider/Tracer/Span. This now no longer works, because these interfaces contain private methods. The stubs that gomock generates obviously don't contain the private methods. Thanks.

@micronull
Copy link

@EdSchouten We have the same problem.

@open-telemetry open-telemetry locked as resolved and limited conversation to collaborators Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:trace Part of OpenTelemetry tracing pkg:API Related to an API package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants