switch to go.opentelemetry.io/otel/semconv/v1.17.0 #46645
Conversation
client/hijack.go
Outdated
| @@ -66,7 +66,8 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, | |||
| } | |||
|
|
|||
| ctx, span := tp.Tracer("").Start(ctx, req.Method+" "+req.URL.Path) | |||
| span.SetAttributes(semconv.HTTPClientAttributesFromHTTPRequest(req)...) | |||
| // TODO(thaJeztah): should this also set trace.SpanKindClient like containerd's tracing package does (or use containerd's tracing.WithHTTPRequest?; https://github.com/containerd/containerd/blob/8c087663b0233f6e6e2f4515cee61d49f14746a8/tracing/tracing.go#L38-L47) | |||
There was a problem hiding this comment.
I stumbled upon that containerd code when looking how the other projects used this code (containerd/containerd@5082fb3), and noticed the code in containerd also set that property;
moby/vendor/github.com/containerd/containerd/tracing/tracing.go
Lines 38 to 47 in 80a9fc6
That code was originally added as part of containerd/containerd@3b87d46#diff-d6dd575a9d10716db9ac385bf5f394a2e65d638311159e8726b3200fcc596989R572-R577, but perhaps someone more familiar with this knows if we should add the same
/cc @cpuguy83 @tonistiigi
There was a problem hiding this comment.
Yes this should set the span kind.
While updating the docker/docker dependency in BuildKit, I noticed that the dependency tree showed _two_ separate versions of the semconv package; BuildKit and containerd were using the v1.17.0 version and docker/docker was using v1.7.0. This patch updates the version we use to align with BuildKit and containerd. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
otel_semconv
branch
from
0bb6b71 to
25fb4dd
Compare
| @@ -65,8 +65,8 @@ func (cli *Client) setupHijackConn(req *http.Request, proto string) (_ net.Conn, | |||
| } | |||
| } | |||
|
|
|||
| ctx, span := tp.Tracer("").Start(ctx, req.Method+" "+req.URL.Path) | |||
| span.SetAttributes(semconv.HTTPClientAttributesFromHTTPRequest(req)...) | |||
| ctx, span := tp.Tracer("").Start(ctx, req.Method+" "+req.URL.Path, trace.WithSpanKind(trace.SpanKindClient)) | |||
There was a problem hiding this comment.
@cpuguy83 I added trace.WithSpanKind(trace.SpanKindClient) here
Do you think we should look at using containerd's tracing package as well (in a follow-up)? I can make create a tracking issue for that if it makes sense to do.
While updating the docker/docker dependency in BuildKit, I noticed that the dependency tree showed two separate versions of the semconv package; BuildKit and containerd were using the v1.17.0 version and docker/docker was using v1.7.0.
This patch updates the version we use to align with BuildKit and containerd.
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)