open-telemetry / opentelemetry-go

OpenTelemetry Go API and SDK
https://opentelemetry.io/docs/languages/go
Apache License 2.0
5.23k stars 1.05k forks source link

go.opentelemetry.io/otel/sdk/metric: meter provider created with `WithResource` does not honor resource-related environment variables (OTEL_RESOURCE_ATTRIBUTES) #5764

Closed basti1302 closed 1 month ago

basti1302 commented 1 month ago

Description

Consider a meter provider created like this:

theResource, err := resource.New(ctx, resource.WithAttributes(semconv.K8SNodeName("my-k8s-node-name")))

...

meterProvider := metric.NewMeterProvider(
    metric.WithResource(theResource),
    metric.WithReader(metric.NewPeriodicReader(metricExporter,
        metric.WithInterval(3*time.Second))),
)

Also, assume the process has been started with resource attributes being provided via environment variables, e.g. OTEL_RESOURCE_ATTRIBUTES.

The metrics produced by this meter provider will only have the resource attributes from the in-code configuration, the key-value pairs from the environment variable will not be used.

If I read the spec correctly, the env vars should be merged with values provided via in-code config. This is also what happens when creating a tracer provider in a similar fashion. At the very least, the behavior is inconsistent between meter provider and tracer provider (I did not test logger provider yet).

Links to sources:

I've created an SSCCE/reproducer here: https://github.com/basti1302/opentelemetry-go-metric-resource-reproducer. Its README lists the produced metrics and spans. Here is an excerpt:

metric:

{
  "Resource": [
    {
      "Key": "k8s.node.name", // <- Wrong: Only the attributes from the in-code config are here.
      "Value": {
        "Type": "STRING",
        "Value": "my-k8s-node-name"
      }
    }
  ],
  "ScopeMetrics": [...]
}

span: 

{
    "Name": "/",
        ...
    "Resource": [             // <- Correct: The attributes from the environment variable are used here as well.
        {
            "Key": "k8s.node.name",
            "Value": {
                "Type": "STRING",
                "Value": "my-k8s-node-name"
            }
        },
        {
            "Key": "service.name",
            "Value": {
                "Type": "STRING",
                "Value": "my-service-name"
            }
        },
        {
            "Key": "service.namespace",
            "Value": {
                "Type": "STRING",
                "Value": "my-service-namespace"
            }
        },
        {
            "Key": "service.version",
            "Value": {
                "Type": "STRING",
                "Value": "1.2.3"
            }
        }
    ],
...
}

Environment

Steps To Reproduce

See https://github.com/basti1302/opentelemetry-go-metric-resource-reproducer/blob/main/README.md

Expected behavior

The meter provider should merge key-value pairs from the environment with the in-code configuration.

basti1302 commented 1 month ago

For the record, I would also be interested in contributing a fix. But before working on that I would like a second opinion on the desired behavior - e.g. is the merging happening in trace provider correct (I thin so) or is meter provider's behavior correct.

dmathieu commented 1 month ago

I agree that merging both resources should be the proper behavior.

basti1302 commented 1 month ago

Created https://github.com/open-telemetry/opentelemetry-go/pull/5773 which will fix this for metrics as well as for logs, where it was equally broken.