diff --git a/test-collector-ruby/CHANGELOG.md b/test-collector-ruby/CHANGELOG.md index 70a98883..68aa65c9 100644 --- a/test-collector-ruby/CHANGELOG.md +++ b/test-collector-ruby/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Honor standard `OTEL_EXPORTER_OTLP_TRACES_HEADERS` and + `OTEL_EXPORTER_OTLP_HEADERS` in both OpenTelemetry modes. These headers take + precedence over collector-generated headers, allowing bktec's local OTLP + relay to authenticate exports without replacing the normal JSON upload token. + Empty header environment variables are treated as unset. + * Align `otel_enabled` and `otel_only` on the same execution spans and run resources. `otel_only` now differs by adding `buildkite.execution.via=otlp` while skipping legacy trace capture and JSON result uploads. diff --git a/test-collector-ruby/README.md b/test-collector-ruby/README.md index 68e5ba01..99a8276a 100644 --- a/test-collector-ruby/README.md +++ b/test-collector-ruby/README.md @@ -131,9 +131,14 @@ regardless of who owns the provider. In suite-owned mode, a supported Export needs Ruby 3.3 or newer, which is what the OpenTelemetry gems require. On older Rubies the option is accepted and does nothing. -Spans need `BUILDKITE_ANALYTICS_TOKEN` to be an agent OIDC token with the -`write_uploads` scope, from `buildkite-agent oidc request-token`. A suite API -token still uploads executions, but its spans are rejected. +The collector honors standard `OTEL_EXPORTER_OTLP_TRACES_HEADERS` (or the +generic `OTEL_EXPORTER_OTLP_HEADERS`) and gives them precedence over its own +headers, including `Authorization`. bktec's OTLP relay uses this to provide its +local credential without changing `BUILDKITE_ANALYTICS_TOKEN`, which remains +available for normal JSON uploads in `otel_enabled` mode. Without an OTLP +Authorization header, spans use `BUILDKITE_ANALYTICS_TOKEN`, which must be an +agent OIDC token with the `write_uploads` scope; a suite API token still uploads +executions, but its spans are rejected. Export failures never fail a test or block the normal Test Engine upload. See the [OpenTelemetry guide](docs/opentelemetry.md) for what you get and how it diff --git a/test-collector-ruby/docs/opentelemetry.md b/test-collector-ruby/docs/opentelemetry.md index edd926ba..96ccabac 100644 --- a/test-collector-ruby/docs/opentelemetry.md +++ b/test-collector-ruby/docs/opentelemetry.md @@ -230,10 +230,19 @@ instrumentation unchanged. A warning reports an `[]` selection that was ignored. ## What gets sent -Spans go to Buildkite over OTLP, using the same `BUILDKITE_ANALYTICS_TOKEN` as -the rest of the collector. Sending them needs an agent OIDC token with the -`write_uploads` scope; a suite API token uploads test results as normal but its -spans are rejected. +The collector merges standard `OTEL_EXPORTER_OTLP_TRACES_HEADERS` (or, when it +is absent, `OTEL_EXPORTER_OTLP_HEADERS`) over its own OTLP headers. Header names +are matched case-insensitively, so a standard `authorization` entry takes +precedence over the credential sourced from `BUILDKITE_ANALYTICS_TOKEN`. Empty +header environment variables are treated as unset. + +bktec's OTLP relay uses the trace-specific header variable to provide its local +credential. bktec forwards spans to Buildkite with its OIDC credential while +`BUILDKITE_ANALYTICS_TOKEN` remains available for normal JSON uploads in +`otel_enabled` mode. Without an OTLP Authorization header, spans go directly to +Buildkite using `BUILDKITE_ANALYTICS_TOKEN`, which must be an agent OIDC token +with the `write_uploads` scope; a suite API token uploads test results as normal +but its spans are rejected. OpenTelemetry's SDK owns batching, retries, and transport. `test.execution` spans have a reserved, faster-draining queue and exporter. Forwarded children diff --git a/test-collector-ruby/lib/buildkite/test_collector/otel.rb b/test-collector-ruby/lib/buildkite/test_collector/otel.rb index e3c0fab0..5be6bf5c 100644 --- a/test-collector-ruby/lib/buildkite/test_collector/otel.rb +++ b/test-collector-ruby/lib/buildkite/test_collector/otel.rb @@ -85,7 +85,13 @@ def configure!(endpoint: DEFAULT_ENDPOINT, api_token: nil, run_env: {}, instrume @api_token = api_token @run_key = run_env["key"] - headers = request_headers(run_env, api_token) + # Passing collector headers to the exporter bypasses its environment + # defaults, so merge the standard OTLP headers here instead. + environment_headers = otlp_headers_from_environment + @authorization_from_environment = environment_headers.keys.any? do |key| + key.casecmp?("Authorization") + end + headers = request_headers(run_env, api_token, environment_headers) # Run-level detail travels as the resource of the providers we create, # so every exported span carries it without repeating it per span. @@ -215,6 +221,7 @@ def shutdown @execution_child_forwarder = nil @exporters = nil @api_token = nil + @authorization_from_environment = nil @run_key = nil @tracer = nil end @@ -288,6 +295,10 @@ def refresh_authorization(api_token) return if api_token.nil? || api_token == @api_token @api_token = api_token + # Standard OTLP configuration remains authoritative across warm-worker + # reconfiguration, even when the collector receives a refreshed token. + return if @authorization_from_environment + value = authorization_header(api_token) refreshed = Array(@exporters).count do |exporter| headers = exporter.instance_variable_defined?(:@headers) && exporter.instance_variable_get(:@headers) @@ -511,12 +522,35 @@ def job_span_links [] end - def request_headers(run_env, api_token) + def request_headers(run_env, api_token, environment_headers = otlp_headers_from_environment) headers = { "Buildkite-Tests-Run-Key" => run_env["key"] } headers["Authorization"] = authorization_header(api_token) if api_token + environment_headers.each do |key, value| + headers.delete_if { |existing, _| existing.casecmp?(key) } + headers[key] = value + end headers end + def otlp_headers_from_environment + raw = ENV["OTEL_EXPORTER_OTLP_TRACES_HEADERS"] + raw = ENV["OTEL_EXPORTER_OTLP_HEADERS"] if raw.nil? || raw.empty? + return {} if raw.nil? || raw.empty? + + entries = raw.split(",") + raise ArgumentError, "invalid OTLP exporter headers" if entries.empty? + + entries.each_with_object({}) do |entry, headers| + key, value = entry.split("=", 2).map { |part| URI.decode_uri_component(part) } + key = key.to_s.strip + value = value.to_s.strip + raise ArgumentError, "invalid OTLP exporter headers" if key.empty? || value.empty? + + headers.delete_if { |existing, _| existing.casecmp?(key) } + headers[key] = value + end + end + def authorization_header(api_token) "Token token=\"#{api_token}\"" end diff --git a/test-collector-ruby/spec/test_collector/otel_spec.rb b/test-collector-ruby/spec/test_collector/otel_spec.rb index 337273b4..000ab7a3 100644 --- a/test-collector-ruby/spec/test_collector/otel_spec.rb +++ b/test-collector-ruby/spec/test_collector/otel_spec.rb @@ -528,6 +528,48 @@ def finish ) end + it "gives trace-specific OTLP headers precedence over generic and collector headers" do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_TRACES_HEADERS") + .and_return( + "authorization=Bearer%20relay-token,buildkite-tests-run-key=relay-run,x-extra=hello%20world" + ) + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_HEADERS") + .and_return("authorization=Bearer%20generic-token") + + headers = described_class.send(:request_headers, { "key" => "test-run-id" }, "suite-token") + + expect(headers).to eq( + "authorization" => "Bearer relay-token", + "buildkite-tests-run-key" => "relay-run", + "x-extra" => "hello world", + ) + end + + it "uses generic OTLP headers when trace-specific headers are empty" do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_TRACES_HEADERS").and_return("") + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_HEADERS") + .and_return("Authorization=Bearer%20generic-token") + + headers = described_class.send(:request_headers, { "key" => "test-run-id" }, "suite-token") + + expect(headers["Authorization"]).to eq("Bearer generic-token") + end + + it "uses collector headers when both standard OTLP header variables are empty" do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_TRACES_HEADERS").and_return("") + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_HEADERS").and_return("") + + headers = described_class.send(:request_headers, { "key" => "test-run-id" }, "suite-token") + + expect(headers).to eq( + "Buildkite-Tests-Run-Key" => "test-run-id", + "Authorization" => %(Token token="suite-token"), + ) + end + it "uses an AlwaysOn sampler, process-safe random IDs, and the run resource for execution roots" do processor = spy( "execution processor", @@ -734,7 +776,9 @@ def finish describe "token refresh" do def exporter_authorization_headers described_class.instance_variable_get(:@exporters).map do |exporter| - exporter.instance_variable_get(:@headers)["Authorization"] + exporter.instance_variable_get(:@headers).find do |key, _| + key.casecmp?("Authorization") + end&.last end end @@ -768,6 +812,32 @@ def exporter_authorization_headers OpenTelemetry.tracer_provider = original end + it "does not replace standard OTLP authorization when the collector token changes" do + original = OpenTelemetry.tracer_provider + suite_provider = OpenTelemetry::SDK::Trace::TracerProvider.new + OpenTelemetry.tracer_provider = suite_provider + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("OTEL_EXPORTER_OTLP_TRACES_HEADERS") + .and_return("authorization=Bearer%20relay-token") + + described_class.configure!( + endpoint: "https://example.invalid/v1/traces", + api_token: "before-refresh", + run_env: { "key" => "run-123" }, + ) + described_class.configure!( + endpoint: "https://example.invalid/v1/traces", + api_token: "after-refresh", + run_env: { "key" => "run-123" }, + ) + + expect(exporter_authorization_headers).to eq(["Bearer relay-token"] * 2) + ensure + described_class.shutdown + suite_provider&.shutdown + OpenTelemetry.tracer_provider = original + end + it "warns when reconfigured with a different run key, keeping the original run" do original = OpenTelemetry.tracer_provider suite_provider = OpenTelemetry::SDK::Trace::TracerProvider.new