Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions test-collector-ruby/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 8 additions & 3 deletions test-collector-ruby/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions test-collector-ruby/docs/opentelemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 36 additions & 2 deletions test-collector-ruby/lib/buildkite/test_collector/otel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
72 changes: 71 additions & 1 deletion test-collector-ruby/spec/test_collector/otel_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down