Make root span the transaction

Author: danschultzer

lib/sentry/opentelemetry/span_processor.ex

@@ -57,8 +57,7 @@ defmodule Sentry.OpenTelemetry.SpanProcessor do
   defp build_transaction(%SpanRecord{origin: :undefined} = root_span, child_spans) do
     Transaction.new(%{
       transaction: root_span.name,
-      start_timestamp: root_span.start_time,
-      timestamp: root_span.end_time,
+      span_id: root_span.span_id,
       contexts: %{
         trace: build_trace_context(root_span)
       },
@@ -69,8 +68,7 @@ defmodule Sentry.OpenTelemetry.SpanProcessor do
   defp build_transaction(%SpanRecord{origin: "opentelemetry_ecto"} = root_span, child_spans) do
     Transaction.new(%{
       transaction: root_span.name,
-      start_timestamp: root_span.start_time,
-      timestamp: root_span.end_time,
+      span_id: root_span.span_id,
       transaction_info: %{
         source: "component"
       },
@@ -90,8 +88,7 @@ defmodule Sentry.OpenTelemetry.SpanProcessor do
        when map_size(attributes) > 0 do
     Transaction.new(%{
       transaction: "#{attributes["phoenix.plug"]}##{attributes["phoenix.action"]}",
-      start_timestamp: root_span.start_time,
-      timestamp: root_span.end_time,
+      span_id: root_span.span_id,
       transaction_info: %{
         source: "view"
       },
@@ -130,8 +127,7 @@ defmodule Sentry.OpenTelemetry.SpanProcessor do
        when map_size(attributes) == 0 do
     Transaction.new(%{
       transaction: root_span.name,
-      start_timestamp: root_span.start_time,
-      timestamp: root_span.end_time,
+      span_id: root_span.span_id,
       transaction_info: %{
         source: "view"
       },
@@ -147,8 +143,7 @@ defmodule Sentry.OpenTelemetry.SpanProcessor do
          child_spans
        ) do
     Transaction.new(%{
-      start_timestamp: root_span.start_time,
-      timestamp: root_span.end_time,
+      span_id: root_span.span_id,
       transaction: attributes["http.target"],
       transaction_info: %{
         source: "url"
@@ -175,8 +170,7 @@ defmodule Sentry.OpenTelemetry.SpanProcessor do
   defp build_transaction(%SpanRecord{origin: "opentelemetry_oban"} = root_span, child_spans) do
     Transaction.new(%{
       transaction: root_span.name |> String.split(" ") |> List.first(),
-      start_timestamp: root_span.start_time,
-      timestamp: root_span.end_time,
+      span_id: root_span.span_id,
       transaction_info: %{
         source: "task"
       },

lib/sentry/test.ex

@@ -361,7 +361,7 @@ defmodule Sentry.Test do
 
       iex> Sentry.Test.start_collecting_sentry_reports()
       :ok
-      iex> Sentry.send_transaction(%Sentry.Transaction{})
+      iex> Sentry.send_transaction(Sentry.Transaction.new(%{span_id: "123", spans: []}))
       {:ok, ""}
       iex> [%Sentry.Transaction{}] = Sentry.Test.pop_sentry_transactions()
 

lib/sentry/transaction.ex

@@ -3,23 +3,24 @@ defmodule Sentry.Transaction do
 
   alias Sentry.{Config, UUID}
 
+  @enforce_keys ~w(event_id span_id spans)a
+
   defstruct [
     :event_id,
     :environment,
-    :start_timestamp,
-    :timestamp,
+    :span_id,
     :transaction,
     :transaction_info,
-    :status,
     :contexts,
     :request,
+    :data,
     :measurements,
-    spans: [],
+    :spans,
     type: "transaction"
   ]
 
   def new(attrs) do
-    struct(
+    struct!(
       __MODULE__,
       attrs
       |> Map.put(:event_id, UUID.uuid4_hex())
@@ -30,27 +31,32 @@ defmodule Sentry.Transaction do
   # Used to then encode the returned map to JSON.
   @doc false
   def to_map(%__MODULE__{} = transaction) do
-    Map.put(
-      Map.from_struct(transaction),
-      :spans,
-      Enum.map(transaction.spans, &Sentry.Span.to_map(&1))
-    )
+    transaction_attrs = Map.take(transaction, [:event_id, :environment, :transaction, :transaction_info, :contexts, :measurements, :type])
+    {[root_span], child_spans} = Enum.split_with(transaction.spans, &is_nil(&1.parent_span_id))
+
+    root_span
+    |> Sentry.Span.to_map()
+    |> Map.put(:spans, Enum.map(child_spans, &Sentry.Span.to_map/1))
+    |> Map.drop([:description])
+    |> Map.merge(transaction_attrs)
   end
 end
 
 defmodule Sentry.Span do
+  @enforce_keys ~w(span_id trace_id start_timestamp timestamp)a
+
   defstruct [
-    :op,
+    :trace_id,
+    :span_id,
+    :parent_span_id,
     :start_timestamp,
     :timestamp,
     :description,
-    :span_id,
-    :parent_span_id,
-    :trace_id,
+    :op,
+    :status,
     :tags,
     :data,
-    :origin,
-    :status
+    :origin
   ]
 
   # Used to then encode the returned map to JSON.

test/envelope_test.exs

@@ -117,14 +117,14 @@ defmodule Sentry.EnvelopeTest do
     test "works with transactions" do
       put_test_config(environment_name: "test")
 
-      spans = [
+      root_span =
         %Sentry.Span{
           start_timestamp: 1_588_601_261.481_961,
           timestamp: 1_588_601_261.488_901,
           description: "GET /sockjs-node/info",
           op: "http",
           span_id: "b01b9f6349558cd1",
-          parent_span_id: "b0e6f15b45c36b12",
+          parent_span_id: nil,
           trace_id: "1e57b752bc6e4544bbaa246cd1d05dee",
           tags: %{"http.status_code" => "200"},
           data: %{
@@ -133,23 +133,27 @@ defmodule Sentry.EnvelopeTest do
             "type" => "xhr",
             "method" => "GET"
           }
-        },
-        %Sentry.Span{
-          start_timestamp: 1_588_601_261.535_386,
-          timestamp: 1_588_601_261.544_196,
-          description: "Vue <App>",
-          op: "update",
-          span_id: "b980d4dec78d7344",
-          parent_span_id: "9312d0d18bf51736",
-          trace_id: "1e57b752bc6e4544bbaa246cd1d05dee"
         }
-      ]
 
-      transaction = %Sentry.Transaction{
-        start_timestamp: System.system_time(:second),
-        timestamp: System.system_time(:second),
-        spans: spans
-      }
+      child_spans =
+        [
+          %Sentry.Span{
+            start_timestamp: 1_588_601_261.535_386,
+            timestamp: 1_588_601_261.544_196,
+            description: "Vue <App>",
+            op: "update",
+            span_id: "b980d4dec78d7344",
+            parent_span_id: "9312d0d18bf51736",
+            trace_id: "1e57b752bc6e4544bbaa246cd1d05dee"
+          }
+        ]
+
+      transaction =
+        Sentry.Transaction.new(%{
+          span_id: root_span.span_id,
+          spans: [root_span | child_spans],
+          transaction: "test-transaction"
+        })
 
       envelope = Envelope.from_transaction(transaction)
 
@@ -159,16 +163,13 @@ defmodule Sentry.EnvelopeTest do
 
       assert {:ok, decoded_transaction} = Jason.decode(transaction_line)
       assert decoded_transaction["type"] == "transaction"
-      assert decoded_transaction["start_timestamp"] == transaction.start_timestamp
-      assert decoded_transaction["timestamp"] == transaction.timestamp
-
-      assert [span1, span2] = decoded_transaction["spans"]
+      assert decoded_transaction["start_timestamp"] == root_span.start_timestamp
+      assert decoded_transaction["timestamp"] == root_span.timestamp
 
-      assert span1["start_timestamp"] == List.first(spans).start_timestamp
-      assert span1["timestamp"] == List.first(spans).timestamp
+      assert [span] = decoded_transaction["spans"]
 
-      assert span2["start_timestamp"] == List.last(spans).start_timestamp
-      assert span2["timestamp"] == List.last(spans).timestamp
+      assert span["start_timestamp"] == List.first(child_spans).start_timestamp
+      assert span["timestamp"] == List.first(child_spans).timestamp
     end
   end
 

test/sentry/client_report/sender_test.exs

@@ -4,7 +4,7 @@ defmodule Sentry.ClientReportTest do
   import Sentry.TestHelpers
 
   alias Sentry.ClientReport.Sender
-  alias Sentry.{Event, Transaction}
+  alias Sentry.{Event, Transaction, Span}
 
   setup do
     original_retries =
@@ -19,6 +19,8 @@ defmodule Sentry.ClientReportTest do
     %{bypass: bypass}
   end
 
+  @span_id Sentry.UUID.uuid4_hex()
+
   describe "record_discarded_events/2 + flushing" do
     test "succefully records the discarded event to the client report", %{bypass: bypass} do
       start_supervised!({Sender, name: :test_client_report})
@@ -28,10 +30,16 @@ defmodule Sentry.ClientReportTest do
           event_id: Sentry.UUID.uuid4_hex(),
           timestamp: "2024-10-12T13:21:13"
         },
-        %Transaction{
-          event_id: Sentry.UUID.uuid4_hex(),
-          timestamp: "2024-10-12T13:21:13"
-        }
+        Transaction.new(%{
+          span_id: @span_id,
+          transaction: "test-transaction",
+          spans: [%Span{
+            span_id: @span_id,
+            trace_id: Sentry.UUID.uuid4_hex(),
+            start_timestamp: "2024-10-12T13:21:13",
+            timestamp: "2024-10-12T13:21:13"
+          }]
+        })
       ]
 
       assert :ok = Sender.record_discarded_events(:before_send, events, :test_client_report)

test/sentry/opentelemetry/span_processor_test.exs

@@ -44,18 +44,17 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
     assert [%Sentry.Transaction{} = transaction] = Sentry.Test.pop_sentry_transactions()
 
-    assert transaction.environment == "test"
-    assert transaction.type == "transaction"
-    assert_valid_iso8601(transaction.timestamp)
-    assert_valid_iso8601(transaction.start_timestamp)
-    assert transaction.timestamp > transaction.start_timestamp
-    assert length(transaction.spans) == 1
-
-    assert_valid_trace_id(transaction.contexts.trace.trace_id)
-
-    assert [span] = transaction.spans
-
-    assert span.op == "child_instrumented_function_one"
+    transaction_data = Sentry.Transaction.to_map(transaction)
+
+    assert transaction_data.event_id
+    assert transaction_data.environment == "test"
+    assert transaction_data.type == "transaction"
+    assert transaction_data.op == "child_instrumented_function_one"
+    assert_valid_iso8601(transaction_data.timestamp)
+    assert_valid_iso8601(transaction_data.start_timestamp)
+    assert transaction_data.timestamp > transaction_data.start_timestamp
+    assert_valid_trace_id(transaction_data.contexts.trace.trace_id)
+    assert length(transaction_data.spans) == 0
   end
 
   test "sends captured spans as transactions with child spans" do
@@ -67,14 +66,15 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
     assert [%Sentry.Transaction{} = transaction] = Sentry.Test.pop_sentry_transactions()
 
-    assert transaction.environment == "test"
-    assert transaction.type == "transaction"
-    assert_valid_iso8601(transaction.timestamp)
-    assert_valid_iso8601(transaction.start_timestamp)
-    assert transaction.timestamp > transaction.start_timestamp
-    assert length(transaction.spans) == 3
+    transaction_data = Sentry.Transaction.to_map(transaction)
+
+    assert transaction_data.op == "instrumented_function"
+    assert_valid_iso8601(transaction_data.timestamp)
+    assert_valid_iso8601(transaction_data.start_timestamp)
+    assert transaction_data.timestamp > transaction_data.start_timestamp
+    assert length(transaction_data.spans) == 2
 
-    [root_span, child_span_one, child_span_two] = transaction.spans
+    [child_span_one, child_span_two] = transaction_data.spans
     assert child_span_one.op == "child_instrumented_function_one"
     assert child_span_two.op == "child_instrumented_function_two"
     assert child_span_one.parent_span_id == transaction.contexts.trace.span_id
@@ -87,10 +87,10 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
     assert child_span_one.timestamp > child_span_one.start_timestamp
     assert child_span_two.timestamp > child_span_two.start_timestamp
-    assert root_span.timestamp >= child_span_one.timestamp
-    assert root_span.timestamp >= child_span_two.timestamp
-    assert root_span.start_timestamp <= child_span_one.start_timestamp
-    assert root_span.start_timestamp <= child_span_two.start_timestamp
+    assert transaction_data.timestamp >= child_span_one.timestamp
+    assert transaction_data.timestamp >= child_span_two.timestamp
+    assert transaction_data.start_timestamp <= child_span_one.start_timestamp
+    assert transaction_data.start_timestamp <= child_span_two.start_timestamp
 
     assert_valid_trace_id(transaction.contexts.trace.trace_id)
     assert_valid_trace_id(child_span_one.trace_id)

test/sentry_test.exs

@@ -240,9 +240,16 @@ defmodule SentryTest do
     setup do
       transaction =
         Sentry.Transaction.new(%{
+          span_id: "root-span",
           transaction: "test-transaction",
-          start_timestamp: System.system_time(:second),
-          timestamp: System.system_time(:second)
+          spans: [
+            %Sentry.Span{
+              span_id: "root-span",
+              trace_id: "trace-id",
+              start_timestamp: 1_234_567_891.123_456,
+              timestamp: 1_234_567_891.123_456
+            }
+          ]
         })
 
       {:ok, transaction: transaction}