detect: use newer semconv for resource and add unit test

[jsternberg]

We upgraded a dependency that upgraded otel and also changed the semconv schema for us to a newer version. We forgot to upgrade our own semconv.

In the future, we may want to find a way to have a detector that doesn't require us to manually specify a semconv so we don't have to remember this. For now, I've added a test to catch when this happens so it doesn't happen again.

[crazy-max]

Should we consider bumping

buildkit/util/tracing/tracing.go

Line 14 in 9d81766

semconv "go.opentelemetry.io/otel/semconv/v1.21.0"

?

[tonistiigi]

@jsternberg PTAL the another import pointed out by @crazy-max

@crazy-max Last time we discussed it, we thought that semconv should not be updated too frequently to avoid errors where old releases does not understand new traces. Talking to @jsternberg that should not be an issue. Did you have any examples for the cases that might be broken. Eg. need to make sure that old Desktop still works with new buildx/buildkit and for example that traces from a new buildx can still be recorded in the history record of old buildkit.

[crazy-max]

@crazy-max Last time we discussed it, we thought that semconv should not be updated too frequently to avoid errors where old releases does not understand new traces. Talking to @jsternberg that should not be an issue. Did you have any examples for the cases that might be broken. Eg. need to make sure that old Desktop still works with new buildx/buildkit and for example that traces from a new buildx can still be recorded in the history record of old buildkit.

Yes I recall our discussion where we needed a pretty old version of Docker Desktop but we didn't have exportable traces feature available back then. I can try with changes from this PR and see what happens. Keep you posted.

[crazy-max]

@jsternberg PTAL the another import pointed out by @crazy-max

There is also the containerd one from vendor:

$ git grep 'go.opentelemetry.io/otel/semconv/v' -- '*.go'
util/tracing/detect/resource.go:        semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
util/tracing/tracing.go:        semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
vendor/github.com/containerd/containerd/tracing/tracing.go:     semconv "go.opentelemetry.io/otel/semconv/v1.21.0"

I guess we would need some coordination in the future (cc @dmcgowan)

[crazy-max]

I can try with changes from this PR and see what happens. Keep you posted.

Ok seems it works fine with changes from this PR with a buildkit container:

• traces-otlp-4042aab40a5226e67da08f133d8b6ee5.json
• traces-jaeger-4042aab40a5226e67da08f133d8b6ee5.json

I see telemetry.sdk.version sets to 1.28.0 (current OTEL):

      {
        "Key": "telemetry.sdk.version",
        "Value": {
          "Type": "STRING",
          "Value": "1.28.0"
        }
      }

Will check with Buildx vendoring this branch.

[jsternberg]

My understanding is that different semconv versions being used by different products isn't an error or something that causes a problem. This message only comes from conflicting schemas being merged into a single one with resources and individual tracers can set their own schema version so we shouldn't need to coordinate with containerd.

It might be useful though if we consolidate how we're using otel though to reduce friction in the future. I might try to mock something up of what that might look like soon.

I've updated all semconv references in this project to the same version.

[crazy-max]

Will check with Buildx vendoring this branch.

Good as well with Buildx.

[tonistiigi]

There are still imports to semconv/v1.21 via containerd and otel/exporters/jeager . Can these cause an issue?

[jsternberg]

@tonistiigi no it cannot. The issue only shows up when used directly with a resource through the resource.Merge method which gets invoked by resource.New. semconv used with individual tracers can't cause an issue and it's expected that different tracers can use different schemas.