opentelemetry-go-contrib
opentelemetry-go-contrib copied to clipboard
Hard coding semconv.SchemaURL references, inconsistencies and brittleness
Hi y'all, hopefully this isn't the wrong place to raise this discussion.
I'm making use of the AWS detectors and it's the root cause of issues I'm facing, in part because the SchemaURL of the detector is hardcoded, and the OTEL specification explicitly negates cross-schema connection of resources.
Adopting a different semconv schema url in my application's instrumentation (in this instance v1.18.0) causes breaking to occur due to:
failed to merge resources: cannot merge resource due to conflicting Schema URL
Code details
// OTEL resource.
resource, err := sdkresource.Detect(ctx)
if err != nil {
log.Printf("failed to detect lambda resources: %s\n", err)
}
// AWS resource.
awsResource, err := lambdadetector.NewResourceDetector().Detect(ctx)
if err != nil {
log.Printf("failed to detect lambda resources: %s\n", err)
}
resource, err = sdkresource.Merge(resource, awsResource)
if err != nil {
return nil, fmt.Errorf("failed to merge resources: %w", err)
}
// Add the deployment environment to the resource.
deployEnv := os.Getenv("DEPLOYMENT_ENV")
if deployEnv == "" {
return nil, ErrDeploymentEnvNil
}
attrs := []attribute.KeyValue{
semconv.DeploymentEnvironment(deployEnv),
semconv.ServiceInstanceID(attrService.InstanceID),
semconv.ServiceName(attrService.Name),
semconv.ServiceNamespace(attrService.Namespace),
semconv.ServiceVersion(attrService.Version),
}
deploymentResource := sdkresource.NewWithAttributes(semconv.SchemaURL, attrs...)
resource, err = sdkresource.Merge(deploymentResource, resource)
if err != nil {
return nil, fmt.Errorf("failed to merge resources: %w", err)
}
Now, while I can appreciate there is a level of alignment required to schema changes, there should be the opportunity to either "migrate" a resource to a different schema, or at the very least, the documentation around available detectors (or anything else with explicit semconv.SchemaURL references) should be documented.
https://github.com/open-telemetry/opentelemetry-go/issues/2341
As it stands, the current most up to date release of the Schema URL is v1.18.0, and there's no possible way to ask a detector (as it stands) to return a resource with a defined schema (for example, as an attribute). Not only that, the schemas used in this repo also vary (it's not all v1.17.0), and dependency updates don't seem to really support patched releases (for example, security related) for older semconv schema urls.
I have a draft PR that I'll link shortly that removes the explicit semconv.SchemaURL references, however I'm wondering if that's considered "bad form" from an Open Telemetry for Golang "best practice".
Seeking guidance and advice (other than "use v1.17.0" – until something else breaks because the brittleness of the implementation of OTEL prevents mixed schema operation.
I have a draft PR that I'll link shortly that removes the explicit semconv.SchemaURL references, however I'm wondering if that's considered "bad form" from an Open Telemetry for Golang "best practice".
This does not sound like changes we can accept. The schema compatibility of a resource is something defined by the specification. Not checking that compatibility would be invalid behavior.
It sounds like schema translation is what is needed here.
This does not sound like changes we can accept. The schema compatibility of a resource is something defined by the specification. Not checking that compatibility would be invalid behavior.
Thanks for the feedback @MrAlias. I suspected that to be the case (regarding non-acceptance).
Equally so, should there not be any "opinion" on specific versions of the schema URL adopted by packages held within here – or at the very minimum, function arguments to select a responding resource with a specific schema URL?
Examples I would highlight include:
- https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/detectors/aws
- https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/detectors/gcp
Both these explicitly make use of the schema URL v1.17.0, thereby rendering any attempt to make use of other detectors returned resources at another version.
As noted elsewhere, an approach that hard codes schema URLs in place within packages inhibits safely upgrading applications that make use of those packages as all parts of an application will need to be amended in a single hit. Further, without having branches + tags with management for different schema URLs, security management of dependencies of this package then is tied to schema URL management.
Thinking through this the last couple of weeks it screams at me that any function returning a resource should be required, by convention within the Go implementation of OTel, to provide the requested schema URL that is desired to be used in the returned resource. Is this something that the otel-go (contrib or core) management team been investigating?
I note that even the core library explicitly sets the expected resource schema its own detector uses.
https://github.com/open-telemetry/opentelemetry-go/blob/main/sdk/resource/container.go#L50
This would tend to indicate that any other schema is invalid according to the core lib for that particular release. It would also imply, wrt semantic versioning (at the least) that a shift to v1.18.0 schema URL would be accompanied by a major release bump. Yet as we see, those bumps do occur at the minor release level (the last schema URL before change to v1.17.0, which was v1.12.0).
Here's a new repo I created to handle schema translations: https://github.com/MrAlias/otel-schema-utils
Only thing supported currently is resource translations.
Similar issue https://github.com/open-telemetry/opentelemetry-go/issues/3769
And another round of #footgun emerges :(
https://github.com/open-telemetry/opentelemetry-go-contrib/pull/4299
until something else breaks because the brittleness of the implementation of OTEL prevents mixed schema operation
Here is the full rundown of the differing semconvs hard coded in the current v1.20.0 release, which shows even a release here is incompatible with itself – even within a single "detector" grouping (gcp):
v1.4.0
instrgen/rtlib/rtlib.go
v1.17.0
detectors/gcp/cloud-function.godetectors/gcp/cloud-run.godetectors/gcp/gce.godetectors/gcp/cloud-function_test.godetectors/gcp/gke.goinstrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/httpconv.goinstrumentation/gopkg.in/macaron.v1/otelmacaron/internal/semconvutil/netconv.goinstrumentation/gopkg.in/macaron.v1/otelmacaron/macaron.goinstrumentation/net/http/otelhttp/handler.goinstrumentation/net/http/otelhttp/test/handler_test.goinstrumentation/net/http/otelhttp/example/server/server.goinstrumentation/net/http/otelhttp/example/client/client.goinstrumentation/net/http/otelhttp/internal/semconvutil/httpconv.goinstrumentation/net/http/otelhttp/internal/semconvutil/netconv.goinstrumentation/net/http/httptrace/otelhttptrace/example/server/server.goinstrumentation/net/http/httptrace/otelhttptrace/example/client/client.goinstrumentation/net/http/httptrace/otelhttptrace/clienttrace.goinstrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/httpconv.goinstrumentation/net/http/httptrace/otelhttptrace/internal/semconvutil/netconv.goinstrumentation/net/http/httptrace/otelhttptrace/httptrace.goinstrumentation/net/http/httptrace/otelhttptrace/httptrace_test.goinstrumentation/host/example/main.goinstrumentation/runtime/example/main.goinstrumentation/google.golang.org/grpc/otelgrpc/config.goinstrumentation/google.golang.org/grpc/otelgrpc/test/grpc_test.goinstrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.goinstrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.goinstrumentation/google.golang.org/grpc/otelgrpc/internal/parse_test.goinstrumentation/google.golang.org/grpc/otelgrpc/internal/parse.goinstrumentation/google.golang.org/grpc/otelgrpc/interceptor.goinstrumentation/google.golang.org/grpc/otelgrpc/stats_handler.goinstrumentation/google.golang.org/grpc/otelgrpc/semconv.goinstrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.goinstrumentation/github.com/gorilla/mux/otelmux/mux.goinstrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/httpconv.goinstrumentation/github.com/gorilla/mux/otelmux/internal/semconvutil/netconv.goinstrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/httpconv.goinstrumentation/github.com/gin-gonic/gin/otelgin/internal/semconvutil/netconv.goinstrumentation/github.com/gin-gonic/gin/otelgin/gintrace.goinstrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/httpconv.goinstrumentation/github.com/labstack/echo/otelecho/internal/semconvutil/netconv.goinstrumentation/github.com/labstack/echo/otelecho/echo.goinstrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/httpconv.goinstrumentation/github.com/emicklei/go-restful/otelrestful/internal/semconvutil/netconv.goinstrumentation/github.com/emicklei/go-restful/otelrestful/restful.gointernal/shared/semconvutil/httpconv.go.tmplinternal/shared/semconvutil/netconv.go.tmpl
v1.21.0
detectors/gcp/detector.godetectors/gcp/detector_test.godetectors/aws/ecs/test/ecs_test.godetectors/aws/ecs/ecs_test.godetectors/aws/ecs/ecs.godetectors/aws/eks/detector.godetectors/aws/eks/detector_test.godetectors/aws/lambda/detector.godetectors/aws/lambda/detector_test.godetectors/aws/ec2/ec2_test.godetectors/aws/ec2/ec2.goinstrumentation/github.com/aws/aws-sdk-go-v2/otelaws/attributes.goinstrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.goinstrumentation/github.com/aws/aws-sdk-go-v2/otelaws/attributes_test.goinstrumentation/github.com/aws/aws-sdk-go-v2/otelaws/sqsattributes_test.goinstrumentation/github.com/aws/aws-sdk-go-v2/otelaws/sqsattributes.goinstrumentation/github.com/aws/aws-sdk-go-v2/otelaws/dynamodbattributes.goinstrumentation/github.com/aws/aws-lambda-go/otellambda/test/lambda_test.goinstrumentation/github.com/aws/aws-lambda-go/otellambda/lambda.go