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

Update the no-op trace implementations to support auto-instrumentation #5702

Closed
MrAlias opened this issue Aug 9, 2024 · 0 comments · Fixed by #5920
Closed

Update the no-op trace implementations to support auto-instrumentation #5702

MrAlias opened this issue Aug 9, 2024 · 0 comments · Fixed by #5920
Assignees
Labels
enhancement New feature or request proposal
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Aug 9, 2024

Problem Statement

The auto-instrumentation project currently instruments the nonRecordingSpan and tracer within the go.opentelemetry.io/otel/internal/global package. Due to the limitations of the auto-instrumentation mentioned in open-telemetry/opentelemetry-go-instrumentation#954, the full functionality of the Span returned from the global package is not instrumentable.

Proposed Solution

Within this proof-of-concept, it was shown that we should be able to have the auto-instrumentation direct the global SDK to run different code-paths when it is installed. With this understanding we are proposing the following:

  1. Add a tracing SDK implementation to the auto-instrumentation project. This will be designed to fit the needs of the auto-instrumentation.
  2. Update the the tracer in go.opentelemetry.io/otel/internal/global to use the auto-instrumentation SDK from (1) when the auto-instrumentation is installed.
  3. Update trace.SpanFromContext similar to go.opentelemetry.io/otel/internal/global so an auto-instrumentation Span is returned when it is installed.

The first part of the proposal is out-of-scope for this repository. We expect to provide an API that allows for the creation of a Span implementation with a signature as follows.

func NewSpan(ctx context.Context, tracerName string, tracerConfig *trace.TracerConfig, spanName string, spanConfig trace.SpanConfig) (context.Context, trace.Span)

The name of the new auto-instrumentation package is not finalized, but for this proposal we are calling it "go.opentelemetry.io/auto/interop".

The second part, we expect to submit a PR with the following changes:

diff --git a/internal/global/trace.go b/internal/global/trace.go
index e31f442b..70609845 100644
--- a/internal/global/trace.go
+++ b/internal/global/trace.go
@@ -25,6 +25,8 @@ import (
        "sync"
        "sync/atomic"

+       "go.opentelemetry.io/auto/interop"
+
        "go.opentelemetry.io/otel/attribute"
        "go.opentelemetry.io/otel/codes"
        "go.opentelemetry.io/otel/trace"
@@ -115,6 +117,9 @@ type tracer struct {
        opts     []trace.TracerOption
        provider *tracerProvider

+       cfg     *trace.TracerConfig
+       cfgOnce sync.Once
+
        delegate atomic.Value
 }

@@ -139,6 +144,32 @@ func (t *tracer) Start(ctx context.Context, name string, opts ...trace.SpanStart
                return delegate.(trace.Tracer).Start(ctx, name, opts...)
        }

+       return t.newSpan(ctx, autoInstEnabled, name, opts)
+}
+
+// autoInstEnabled determines if the auto-instrumentation interop span is
+// returned from the tracer when not backed by a delegate and
+// auto-instrumentation has attached to this process.
+//
+// The auto-instrumentation is expected to overwrite this value to true when it
+// attaches. By default, this will point to false and mean a tracer will return
+// a nonRecordingSpan by default.
+var autoInstEnabled *bool = new(bool)
+
+func (t *tracer) newSpan(ctx context.Context, autoSpan *bool, name string, opts []trace.SpanStartOption) (context.Context, trace.Span) {
+       if *autoSpan {
+               // auto-instrumentation is not able to resolve options into
+               // configuration structs. Process the options before creating the
+               // [interop.Span].
+               t.cfgOnce.Do(func() {
+                       // Lazy initialize the tracer configuration only once.
+                       cfg := trace.NewTracerConfig(t.opts...)
+                       t.cfg = &cfg
+               })
+               cfg := trace.NewSpanStartConfig(opts...)
+               return interop.NewSpan(ctx, t.name, t.cfg, name, cfg)
+       }
+
        s := nonRecordingSpan{sc: trace.SpanContextFromContext(ctx), tracer: t}
        ctx = trace.ContextWithSpan(ctx, s)
        return ctx, s

A similar approach would also be implemented for (3).

Performance Analysis

TODO

Alternatives

  1. We could investigate an alternate modification of the global package if issues are identified with the proposed solution.
  2. We could do nothing and have auto-instrumentation not provide full functionality to users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request proposal
Projects
Development

Successfully merging a pull request may close this issue.

2 participants