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

Incorrect API behavior in absence of SDK #1893

Closed
MrAlias opened this issue May 7, 2021 · 5 comments · Fixed by #1901
Closed

Incorrect API behavior in absence of SDK #1893

MrAlias opened this issue May 7, 2021 · 5 comments · Fixed by #1901
Assignees
Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented May 7, 2021

Description

The specification states

In general, in the absence of an installed SDK, the Trace API is a "no-op" API.
This means that operations on a Tracer, or on Spans, should have no side effects and do nothing.
However, there is one important exception to this general rule, and that is related to propagation of a SpanContext:
The API MUST create a non-recording Span with the SpanContext that is in the Span in the parent Context (whether explicitly given or implicit current) or, if the parent is a non-recording Span (which it usually always is if no SDK is present), it MAY return the parent Span back from the creation method.
If the parent Context contains no Span, an empty non-recording Span MUST be returned instead (i.e., having a SpanContext with all-zero Span and Trace IDs, empty Tracestate, and unsampled TraceFlags).
This means that a SpanContext that has been provided by a configured Propagator will be propagated through to any child span and ultimately also Inject, but that no new SpanContexts will be created.

Spans created in a trace tree that originates with a propagator correctly implement this, they are, at the root, a non-recording span (that is stored in a context). Spans created via a Tracer prior to an SDK being set that implements the API do not. They only are no-op spans that do not handle any part of the `SpanContext.

Expected behavior

The API correctly implements the specification and will generate a non-recording span with the global Tracer prior to an SDK being set.

@MrAlias MrAlias added bug Something isn't working area:trace Part of OpenTelemetry tracing labels May 7, 2021
@MrAlias MrAlias added this to the RC1 milestone May 7, 2021
@MrAlias MrAlias self-assigned this May 7, 2021
@MrAlias
Copy link
Contributor Author

MrAlias commented May 7, 2021

Validation test:

package otel

import (
	"context"
	"testing"

	"go.opentelemetry.io/otel/internal/global"
	"go.opentelemetry.io/otel/trace"
)

func TestSpanContextPropagated(t *testing.T) {
	global.ResetForTest()

	sc := trace.NewSpanContext(trace.SpanContextConfig{
		TraceID:    [16]byte{0x01},
		SpanID:     [8]byte{0x01},
		TraceFlags: trace.FlagsSampled,
	})
	ctx := trace.ContextWithSpanContext(context.Background(), sc)
	_, span := Tracer("test").Start(ctx, "test")

	if !span.SpanContext().Equal(sc) {
		t.Error("span context not propagated by default")
	}
}

@dashpole
Copy link
Contributor

Does this address #877?

Based on my reading of it, it means that if I have, for example:

otelhttp.Handler.ServeHttp -> StartSpan(ctx, ...) -> otelhttp.Transport.RoundTrip

If I don't register an SDK, but do register a propagator, we would still extract context from http headers. Then, the StartSpan calls in the handler, code, and transport don't change the SpanContext in the context. Then, the context is encoded into outgoing http headers in the transport. The tracecontext headers in the incoming request should match the headers in the outgoing request, right? Or am I reading this incorrectly?

@MrAlias
Copy link
Contributor Author

MrAlias commented May 12, 2021

If I don't register an SDK, but do register a propagator, we would still extract context from http headers. Then, the StartSpan calls in the handler, code, and transport don't change the SpanContext in the context. Then, the context is encoded into outgoing http headers in the transport. The tracecontext headers in the incoming request should match the headers in the outgoing request, right? Or am I reading this incorrectly?

Yes, this is correct. 👍

Does this address #877?

Yeah, I think it does. @dashpole based on your understanding outlined above, should I resolve that issue with #1901 as well?

@dashpole
Copy link
Contributor

Yes, you should. It would be nice to document the "Passthrough" use-case, and potentially provide an example

@MrAlias
Copy link
Contributor Author

MrAlias commented May 12, 2021

It would be nice to document the "Passthrough" use-case, and potentially provide an example

Tracking with #1908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants