From 649a6beb7f7be51e1edd332b7cb9f1e0f8f68f4a Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 7 Jan 2025 14:05:14 +0000 Subject: [PATCH] WIP - refine Transaction Make it closer to the actual Sentry spec. This will require tweaking OpenTelemetry.SpanProcessor in the other branch so that it creates Transaction structs with the correct fields. --- lib/sentry/client.ex | 2 +- lib/sentry/interfaces.ex | 2 +- lib/sentry/test.ex | 2 +- lib/sentry/transaction.ex | 81 +++++++++++------------ test/envelope_test.exs | 44 ++++++------ test/sentry/client_report/sender_test.exs | 9 ++- test/sentry/transaction_test.exs | 24 +++++++ test/sentry_test.exs | 9 ++- test/support/test_helpers.ex | 36 ++++++++++ 9 files changed, 135 insertions(+), 74 deletions(-) create mode 100644 test/sentry/transaction_test.exs diff --git a/lib/sentry/client.ex b/lib/sentry/client.ex index 169d4e6a..0ce1b467 100644 --- a/lib/sentry/client.ex +++ b/lib/sentry/client.ex @@ -285,7 +285,7 @@ defmodule Sentry.Client do @spec render_transaction(%Transaction{}) :: map() def render_transaction(%Transaction{} = transaction) do transaction - |> Transaction.to_map() + |> Transaction.to_payload() |> Map.merge(%{ platform: "elixir", sdk: %{ diff --git a/lib/sentry/interfaces.ex b/lib/sentry/interfaces.ex index a5df9385..3efdf9fc 100644 --- a/lib/sentry/interfaces.ex +++ b/lib/sentry/interfaces.ex @@ -306,7 +306,7 @@ defmodule Sentry.Interfaces do ] @doc false - def to_map(%__MODULE__{} = span) do + def to_payload(%__MODULE__{} = span) do Map.from_struct(span) end end diff --git a/lib/sentry/test.ex b/lib/sentry/test.ex index 7ab4ab34..41fa0d0d 100644 --- a/lib/sentry/test.ex +++ b/lib/sentry/test.ex @@ -337,7 +337,7 @@ defmodule Sentry.Test do iex> Sentry.Test.start_collecting_sentry_reports() :ok - iex> Sentry.send_transaction(Sentry.Transaction.new(%{span_id: "123", spans: []})) + iex> Sentry.send_transaction(Sentry.Transaction.new(%{span_id: "123", start_timestamp: "2024-10-12T13:21:13", timestamp: "2024-10-12T13:21:13", spans: []})) {:ok, ""} iex> [%Sentry.Transaction{}] = Sentry.Test.pop_sentry_transactions() diff --git a/lib/sentry/transaction.ex b/lib/sentry/transaction.ex index c4d7a76f..68d53bbf 100644 --- a/lib/sentry/transaction.ex +++ b/lib/sentry/transaction.ex @@ -7,30 +7,54 @@ defmodule Sentry.Transaction do @moduledoc since: "11.0.0" + alias Sentry.{Config, UUID, Interfaces.Span, Interfaces.SDK} + @typedoc since: "11.0.0" + @type t() :: %{ + # Required + required(:event_id) => <<_::256>>, + required(:start_timestamp) => String.t() | number(), + required(:timestamp) => String.t() | number(), + required(:platform) => :elixir, + required(:contexts) => %{ + required(:trace) => %{ + required(:trace_id) => String.t(), + required(:span_id) => String.t(), + optional(:parent_span_id) => String.t(), + optional(:op) => String.t(), + optional(:description) => String.t(), + optional(:status) => String.t() + } + }, - alias Sentry.{Config, UUID, Interfaces.Span} + # Optional + optional(:environment) => String.t(), + optional(:transaction) => String.t(), + optional(:transaction_info) => map(), + optional(:measurements) => map(), + optional(:type) => String.t(), + optional(:tags) => map(), + optional(:data) => map(), - @type t() :: %__MODULE__{ - event_id: String.t(), - span_id: String.t(), - spans: [Span.t()], - environment: String.t(), - transaction: String.t(), - transaction_info: map(), - contexts: map(), - measurements: map(), - type: String.t() + # Interfaces + optional(:spans) => [Span.t()], + optional(:sdk) => SDK.t() } - @enforce_keys [:event_id, :span_id, :spans, :environment] + @enforce_keys [:event_id, :span_id, :start_timestamp, :timestamp] defstruct @enforce_keys ++ [ + :spans, :transaction, :transaction_info, :contexts, :measurements, + :sdk, + :platform, + :environment, + :tags, + :data, type: "transaction" ] @@ -44,35 +68,10 @@ defmodule Sentry.Transaction do ) end - # Common attributes between Transaction and Span: - # - # span_id - Used to identify spans in both structs - # type - Transaction has a default "transaction" type, Span has it in data - # data - Transaction has contexts/measurements, Span has data field - # timestamp - Transaction inherits from root span, Span has explicit timestamp - # @doc false - def to_map(%__MODULE__{} = transaction) do - 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)) - - IO.inspect(root_span) - IO.inspect(child_spans) - - root_span - |> Span.to_map() - |> Map.put(:spans, Enum.map(child_spans, &Span.to_map/1)) - |> Map.drop([:description]) - |> Map.merge(transaction_attrs) + def to_payload(%__MODULE__{} = transaction) do + transaction + |> Map.from_struct() + |> Map.update(:spans, [], fn spans -> Enum.map(spans, &Span.to_payload/1) end) end end diff --git a/test/envelope_test.exs b/test/envelope_test.exs index 3d3d1f59..296f3303 100644 --- a/test/envelope_test.exs +++ b/test/envelope_test.exs @@ -117,24 +117,6 @@ defmodule Sentry.EnvelopeTest do test "works with transactions" do put_test_config(environment_name: "test") - root_span = - %Sentry.Interfaces.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: nil, - trace_id: "1e57b752bc6e4544bbaa246cd1d05dee", - tags: %{"http.status_code" => "200"}, - data: %{ - "url" => "http://localhost:8080/sockjs-node/info?t=1588601703755", - "status_code" => 200, - "type" => "xhr", - "method" => "GET" - } - } - child_spans = [ %Sentry.Interfaces.Span{ @@ -149,9 +131,25 @@ defmodule Sentry.EnvelopeTest do ] transaction = - Sentry.Transaction.new(%{ - span_id: root_span.span_id, - spans: [root_span | child_spans], + create_transaction(%{ + start_timestamp: 1_588_601_261.481_961, + timestamp: 1_588_601_261.488_901, + contexts: %{ + trace: %{ + trace_id: "1e57b752bc6e4544bbaa246cd1d05dee", + span_id: "b01b9f6349558cd1", + description: "GET /sockjs-node/info", + op: "http" + } + }, + tags: %{"http.status_code" => "200"}, + data: %{ + "url" => "http://localhost:8080/sockjs-node/info?t=1588601703755", + "status_code" => 200, + "type" => "xhr", + "method" => "GET" + }, + spans: child_spans, transaction: "test-transaction" }) @@ -163,8 +161,8 @@ defmodule Sentry.EnvelopeTest do assert {:ok, decoded_transaction} = Jason.decode(transaction_line) assert decoded_transaction["type"] == "transaction" - assert decoded_transaction["start_timestamp"] == root_span.start_timestamp - assert decoded_transaction["timestamp"] == root_span.timestamp + assert decoded_transaction["start_timestamp"] == transaction.start_timestamp + assert decoded_transaction["timestamp"] == transaction.timestamp assert [span] = decoded_transaction["spans"] diff --git a/test/sentry/client_report/sender_test.exs b/test/sentry/client_report/sender_test.exs index 19104e83..cea2a7d5 100644 --- a/test/sentry/client_report/sender_test.exs +++ b/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, Interfaces.Span} + alias Sentry.Event setup do original_retries = @@ -30,16 +30,15 @@ defmodule Sentry.ClientReportTest do event_id: Sentry.UUID.uuid4_hex(), timestamp: "2024-10-12T13:21:13" }, - Transaction.new(%{ - span_id: @span_id, + create_transaction(%{ transaction: "test-transaction", spans: [ - %Span{ + create_span(%{ span_id: @span_id, trace_id: Sentry.UUID.uuid4_hex(), start_timestamp: "2024-10-12T13:21:13", timestamp: "2024-10-12T13:21:13" - } + }) ] }) ] diff --git a/test/sentry/transaction_test.exs b/test/sentry/transaction_test.exs new file mode 100644 index 00000000..bd2ee3c1 --- /dev/null +++ b/test/sentry/transaction_test.exs @@ -0,0 +1,24 @@ +defmodule Sentry.TransactionTest do + use Sentry.Case, async: true + + alias Sentry.Transaction + + import Sentry.TestHelpers + + describe "to_payload/1" do + setup do + {:ok, %{transaction: create_transaction()}} + end + + test "returns a map representation of the transaction", %{transaction: transaction} do + transaction_payload = Transaction.to_payload(transaction) + + [child_span] = transaction_payload[:spans] + + assert transaction_payload[:contexts][:trace][:trace_id] == "trace-312" + + assert child_span[:parent_span_id] == transaction.span_id + assert child_span[:span_id] == "span-123" + end + end +end diff --git a/test/sentry_test.exs b/test/sentry_test.exs index bfefb025..a3149afc 100644 --- a/test/sentry_test.exs +++ b/test/sentry_test.exs @@ -239,9 +239,14 @@ defmodule SentryTest do describe "send_transaction/2" do setup do transaction = - Sentry.Transaction.new(%{ - span_id: "root-span", + create_transaction(%{ transaction: "test-transaction", + contexts: %{ + trace: %{ + trace_id: "trace-id", + span_id: "root-span" + } + }, spans: [ %Sentry.Interfaces.Span{ span_id: "root-span", diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index eca6a617..2da39c97 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -2,6 +2,8 @@ defmodule Sentry.TestHelpers do import ExUnit.Assertions alias Sentry.Config + alias Sentry.Interfaces.Span + alias Sentry.Transaction @spec put_test_config(keyword()) :: :ok def put_test_config(config) when is_list(config) do @@ -50,6 +52,40 @@ defmodule Sentry.TestHelpers do decode_envelope_items(rest) end + def create_span(attrs \\ %{}) do + Map.merge( + %Span{ + trace_id: "trace-312", + span_id: "span-123", + start_timestamp: "2025-01-01T00:00:00Z", + timestamp: "2025-01-02T02:03:00Z" + }, + attrs + ) + end + + def create_transaction(attrs \\ %{}) do + Transaction.new( + Map.merge( + %{ + span_id: "parent-312", + start_timestamp: "2025-01-01T00:00:00Z", + timestamp: "2025-01-02T02:03:00Z", + contexts: %{ + trace: %{ + trace_id: "trace-312", + span_id: "parent-312" + } + }, + spans: [ + create_span(%{parent_span_id: "parent-312"}) + ] + }, + attrs + ) + ) + end + defp decode_envelope_items(items) do items |> Enum.chunk_every(2)