feat(otel): support standard OTEL resource attribute env vars (#10974)

* fix(otel): pick up standard OTEL_RESOURCE_ATTRIBUTES on the exported resource The OTEL resource was built with `Resource::builder_empty()`, which runs no resource detectors, so attributes injected through the standard `OTEL_RESOURCE_ATTRIBUTES` env var were silently dropped. Deployments that inject `k8s.pod.uid`, `k8s.container.name` or `service.namespace` saw none of them reach their backend. Use `Resource::builder()`, which seeds from the SDK's env detector. Windmill's own attributes keep being applied on top, so per the OTel resource spec the env var is the secondary resource and `service.name`, `service.version`, `host.name` and `deployment.environment*` stay authoritative. The EE change lives in windmill-ee-private; this carries the ee-repo-ref bump and a regression test pinning both halves of the contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * test(otel): clear OTEL_HOST_NAME so the resource test is hermetic OTEL_HOST_NAME takes precedence over the hostname argument, so an ambient one failed the host.name assertion with a message pointing at the merge logic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * feat(otel): honor OTEL_SERVICE_NAME and OTEL_SERVICE_VERSION Deployments identify each pod from its own labels, e.g. through the Kubernetes downward API, so `service.name` and `service.version` must be settable per pod. Both were ignored: OTEL_SERVICE_NAME was read by the SDK and then overwritten, and because the two attributes are set in code they also outrank OTEL_RESOURCE_ATTRIBUTES, leaving no route to set them at all. The EE change lives in windmill-ee-private; this carries the ee-repo-ref bump and tests for the dedicated overrides. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * fix(otel): keep service.version pinned to the build version OTEL_SERVICE_VERSION is not an OTel env var, and service.version identifies the build that produced the telemetry, which a deployment cannot state more precisely than GIT_VERSION already does. A deployment that wants its own release version in telemetry can carry it under its own key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * test(otel): pin OTEL_SERVICE_NAME above service.name in OTEL_RESOURCE_ATTRIBUTES The spec ranks OTEL_SERVICE_NAME above a service.name carried in OTEL_RESOURCE_ATTRIBUTES; that ordering was only checked by hand. The three candidate values are distinct, so the assertions fail if either ranking breaks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * feat(otel): honor OTEL_SERVICE_VERSION The spec defines no OTEL_SERVICE_VERSION, but deployments set it expecting it to work because it sits next to OTEL_SERVICE_NAME, and setting service.version in code blocks the OTEL_RESOURCE_ATTRIBUTES route, so there is otherwise no way to set it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * test(otel): guard against unknown_service on a default deployment Resource::builder seeds SdkProvidedResourceDetector, which sets service.name to "unknown_service" when neither OTEL_SERVICE_NAME nor a service.name in OTEL_RESOURCE_ATTRIBUTES is present. Only our own attribute keeps that out of the exported resource, and no assertion covered the case where nothing is set at all — which is the default deployment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * test(otel): pin the empty-means-unset fallback for OTEL_SERVICE_VERSION The empty case asserted the fallback for service.name and host.name but not service.version, leaving one branch of the three-variable contract uncovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yUJaBDHPgjZPodP2u4aqR * chore: update ee-repo-ref to b964f0caaae57dc526c7ac9dc54d753372989f63 This commit updates the EE repository reference after PR #779 was merged in windmill-ee-private. Previous ee-repo-ref: 62efa909aabdba4cb31ffabe9aae0e4909ca1e07 New ee-repo-ref: b964f0caaae57dc526c7ac9dc54d753372989f63 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
2026-09-10 00:05:27 +00:00 · 2026-09-09 17:41:41 +02:00
parent 8aa8b7ee6c
commit 0a40eea37a
2 changed files with 119 additions and 1 deletions
@@ -1 +1 @@
-81edd1382d951265ab3e9b67fc7ca7967676fd56
+b964f0caaae57dc526c7ac9dc54d753372989f63
@@ -627,3 +627,121 @@ async fn test_root_job_span_relocated_to_inbound_trace() {
        expected_uuid_trace
    );
 }
+
+// ═══════════════════════════════════════════════════════════════════════
+// RESOURCE ATTRIBUTES (OTEL_RESOURCE_ATTRIBUTES)
+// ═══════════════════════════════════════════════════════════════════════
+
+fn resource_attrs() -> std::collections::HashMap<String, String> {
+    otlp_service_resource(
+        &windmill_common::utils::Mode::Worker,
+        "fallback-host",
+        "dev",
+    )
+    .iter()
+    .map(|(k, v)| (k.to_string(), v.to_string()))
+    .collect()
+}
+
+#[test]
+#[serial_test::serial]
+fn test_otlp_resource_merges_env_attributes_without_losing_windmill_identity() {
+    // These take precedence over the hostname argument and over OTEL_RESOURCE_ATTRIBUTES,
+    // so clear them or an ambient one fails the assertions below for an unrelated reason.
+    for var in [
+        "OTEL_HOST_NAME",
+        "OTEL_SERVICE_NAME",
+        "OTEL_SERVICE_VERSION",
+    ] {
+        std::env::remove_var(var);
+    }
+    std::env::set_var(
+        "OTEL_RESOURCE_ATTRIBUTES",
+        "k8s.pod.uid=abc-123,service.name=injected,host.name=injected",
+    );
+    let attrs = resource_attrs();
+    std::env::remove_var("OTEL_RESOURCE_ATTRIBUTES");
+
+    // Attributes the deployment injects reach the exporters.
+    assert_eq!(
+        attrs.get("k8s.pod.uid").map(String::as_str),
+        Some("abc-123")
+    );
+    // OTEL_RESOURCE_ATTRIBUTES is the secondary resource, so Windmill's own values still win.
+    assert_eq!(
+        attrs.get("service.name").map(String::as_str),
+        Some("windmill-worker")
+    );
+    assert_eq!(
+        attrs.get("host.name").map(String::as_str),
+        Some("fallback-host")
+    );
+}
+
+#[test]
+#[serial_test::serial]
+fn test_otlp_resource_dedicated_overrides_win() {
+    // A deployment sets these per pod, e.g. from Kubernetes downward-API labels. The
+    // competing service.name must lose: the spec ranks OTEL_SERVICE_NAME above it.
+    std::env::set_var("OTEL_RESOURCE_ATTRIBUTES", "service.name=should-lose");
+    std::env::set_var("OTEL_SERVICE_NAME", "windmill-workers");
+    std::env::set_var("OTEL_SERVICE_VERSION", "1.802.0");
+    std::env::set_var("OTEL_HOST_NAME", "pod-7");
+    let overridden = resource_attrs();
+
+    // An empty value means unset, which is what the downward API yields for a missing label.
+    for var in [
+        "OTEL_SERVICE_NAME",
+        "OTEL_SERVICE_VERSION",
+        "OTEL_HOST_NAME",
+    ] {
+        std::env::set_var(var, "");
+    }
+    let empty = resource_attrs();
+    for var in [
+        "OTEL_SERVICE_NAME",
+        "OTEL_SERVICE_VERSION",
+        "OTEL_HOST_NAME",
+        "OTEL_RESOURCE_ATTRIBUTES",
+    ] {
+        std::env::remove_var(var);
+    }
+    let unset = resource_attrs();
+
+    assert_eq!(
+        overridden.get("service.name").map(String::as_str),
+        Some("windmill-workers")
+    );
+    assert_eq!(
+        overridden.get("service.version").map(String::as_str),
+        Some("1.802.0")
+    );
+    assert_eq!(
+        overridden.get("host.name").map(String::as_str),
+        Some("pod-7")
+    );
+
+    assert_eq!(
+        empty.get("service.name").map(String::as_str),
+        Some("windmill-worker")
+    );
+    assert_eq!(
+        empty.get("host.name").map(String::as_str),
+        Some("fallback-host")
+    );
+    assert_eq!(
+        empty.get("service.version").map(String::as_str),
+        Some(windmill_common::utils::GIT_VERSION)
+    );
+
+    // With nothing set at all — the default deployment — SdkProvidedResourceDetector still
+    // contributes service.name = "unknown_service". Ours has to overwrite it.
+    assert_eq!(
+        unset.get("service.name").map(String::as_str),
+        Some("windmill-worker")
+    );
+    assert_eq!(
+        unset.get("service.version").map(String::as_str),
+        Some(windmill_common::utils::GIT_VERSION)
+    );
+}