diff --git a/lib/sentry/opentelemetry/span_processor.ex b/lib/sentry/opentelemetry/span_processor.ex index af16ac02..ea42d86c 100644 --- a/lib/sentry/opentelemetry/span_processor.ex +++ b/lib/sentry/opentelemetry/span_processor.ex @@ -27,7 +27,11 @@ if Code.ensure_loaded?(OpenTelemetry) do SpanStorage.store_span(span_record) - if span_record.parent_span_id == nil do + # Check if this is a root span (no parent) or a transaction root (HTTP request span) + is_transaction_root = + span_record.parent_span_id == nil or is_http_request_span?(span_record) + + if is_transaction_root do child_span_records = SpanStorage.get_child_spans(span_record.span_id) transaction = build_transaction(span_record, child_span_records) @@ -60,6 +64,20 @@ if Code.ensure_loaded?(OpenTelemetry) do :ok end + # Helper function to detect if a span represents an HTTP request that should be treated as a transaction root + defp is_http_request_span?(%{attributes: attributes, name: name}) do + # Check if this is an HTTP request span (has HTTP method and is likely a server span) + has_http_method = Map.has_key?(attributes, to_string(HTTPAttributes.http_request_method())) + has_http_route = Map.has_key?(attributes, "http.route") + has_url_path = Map.has_key?(attributes, to_string(URLAttributes.url_path())) + + # Check if the name looks like an HTTP endpoint + name_looks_like_http = + String.contains?(name, ["/", "POST", "GET", "PUT", "DELETE", "PATCH"]) + + (has_http_method and (has_http_route or has_url_path)) or name_looks_like_http + end + defp build_transaction(root_span_record, child_span_records) do root_span = build_span(root_span_record) child_spans = Enum.map(child_span_records, &build_span(&1)) diff --git a/test/sentry/opentelemetry/span_processor_test.exs b/test/sentry/opentelemetry/span_processor_test.exs index 331acd08..ef8c3250 100644 --- a/test/sentry/opentelemetry/span_processor_test.exs +++ b/test/sentry/opentelemetry/span_processor_test.exs @@ -235,6 +235,74 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do assert transaction.transaction == "child_instrumented_function_standalone" end + @tag span_storage: true + test "treats HTTP request spans as transaction roots even with external parents" do + put_test_config(environment_name: "test", traces_sample_rate: 1.0) + + Sentry.Test.start_collecting_sentry_reports() + + # Simulate an HTTP request span with external parent (like from browser tracing) + require OpenTelemetry.Tracer, as: Tracer + require OpenTelemetry.SemConv.Incubating.HTTPAttributes, as: HTTPAttributes + require OpenTelemetry.SemConv.Incubating.URLAttributes, as: URLAttributes + + # Create a span with HTTP attributes and an external parent span ID + external_parent_span_id = "b943d7459127970c" + + # Start a span that simulates an HTTP request from an external trace + Tracer.with_span "POST /api/v1alpha", %{ + attributes: %{ + HTTPAttributes.http_request_method() => :POST, + URLAttributes.url_path() => "/api/v1alpha", + "http.route" => "/api/v1alpha", + "server.address" => "localhost", + "server.port" => 4000 + }, + parent: + {:span_context, :undefined, external_parent_span_id, :undefined, :undefined, :undefined, + :undefined, :undefined, :undefined, :undefined} + } do + # Simulate child spans (database queries, etc.) with proper DB attributes + Tracer.with_span "matrix_data.repo.query", %{ + attributes: %{ + "db.system" => :postgresql, + "db.statement" => "SELECT * FROM users" + } + } do + Process.sleep(10) + end + + Tracer.with_span "matrix_data.repo.query:agents", %{ + attributes: %{ + "db.system" => :postgresql, + "db.statement" => "INSERT INTO agents (...) VALUES (...)" + } + } do + Process.sleep(10) + end + end + + # Should capture the HTTP request span as a transaction root despite having an external parent + assert [%Sentry.Transaction{} = transaction] = Sentry.Test.pop_sentry_transactions() + + # Verify transaction properties + assert transaction.transaction == "POST /api/v1alpha" + assert transaction.transaction_info == %{source: :custom} + assert length(transaction.spans) == 2 + + # Verify child spans are properly included - they should have "db" operation + span_names = Enum.map(transaction.spans, & &1.op) |> Enum.sort() + expected_names = ["db", "db"] + assert span_names == expected_names + + # Verify all spans share the same trace ID + trace_id = transaction.contexts.trace.trace_id + + Enum.each(transaction.spans, fn span -> + assert span.trace_id == trace_id + end) + end + @tag span_storage: true test "concurrent traces maintain independent sampling decisions" do put_test_config(environment_name: "test", traces_sample_rate: 0.5)