diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index ee13bb2f..738f22a3 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -77,7 +77,7 @@ jobs: working-directory: test_integrations/phoenix_app run: | rm -f tmp/sentry_debug_events.log - SENTRY_E2E_TEST_MODE=true mix phx.server & + SENTRY_E2E_TEST_MODE=true SENTRY_ORG_ID=123 mix phx.server & echo $! > /tmp/phoenix.pid echo "Phoenix server started with PID $(cat /tmp/phoenix.pid)" diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index 81a59198..4dbdece2 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -444,6 +444,29 @@ defmodule Sentry.Config do ] ] ], + org_id: [ + type: {:custom, __MODULE__, :__validate_org_id__, []}, + default: nil, + type_doc: "`t:String.t/0` or `nil`", + doc: """ + An explicit organization ID for trace continuation validation. If not set, the SDK + will extract it from the DSN host (e.g., `o1234` from `o1234.ingest.sentry.io` gives `"1234"`). + This is useful for self-hosted Sentry or Relay setups where the org ID cannot be extracted + from the DSN. *Available since 12.1.0*. + """ + ], + strict_trace_continuation: [ + type: :boolean, + default: false, + doc: """ + When `true`, both the SDK's org ID and the incoming baggage `sentry-org_id` must be present + and match for a trace to be continued. Traces with a missing org ID on either side are rejected + and a new trace is started. When `false` (the default), only a mismatch between two present + org IDs will cause a new trace to be started. See the + [SDK spec](https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation) + for the full decision matrix. *Available since 12.1.0*. + """ + ], telemetry_processor_categories: [ type: {:list, {:in, [:error, :check_in, :transaction, :log]}}, default: [], @@ -972,6 +995,29 @@ defmodule Sentry.Config do @spec transport_capacity() :: pos_integer() def transport_capacity, do: fetch!(:transport_capacity) + @spec org_id() :: String.t() | nil + def org_id, do: get(:org_id) + + @spec strict_trace_continuation?() :: boolean() + def strict_trace_continuation?, do: fetch!(:strict_trace_continuation) + + @doc """ + Returns the effective org ID, preferring the explicit `:org_id` config over the DSN-derived value. + """ + @spec effective_org_id() :: String.t() | nil + def effective_org_id do + case org_id() do + nil -> + case dsn() do + %Sentry.DSN{org_id: org_id} -> org_id + _ -> nil + end + + explicit -> + explicit + end + end + @spec telemetry_processor_categories() :: [atom()] def telemetry_processor_categories, do: fetch!(:telemetry_processor_categories) @@ -1279,4 +1325,18 @@ defmodule Sentry.Config do def __validate_namespace__(other) do {:error, "expected :namespace to be a {module, function} tuple, got: #{inspect(other)}"} end + + def __validate_org_id__(nil), do: {:ok, nil} + + def __validate_org_id__(value) when is_binary(value) and value != "" do + {:ok, value} + end + + def __validate_org_id__("") do + {:error, "expected :org_id to be a non-empty string or nil, got empty string"} + end + + def __validate_org_id__(other) do + {:error, "expected :org_id to be a non-empty string or nil, got: #{inspect(other)}"} + end end diff --git a/lib/sentry/dsn.ex b/lib/sentry/dsn.ex index 93704244..3d6822ae 100644 --- a/lib/sentry/dsn.ex +++ b/lib/sentry/dsn.ex @@ -5,14 +5,16 @@ defmodule Sentry.DSN do original_dsn: String.t(), endpoint_uri: String.t(), public_key: String.t(), - secret_key: String.t() | nil + secret_key: String.t() | nil, + org_id: String.t() | nil } defstruct [ :original_dsn, :endpoint_uri, :public_key, - :secret_key + :secret_key, + :org_id ] # {PROTOCOL}://{PUBLIC_KEY}:{SECRET_KEY}@{HOST}{PATH}/{PROJECT_ID} @@ -65,7 +67,8 @@ defmodule Sentry.DSN do endpoint_uri: URI.to_string(endpoint_uri), public_key: public_key, secret_key: secret_key, - original_dsn: dsn + original_dsn: dsn, + org_id: extract_org_id(uri.host) } {:ok, parsed_dsn} @@ -80,6 +83,16 @@ defmodule Sentry.DSN do ## Helpers + # Extract org ID from host (e.g., "o123.ingest.sentry.io" -> "123") + defp extract_org_id(host) when is_binary(host) do + case Regex.run(~r/^o(\d+)\./, host) do + [_, org_id] -> org_id + _ -> nil + end + end + + defp extract_org_id(_host), do: nil + defp pop_project_id(uri_path) do path = String.split(uri_path, "/") {project_id, path} = List.pop_at(path, -1) diff --git a/lib/sentry/opentelemetry/propagator.ex b/lib/sentry/opentelemetry/propagator.ex index cb3d73fe..6830fd1c 100644 --- a/lib/sentry/opentelemetry/propagator.ex +++ b/lib/sentry/opentelemetry/propagator.ex @@ -10,6 +10,7 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do import Bitwise require Record + require Logger require OpenTelemetry.Tracer, as: Tracer @behaviour :otel_propagator_text_map @@ -35,6 +36,7 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do carrier = setter.(@sentry_trace_key, sentry_trace_header, carrier) baggage_value = :otel_ctx.get_value(ctx, @sentry_baggage_ctx_key, :not_found) + baggage_value = ensure_org_id_in_baggage(baggage_value) if is_binary(baggage_value) and baggage_value != :not_found do setter.(@sentry_baggage_key, baggage_value, carrier) @@ -56,19 +58,39 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do header when is_binary(header) -> case decode_sentry_trace(header) do {:ok, {trace_hex, span_hex, sampled}} -> - ctx = - ctx - |> :otel_ctx.set_value(@sentry_trace_ctx_key, {trace_hex, span_hex, sampled}) - |> maybe_set_baggage(getter.(@sentry_baggage_key, carrier)) + raw_baggage = getter.(@sentry_baggage_key, carrier) + + if should_continue_trace?(raw_baggage) do + ctx = + ctx + |> :otel_ctx.set_value(@sentry_trace_ctx_key, {trace_hex, span_hex, sampled}) + |> maybe_set_baggage(raw_baggage) + + trace_id = hex_to_int(trace_hex) + span_id = hex_to_int(span_hex) - trace_id = hex_to_int(trace_hex) - span_id = hex_to_int(span_hex) + # Create a remote, sampled parent span in the OTEL context. + # We will set to "always sample" because Sentry will decide real sampling + remote_span_ctx = :otel_tracer.from_remote_span(trace_id, span_id, 1) - # Create a remote, sampled parent span in the OTEL context. - # We will set to "always sample" because Sentry will decide real sampling - remote_span_ctx = :otel_tracer.from_remote_span(trace_id, span_id, 1) + Tracer.set_current_span(ctx, remote_span_ctx) + else + sdk_org_id = Sentry.Config.effective_org_id() + baggage_org_id = extract_baggage_org_id(raw_baggage) - Tracer.set_current_span(ctx, remote_span_ctx) + reason = + if sdk_org_id != nil and baggage_org_id != nil do + "org ID mismatch" + else + "org ID missing (strict mode)" + end + + Logger.warning( + "[Sentry] Not continuing trace: #{reason} (sdk: #{inspect(sdk_org_id)}, incoming: #{inspect(baggage_org_id)})" + ) + + ctx + end {:error, _reason} -> ctx @@ -131,5 +153,82 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do missing = total_bytes - byte_size(bin) if missing > 0, do: :binary.copy(<<0>>, missing) <> bin, else: bin end + + # Ensure sentry-org_id is present in the baggage string + defp ensure_org_id_in_baggage(baggage) when is_binary(baggage) do + org_id = Sentry.Config.effective_org_id() + + if org_id != nil and extract_baggage_org_id(baggage) == nil do + # Strip any existing sentry-org_id entries with empty values before appending + # to avoid producing duplicate keys (e.g. "sentry-org_id=,sentry-org_id=99"). + stripped = + baggage + |> String.split(",") + |> Enum.reject(fn entry -> + case String.split(String.trim(entry), "=", parts: 2) do + ["sentry-org_id", value] -> String.trim(value) == "" + _ -> false + end + end) + |> Enum.join(",") + + if stripped == "" do + "sentry-org_id=" <> org_id + else + stripped <> ",sentry-org_id=" <> org_id + end + else + baggage + end + end + + defp ensure_org_id_in_baggage(_baggage) do + case Sentry.Config.effective_org_id() do + nil -> :not_found + org_id -> "sentry-org_id=" <> org_id + end + end + + # Extract sentry-org_id from a baggage header string + defp extract_baggage_org_id(baggage) when is_binary(baggage) do + baggage + |> String.split(",") + |> Enum.find_value(fn entry -> + case String.split(String.trim(entry), "=", parts: 2) do + ["sentry-org_id", value] -> + trimmed = String.trim(value) + if trimmed == "", do: nil, else: trimmed + + _ -> + nil + end + end) + end + + defp extract_baggage_org_id(_), do: nil + + # Determine whether to continue an incoming trace based on org_id validation + @doc false + def should_continue_trace?(raw_baggage) do + sdk_org_id = Sentry.Config.effective_org_id() + baggage_org_id = extract_baggage_org_id(raw_baggage) + strict = Sentry.Config.strict_trace_continuation?() + + cond do + # Mismatched org IDs always reject + sdk_org_id != nil and baggage_org_id != nil and sdk_org_id != baggage_org_id -> + false + + # In strict mode, both must be present and match (unless both are missing) + strict and sdk_org_id == nil and baggage_org_id == nil -> + true + + strict -> + sdk_org_id != nil and sdk_org_id == baggage_org_id + + true -> + true + end + end end end diff --git a/test/sentry/config_test.exs b/test/sentry/config_test.exs index d3a2bc4f..ca806c22 100644 --- a/test/sentry/config_test.exs +++ b/test/sentry/config_test.exs @@ -1,6 +1,8 @@ defmodule Sentry.ConfigTest do use Sentry.Case, async: false + import Sentry.TestHelpers + alias Sentry.Config describe "validate!/0" do @@ -429,4 +431,67 @@ defmodule Sentry.ConfigTest do assert config[:before_send_metric] == nil end end + + describe ":org_id" do + test "defaults to nil" do + assert Config.validate!([])[:org_id] == nil + end + + test "accepts a non-empty string" do + assert Config.validate!(org_id: "1234567")[:org_id] == "1234567" + end + + test "accepts nil explicitly" do + assert Config.validate!(org_id: nil)[:org_id] == nil + end + + test "rejects an empty string" do + assert_raise ArgumentError, ~r/expected :org_id to be a non-empty string or nil/, fn -> + Config.validate!(org_id: "") + end + end + + test "rejects a non-string value" do + assert_raise ArgumentError, ~r/invalid value for :org_id option/, fn -> + Config.validate!(org_id: 1234) + end + end + end + + describe "effective_org_id/0" do + test "returns nil when no org_id is configured and DSN has no org ID" do + put_test_config(dsn: "https://public:secret@app.getsentry.com/1", org_id: nil) + assert Config.effective_org_id() == nil + end + + test "returns explicit org_id when configured" do + put_test_config(org_id: "9876543") + assert Config.effective_org_id() == "9876543" + end + + test "falls back to org ID extracted from DSN host" do + put_test_config(dsn: "https://public@o1234567.ingest.sentry.io/123", org_id: nil) + assert Config.effective_org_id() == "1234567" + end + + test "explicit org_id takes precedence over DSN-derived org ID" do + put_test_config(dsn: "https://public@o1234567.ingest.sentry.io/123", org_id: "9999999") + assert Config.effective_org_id() == "9999999" + end + end + + describe ":strict_trace_continuation" do + test "defaults to false" do + assert Config.validate!([])[:strict_trace_continuation] == false + end + + test "accepts true" do + assert Config.validate!(strict_trace_continuation: true)[:strict_trace_continuation] == true + end + + test "accepts false" do + assert Config.validate!(strict_trace_continuation: false)[:strict_trace_continuation] == + false + end + end end diff --git a/test/sentry/opentelemetry/propagator_test.exs b/test/sentry/opentelemetry/propagator_test.exs index 3dcfec72..0d0e382d 100644 --- a/test/sentry/opentelemetry/propagator_test.exs +++ b/test/sentry/opentelemetry/propagator_test.exs @@ -179,6 +179,8 @@ defmodule Sentry.OpenTelemetry.PropagatorTest do describe "baggage propagation" do test "injects baggage from context" do + put_test_config(dsn: "http://public:secret@localhost:9000/1") + trace_id = 0x1234567890ABCDEF1234567890ABCDEF span_id = 0x1234567890ABCDEF trace_flags = 1 @@ -210,6 +212,8 @@ defmodule Sentry.OpenTelemetry.PropagatorTest do end test "does not inject baggage when not in context" do + put_test_config(dsn: "http://public:secret@localhost:9000/1") + trace_id = 0x1234567890ABCDEF1234567890ABCDEF span_id = 0x1234567890ABCDEF trace_flags = 1 @@ -269,5 +273,146 @@ defmodule Sentry.OpenTelemetry.PropagatorTest do end end end + + describe "strict trace continuation integration" do + test "org IDs match: trace is continued end-to-end" do + put_test_config( + dsn: "https://key@o99.ingest.sentry.io/123", + strict_trace_continuation: false + ) + + sentry_trace = "1234567890abcdef1234567890abcdef-1234567890abcdef-1" + baggage = "sentry-org_id=99,sentry-public_key=key" + + getter = fn + "sentry-trace", _ -> sentry_trace + "baggage", _ -> baggage + _, _ -> :undefined + end + + ctx = Propagator.extract(:otel_ctx.new(), %{}, nil, getter, []) + + assert Tracer.current_span_ctx(ctx) != :undefined + end + + test "org ID mismatch: trace is NOT continued and a fresh context is returned" do + put_test_config( + dsn: "https://key@o99.ingest.sentry.io/123", + strict_trace_continuation: false + ) + + sentry_trace = "1234567890abcdef1234567890abcdef-1234567890abcdef-1" + baggage = "sentry-org_id=42,sentry-public_key=key" + + getter = fn + "sentry-trace", _ -> sentry_trace + "baggage", _ -> baggage + _, _ -> :undefined + end + + ctx = Propagator.extract(:otel_ctx.new(), %{}, nil, getter, []) + + assert Tracer.current_span_ctx(ctx) == :undefined + end + + test "strict=true, baggage missing org ID: trace is NOT continued" do + put_test_config( + dsn: "https://key@o99.ingest.sentry.io/123", + strict_trace_continuation: true + ) + + sentry_trace = "1234567890abcdef1234567890abcdef-1234567890abcdef-1" + baggage = "sentry-public_key=key" + + getter = fn + "sentry-trace", _ -> sentry_trace + "baggage", _ -> baggage + _, _ -> :undefined + end + + ctx = Propagator.extract(:otel_ctx.new(), %{}, nil, getter, []) + + assert Tracer.current_span_ctx(ctx) == :undefined + end + + test "inject adds sentry-org_id to outgoing baggage when SDK org is configured" do + put_test_config(dsn: "https://key@o99.ingest.sentry.io/123") + + Tracer.with_span "test_span" do + baggage_value = "sentry-trace_id=abc,sentry-public_key=key" + + ctx = + :otel_ctx.get_current() + |> :otel_ctx.set_value(:"sentry-baggage", baggage_value) + + setter = fn key, value, carrier -> Map.put(carrier, key, value) end + carrier = Propagator.inject(ctx, %{}, setter, []) + + assert String.contains?(Map.get(carrier, "baggage", ""), "sentry-org_id=99") + end + end + + test "inject does not produce leading comma when baggage is empty string" do + put_test_config(dsn: "https://key@o99.ingest.sentry.io/123") + + Tracer.with_span "test_span" do + ctx = + :otel_ctx.get_current() + |> :otel_ctx.set_value(:"sentry-baggage", "") + + setter = fn key, value, carrier -> Map.put(carrier, key, value) end + carrier = Propagator.inject(ctx, %{}, setter, []) + + injected_baggage = Map.get(carrier, "baggage", "") + + refute String.starts_with?(injected_baggage, ",") + assert injected_baggage == "sentry-org_id=99" + end + end + + test "inject does not append sentry-org_id when baggage already has an empty sentry-org_id entry" do + put_test_config(dsn: "https://key@o99.ingest.sentry.io/123") + + Tracer.with_span "test_span" do + # A downstream service may forward baggage with sentry-org_id= (empty value). + # We must not append a second sentry-org_id entry; the empty one should be + # replaced (or at minimum, no duplicate should be produced). + baggage_value = "sentry-trace_id=abc,sentry-org_id=,sentry-public_key=key" + + ctx = + :otel_ctx.get_current() + |> :otel_ctx.set_value(:"sentry-baggage", baggage_value) + + setter = fn key, value, carrier -> Map.put(carrier, key, value) end + carrier = Propagator.inject(ctx, %{}, setter, []) + + injected_baggage = Map.get(carrier, "baggage", "") + + # Must not produce two sentry-org_id entries + assert length(String.split(injected_baggage, "sentry-org_id=")) == 2, + "expected exactly one sentry-org_id entry, got: #{inspect(injected_baggage)}" + end + end + + test "inject does not duplicate sentry-org_id when already present in baggage" do + put_test_config(dsn: "https://key@o99.ingest.sentry.io/123") + + Tracer.with_span "test_span" do + baggage_value = "sentry-trace_id=abc,sentry-org_id=99" + + ctx = + :otel_ctx.get_current() + |> :otel_ctx.set_value(:"sentry-baggage", baggage_value) + + setter = fn key, value, carrier -> Map.put(carrier, key, value) end + carrier = Propagator.inject(ctx, %{}, setter, []) + + injected_baggage = Map.get(carrier, "baggage", "") + + assert String.contains?(injected_baggage, "sentry-org_id=99") + assert length(String.split(injected_baggage, "sentry-org_id=")) == 2 + end + end + end end end diff --git a/test/sentry/strict_trace_continuation_test.exs b/test/sentry/strict_trace_continuation_test.exs new file mode 100644 index 00000000..31c4d93e --- /dev/null +++ b/test/sentry/strict_trace_continuation_test.exs @@ -0,0 +1,167 @@ +defmodule Sentry.StrictTraceContinuationTest do + use Sentry.Case, async: false + + import Sentry.TestHelpers + + describe "DSN org_id extraction" do + test "extracts org_id from standard DSN host" do + {:ok, dsn} = Sentry.DSN.parse("https://key@o1234.ingest.sentry.io/123") + assert dsn.org_id == "1234" + end + + test "extracts org_id from DSN with US region" do + {:ok, dsn} = Sentry.DSN.parse("https://key@o42.ingest.us.sentry.io/123") + assert dsn.org_id == "42" + end + + test "returns nil for DSN without org_id" do + {:ok, dsn} = Sentry.DSN.parse("https://key@sentry.io/123") + assert dsn.org_id == nil + end + + test "returns nil for self-hosted DSN" do + {:ok, dsn} = Sentry.DSN.parse("https://key@my-sentry.example.com/123") + assert dsn.org_id == nil + end + + test "returns nil for localhost DSN" do + {:ok, dsn} = Sentry.DSN.parse("http://key@localhost:9000/123") + assert dsn.org_id == nil + end + end + + if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do + describe "should_continue_trace?/1" do + # Decision matrix tests + # | Baggage org | SDK org | strict=false | strict=true | + # |-------------|---------|-------------|-------------| + # | 1 | 1 | Continue | Continue | + # | None | 1 | Continue | New trace | + # | 1 | None | Continue | New trace | + # | None | None | Continue | Continue | + # | 1 | 2 | New trace | New trace | + + test "strict=false, matching orgs - continues trace" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + strict_trace_continuation: false + ) + + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "strict=false, baggage missing org - continues trace" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + strict_trace_continuation: false + ) + + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?("sentry-trace_id=abc") + end + + test "strict=false, SDK missing org - continues trace" do + put_test_config(dsn: "https://key@sentry.io/123", strict_trace_continuation: false) + + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "strict=false, both missing org - continues trace" do + put_test_config(dsn: "https://key@sentry.io/123", strict_trace_continuation: false) + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?("sentry-trace_id=abc") + end + + test "strict=false, mismatched orgs - starts new trace" do + put_test_config( + dsn: "https://key@o2.ingest.sentry.io/123", + strict_trace_continuation: false + ) + + refute Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "strict=true, matching orgs - continues trace" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + strict_trace_continuation: true + ) + + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "strict=true, baggage missing org - starts new trace" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + strict_trace_continuation: true + ) + + refute Sentry.OpenTelemetry.Propagator.should_continue_trace?("sentry-trace_id=abc") + end + + test "strict=true, SDK missing org - starts new trace" do + put_test_config(dsn: "https://key@sentry.io/123", strict_trace_continuation: true) + + refute Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "strict=true, both missing org - continues trace" do + put_test_config(dsn: "https://key@sentry.io/123", strict_trace_continuation: true) + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?("sentry-trace_id=abc") + end + + test "strict=true, mismatched orgs - starts new trace" do + put_test_config( + dsn: "https://key@o2.ingest.sentry.io/123", + strict_trace_continuation: true + ) + + refute Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "explicit org_id overrides DSN for validation" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + org_id: "2", + strict_trace_continuation: false + ) + + # SDK org is "2" (explicit), baggage org is "1" -> mismatch + refute Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-trace_id=abc,sentry-org_id=1" + ) + end + + test "handles nil baggage" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + strict_trace_continuation: false + ) + + assert Sentry.OpenTelemetry.Propagator.should_continue_trace?(nil) + end + + test "handles empty baggage org_id value" do + put_test_config( + dsn: "https://key@o1.ingest.sentry.io/123", + strict_trace_continuation: true + ) + + # Empty org_id in baggage should be treated as missing + refute Sentry.OpenTelemetry.Propagator.should_continue_trace?( + "sentry-org_id=,sentry-trace_id=abc" + ) + end + end + end +end diff --git a/test_integrations/phoenix_app/config/runtime.exs b/test_integrations/phoenix_app/config/runtime.exs index 24cad9a7..670b78b5 100644 --- a/test_integrations/phoenix_app/config/runtime.exs +++ b/test_integrations/phoenix_app/config/runtime.exs @@ -23,9 +23,25 @@ end # For e2e tracing tests, use the TestClient to log events to a file # This must be in runtime.exs because the env var is set at runtime, not compile time if System.get_env("SENTRY_E2E_TEST_MODE") == "true" do - config :sentry, - dsn: "https://public@sentry.example.com/1", - client: PhoenixApp.TestClient + sentry_e2e_opts = + [ + dsn: "https://public@sentry.example.com/1", + client: PhoenixApp.TestClient + ] + |> then(fn opts -> + case System.get_env("SENTRY_ORG_ID") do + nil -> opts + org_id -> Keyword.put(opts, :org_id, org_id) + end + end) + |> then(fn opts -> + case System.get_env("SENTRY_STRICT_TRACE") do + "true" -> Keyword.put(opts, :strict_trace_continuation, true) + _ -> opts + end + end) + + config :sentry, sentry_e2e_opts else # Allow runtime configuration of Sentry DSN and environment if dsn = System.get_env("SENTRY_DSN") do diff --git a/test_integrations/phoenix_app/lib/phoenix_app_web/controllers/test_config_controller.ex b/test_integrations/phoenix_app/lib/phoenix_app_web/controllers/test_config_controller.ex new file mode 100644 index 00000000..ae6c2de3 --- /dev/null +++ b/test_integrations/phoenix_app/lib/phoenix_app_web/controllers/test_config_controller.ex @@ -0,0 +1,21 @@ +defmodule PhoenixAppWeb.TestConfigController do + @moduledoc """ + Allows E2E tests to toggle Sentry configuration at runtime. + Only intended for use when SENTRY_E2E_TEST_MODE=true. + """ + + use PhoenixAppWeb, :controller + + @allowed_keys ~w(strict_trace_continuation)a + + def update(conn, params) do + Enum.each(@allowed_keys, fn key -> + case Map.fetch(params, to_string(key)) do + {:ok, value} when is_boolean(value) -> Sentry.put_config(key, value) + _ -> :ok + end + end) + + json(conn, %{ok: true}) + end +end diff --git a/test_integrations/phoenix_app/lib/phoenix_app_web/router.ex b/test_integrations/phoenix_app/lib/phoenix_app_web/router.ex index a130ed66..d917e6bb 100644 --- a/test_integrations/phoenix_app/lib/phoenix_app_web/router.ex +++ b/test_integrations/phoenix_app/lib/phoenix_app_web/router.ex @@ -56,6 +56,7 @@ defmodule PhoenixAppWeb.Router do get "/health", PageController, :health get "/api/data", PageController, :api_data post "/api/oban-job", PageController, :api_oban_job + put "/sentry-test-config", TestConfigController, :update end # Other scopes may use custom stacks. diff --git a/test_integrations/tracing/playwright.config.ts b/test_integrations/tracing/playwright.config.ts index 622ffb09..bb1f01a7 100644 --- a/test_integrations/tracing/playwright.config.ts +++ b/test_integrations/tracing/playwright.config.ts @@ -1,9 +1,21 @@ import { defineConfig, devices } from "@playwright/test"; -const PHOENIX_URL = - process.env.SENTRY_E2E_PHOENIX_APP_URL || "http://localhost:4000"; -const SVELTE_URL = - process.env.SENTRY_E2E_SVELTE_APP_URL || "http://localhost:4001"; +// Apply local defaults before any validation. These match the ports used by the +// webServer block below, so `npx playwright test` works without any setup. +// In CI or custom environments, set the env vars explicitly to override these. +process.env.SENTRY_E2E_PHOENIX_APP_URL ??= "http://localhost:4000"; +process.env.SENTRY_E2E_SVELTE_APP_URL ??= "http://localhost:4001"; + +function requireEnv(name: string): string { + const value = process.env[name]; + if (!value) { + throw new Error(`Required environment variable ${name} is not set.`); + } + return value; +} + +const PHOENIX_URL = requireEnv("SENTRY_E2E_PHOENIX_APP_URL"); +const SVELTE_URL = requireEnv("SENTRY_E2E_SVELTE_APP_URL"); // When servers are started externally (e.g., in CI workflow steps), skip webServer config const serversRunningExternally = process.env.SENTRY_E2E_SERVERS_RUNNING === "true"; @@ -39,7 +51,7 @@ export default defineConfig({ webServer: [ { command: - 'cd ../phoenix_app && rm -f tmp/sentry_debug_events.log && SENTRY_E2E_TEST_MODE=true mix phx.server', + 'cd ../phoenix_app && rm -f tmp/sentry_debug_events.log && SENTRY_E2E_TEST_MODE=true SENTRY_ORG_ID=123 mix phx.server', url: `${PHOENIX_URL}/health`, reuseExistingServer: true }, diff --git a/test_integrations/tracing/tests/tracing.spec.ts b/test_integrations/tracing/tests/tracing.spec.ts index cfbcaecf..03519437 100644 --- a/test_integrations/tracing/tests/tracing.spec.ts +++ b/test_integrations/tracing/tests/tracing.spec.ts @@ -9,6 +9,11 @@ import { type Span, } from "./helpers"; +const PHOENIX_URL = process.env.SENTRY_E2E_PHOENIX_APP_URL; +if (!PHOENIX_URL) { + throw new Error("Required environment variable SENTRY_E2E_PHOENIX_APP_URL is not set."); +} + test.describe("Tracing", () => { test.beforeEach(() => { clearLoggedEvents(); @@ -155,15 +160,15 @@ test.describe("Tracing", () => { await expect(page.locator("h1")).toContainText("Svelte Mini App"); - await page.evaluate(async () => { - const response = await fetch("http://localhost:4000/api/data", { + await page.evaluate(async (phoenixUrl) => { + const response = await fetch(`${phoenixUrl}/api/data`, { method: "GET", headers: { "Content-Type": "application/json", }, }); return response.json(); - }); + }, PHOENIX_URL); await page.waitForTimeout(2000); @@ -225,8 +230,6 @@ test.describe("Tracing", () => { }); test.describe("LiveView tracing", () => { - const PHOENIX_URL = process.env.SENTRY_E2E_PHOENIX_APP_URL || "http://localhost:4000"; - test("generates transaction for LiveView page mount with valid trace context", async ({ page }) => { await page.goto(`${PHOENIX_URL}/tracing-test`); @@ -555,8 +558,6 @@ test.describe("Tracing", () => { }); test.describe("Oban job tracing", () => { - const PHOENIX_URL = process.env.SENTRY_E2E_PHOENIX_APP_URL || "http://localhost:4000"; - test("LiveView-scheduled Oban job generates transaction with valid trace context", async ({ page }) => { await page.goto(`${PHOENIX_URL}/test-worker`); @@ -728,4 +729,231 @@ test.describe("Tracing", () => { }); }); }); + + test.describe("Strict trace continuation", () => { + // Must match SENTRY_ORG_ID set in playwright.config.ts webServer command + const SDK_ORG_ID = "123"; + + test.beforeEach(() => { + clearLoggedEvents(); + }); + + // Restore strict_trace_continuation to false after each test so any test + // that enables strict mode doesn't bleed into the next one. + test.afterEach(async ({ page }) => { + await page.evaluate(async (phoenixUrl) => { + await fetch(`${phoenixUrl}/sentry-test-config`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ strict_trace_continuation: false }), + }); + }, PHOENIX_URL); + }); + + test("continues incoming trace when baggage org_id matches SDK's org_id", async ({ + page, + }) => { + const incomingTraceId = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6"; + const incomingSpanId = "1234567890abcdef"; + + await page.goto(`${PHOENIX_URL}/health`); + + await page.evaluate( + async ({ traceId, spanId, orgId, phoenixUrl }) => { + await fetch(`${phoenixUrl}/api/data`, { + headers: { + "sentry-trace": `${traceId}-${spanId}-1`, + baggage: `sentry-org_id=${orgId},sentry-trace_id=${traceId},sentry-public_key=public`, + }, + }); + }, + { + traceId: incomingTraceId, + spanId: incomingSpanId, + orgId: SDK_ORG_ID, + phoenixUrl: PHOENIX_URL, + } + ); + + const logged = await waitForEvents( + (events) => + events.events.some( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data")) + ), + { timeout: 10000 } + ); + + const transactions = logged.events.filter( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data") || + (e.contexts?.trace?.data as any)?.["http.route"] === "/api/data") + ); + expect(transactions.length).toBeGreaterThan(0); + + // Matching org_id → trace is continued; trace_id preserved from sentry-trace header + expect(transactions[transactions.length - 1].contexts?.trace?.trace_id).toBe( + incomingTraceId + ); + }); + + test("starts new trace when baggage org_id does not match SDK's org_id", async ({ + page, + }) => { + const incomingTraceId = "b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a1"; + const incomingSpanId = "abcdef1234567890"; + const wrongOrgId = "456"; + + await page.goto(`${PHOENIX_URL}/health`); + + await page.evaluate( + async ({ traceId, spanId, orgId, phoenixUrl }) => { + await fetch(`${phoenixUrl}/api/data`, { + headers: { + "sentry-trace": `${traceId}-${spanId}-1`, + baggage: `sentry-org_id=${orgId},sentry-trace_id=${traceId},sentry-public_key=public`, + }, + }); + }, + { + traceId: incomingTraceId, + spanId: incomingSpanId, + orgId: wrongOrgId, + phoenixUrl: PHOENIX_URL, + } + ); + + const logged = await waitForEvents( + (events) => + events.events.some( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data")) + ), + { timeout: 10000 } + ); + + const transactions = logged.events.filter( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data") || + (e.contexts?.trace?.data as any)?.["http.route"] === "/api/data") + ); + expect(transactions.length).toBeGreaterThan(0); + + // Mismatched org_id → new trace started regardless of strict setting + expect(transactions[transactions.length - 1].contexts?.trace?.trace_id).not.toBe( + incomingTraceId + ); + }); + + test("continues incoming trace when baggage carries no org_id (strict=false)", async ({ + page, + }) => { + const incomingTraceId = "c3d4e5f6a7b8c9d0e1f2a3b4c5d6a1b2"; + const incomingSpanId = "fedcba9876543210"; + + await page.goto(`${PHOENIX_URL}/health`); + + await page.evaluate( + async ({ traceId, spanId, phoenixUrl }) => { + await fetch(`${phoenixUrl}/api/data`, { + headers: { + "sentry-trace": `${traceId}-${spanId}-1`, + // Baggage has no sentry-org_id + baggage: `sentry-trace_id=${traceId},sentry-public_key=public`, + }, + }); + }, + { traceId: incomingTraceId, spanId: incomingSpanId, phoenixUrl: PHOENIX_URL } + ); + + const logged = await waitForEvents( + (events) => + events.events.some( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data")) + ), + { timeout: 10000 } + ); + + const transactions = logged.events.filter( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data") || + (e.contexts?.trace?.data as any)?.["http.route"] === "/api/data") + ); + expect(transactions.length).toBeGreaterThan(0); + + // No org_id in baggage with strict=false → trace is continued + expect(transactions[transactions.length - 1].contexts?.trace?.trace_id).toBe( + incomingTraceId + ); + }); + + test("starts new trace when baggage carries no org_id and strict=true", async ({ + page, + }) => { + const incomingTraceId = "d4e5f6a7b8c9d0e1f2a3b4c5d6a1b2c3"; + const incomingSpanId = "0123456789abcdef"; + + // Enable strict mode for this test + await page.evaluate(async (phoenixUrl) => { + await fetch(`${phoenixUrl}/sentry-test-config`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ strict_trace_continuation: true }), + }); + }, PHOENIX_URL); + + clearLoggedEvents(); + + await page.evaluate( + async ({ traceId, spanId, phoenixUrl }) => { + await fetch(`${phoenixUrl}/api/data`, { + headers: { + "sentry-trace": `${traceId}-${spanId}-1`, + // Baggage has no sentry-org_id + baggage: `sentry-trace_id=${traceId},sentry-public_key=public`, + }, + }); + }, + { traceId: incomingTraceId, spanId: incomingSpanId, phoenixUrl: PHOENIX_URL } + ); + + const logged = await waitForEvents( + (events) => + events.events.some( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data")) + ), + { timeout: 10000 } + ); + + const transactions = logged.events.filter( + (e) => + e.type === "transaction" && + (e.transaction?.includes("/api/data") || + e.transaction?.includes("fetch_data") || + (e.contexts?.trace?.data as any)?.["http.route"] === "/api/data") + ); + expect(transactions.length).toBeGreaterThan(0); + + // No org_id in baggage with strict=true → new trace started + expect(transactions[transactions.length - 1].contexts?.trace?.trace_id).not.toBe( + incomingTraceId + ); + }); + }); });