diff --git a/client/hijack.go b/client/hijack.go index d1ad96fcfe..573fe157fb 100644 --- a/client/hijack.go +++ b/client/hijack.go @@ -16,7 +16,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" - "go.opentelemetry.io/otel/semconv/v1.17.0/httpconv" "go.opentelemetry.io/otel/trace" ) @@ -66,7 +65,8 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, } ctx, span := tp.Tracer("").Start(ctx, req.Method+" "+req.URL.Path, trace.WithSpanKind(trace.SpanKindClient)) - span.SetAttributes(httpconv.ClientRequest(req)...) + // FIXME(thaJeztah): httpconv.ClientRequest is now an internal package; replace this with alternative for semconv v1.21 + // span.SetAttributes(httpconv.ClientRequest(req)...) defer func() { if retErr != nil { span.RecordError(retErr) @@ -98,7 +98,27 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, // Server hijacks the connection, error 'connection closed' expected resp, err := clientconn.Do(req) if resp != nil { - span.SetStatus(httpconv.ClientStatus(resp.StatusCode)) + // This is a simplified variant of "httpconv.ClientStatus(resp.StatusCode))"; + // + // The main purpose of httpconv.ClientStatus() is to detect whether the + // status was successful (1xx, 2xx, 3xx) or non-successful (4xx/5xx). + // + // It also provides complex logic to *validate* status-codes against + // a hard-coded list meant to exclude "bogus" status codes in "success" + // ranges (1xx, 2xx) and convert them into an error status. That code + // seemed over-reaching (and not accounting for potential future valid + // status codes). We assume we only get valid status codes, and only + // look at status-code ranges. + // + // For reference, see: + // https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/v1.17.0/httpconv/http.go#L85-L89 + // https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/internal/v2/http.go#L322-L330 + // https://github.com/open-telemetry/opentelemetry-go/blob/v1.21.0/semconv/internal/v2/http.go#L356-L404 + code := codes.Unset + if resp.StatusCode >= http.StatusBadRequest { + code = codes.Error + } + span.SetStatus(code, "") } //nolint:staticcheck // ignore SA1019 for connecting to old (pre go1.8) daemons diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 5a9b132557..d356566c2f 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -63,6 +63,7 @@ import ( "github.com/spf13/pflag" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/sdk/resource" "tags.cncf.io/container-device-interface/pkg/cdi" ) @@ -238,6 +239,10 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { setOTLPProtoDefault() otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) + + // Override BuildKit's default Resource so that it matches the semconv + // version that is used in our code. + detect.Resource = resource.Default() detect.Recorder = detect.NewTraceRecorder() tp, err := detect.TracerProvider() diff --git a/testutil/helpers.go b/testutil/helpers.go index b149142592..f31ad27944 100644 --- a/testutil/helpers.go +++ b/testutil/helpers.go @@ -16,7 +16,7 @@ import ( "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/sdk/resource" "go.opentelemetry.io/otel/sdk/trace" - semconv "go.opentelemetry.io/otel/semconv/v1.17.0" + semconv "go.opentelemetry.io/otel/semconv/v1.21.0" "gotest.tools/v3/icmd" ) @@ -34,7 +34,7 @@ func (d devZero) Read(p []byte) (n int, err error) { var tracingOnce sync.Once -// configureTracing sets up an OTLP tracing exporter for use in tests. +// ConfigureTracing sets up an OTLP tracing exporter for use in tests. func ConfigureTracing() func(context.Context) { if os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT") == "" { // No OTLP endpoint configured, so don't bother setting up tracing. @@ -52,9 +52,7 @@ func ConfigureTracing() func(context.Context) { tp = trace.NewTracerProvider( trace.WithSpanProcessor(sp), trace.WithSampler(trace.AlwaysSample()), - trace.WithResource(resource.NewSchemaless( - attribute.KeyValue{Key: semconv.ServiceNameKey, Value: attribute.StringValue("integration-test-client")}, - )), + trace.WithResource(resource.NewSchemaless(semconv.ServiceName("integration-test-client"))), ) otel.SetTracerProvider(tp)