wip - add pending children tracking to SpanStorage

Author: solnic

lib/sentry/opentelemetry/span_storage.ex

@@ -61,6 +61,25 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do
       end
     end
 
+    @spec span_exists?(String.t(), keyword()) :: boolean()
+    def span_exists?(span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+
+      # Check root spans first
+      case :ets.lookup(table_name, {:root_span, span_id}) do
+        [_] ->
+          true
+
+        [] ->
+          # Check child spans - scan for any child span with this span_id
+          # This is O(n) but necessary when the span has a remote parent
+          case :ets.match_object(table_name, {{:child_span, :_, span_id}, :_, :_}) do
+            [_ | _] -> true
+            [] -> false
+          end
+      end
+    end
+
     @spec get_child_spans(String.t(), keyword()) :: [SpanRecord.t()]
     def get_child_spans(parent_span_id, opts \\ []) do
       table_name = Keyword.get(opts, :table_name, default_table_name())
@@ -121,6 +140,117 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do
       :ok
     end
 
+    @spec remove_child_span(String.t(), String.t(), keyword()) :: :ok
+    def remove_child_span(parent_span_id, span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+      key = {:child_span, parent_span_id, span_id}
+
+      :ets.delete(table_name, key)
+
+      :ok
+    end
+
+    # Pending children tracking functions
+    #
+    # These functions track spans that have started (on_start) but not yet ended (on_end).
+    # This is crucial for handling the race condition where a parent span's on_end is called
+    # before its child spans' on_end callbacks.
+    #
+    # The key insight is that when a child span starts, we record its existence in ETS.
+    # When it ends, we remove the pending record. This allows us to:
+    # 1. Know how many children are still in-flight when a parent ends
+    # 2. Defer transaction building until all pending children have ended
+    # 3. Have the last child trigger the transaction build
+
+    @doc """
+    Register a pending child span when it starts (via on_start callback).
+    This creates a lightweight record tracking that a child span is in-flight.
+    """
+    @spec store_pending_child(String.t(), String.t(), keyword()) :: true
+    def store_pending_child(parent_span_id, child_span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+      stored_at = System.system_time(:second)
+
+      key = {:pending_child, parent_span_id, child_span_id}
+      :ets.insert(table_name, {key, stored_at})
+    end
+
+    @doc """
+    Remove a pending child span when it ends (via on_end callback).
+    """
+    @spec remove_pending_child(String.t(), String.t(), keyword()) :: :ok
+    def remove_pending_child(parent_span_id, child_span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+      key = {:pending_child, parent_span_id, child_span_id}
+
+      :ets.delete(table_name, key)
+      :ok
+    end
+
+    @doc """
+    Check if a span has any pending (in-flight) children.
+    """
+    @spec has_pending_children?(String.t(), keyword()) :: boolean()
+    def has_pending_children?(parent_span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+
+      case :ets.match_object(table_name, {{:pending_child, parent_span_id, :_}, :_}, 1) do
+        {[_ | _], _} -> true
+        :"$end_of_table" -> false
+      end
+    end
+
+    @doc """
+    Store a completed parent span that's waiting for its children to finish.
+    This is used when a parent span ends but has pending children.
+    """
+    @spec store_waiting_parent(SpanRecord.t(), keyword()) :: true
+    def store_waiting_parent(span_record, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+      stored_at = System.system_time(:second)
+
+      key = {:waiting_parent, span_record.span_id}
+      :ets.insert(table_name, {key, span_record, stored_at})
+    end
+
+    @doc """
+    Get a waiting parent span by its span_id.
+    """
+    @spec get_waiting_parent(String.t(), keyword()) :: SpanRecord.t() | nil
+    def get_waiting_parent(span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+
+      case :ets.lookup(table_name, {:waiting_parent, span_id}) do
+        [{{:waiting_parent, ^span_id}, span_record, _stored_at}] -> span_record
+        [] -> nil
+      end
+    end
+
+    @doc """
+    Remove a waiting parent span after transaction has been built and sent.
+    """
+    @spec remove_waiting_parent(String.t(), keyword()) :: :ok
+    def remove_waiting_parent(span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+      :ets.delete(table_name, {:waiting_parent, span_id})
+      :ok
+    end
+
+    @doc """
+    Remove all pending children for a given parent span.
+    Used during cleanup when a transaction is sent.
+    """
+    @spec remove_pending_children(String.t(), keyword()) :: :ok
+    def remove_pending_children(parent_span_id, opts \\ []) do
+      table_name = Keyword.get(opts, :table_name, default_table_name())
+
+      :ets.select_delete(table_name, [
+        {{{:pending_child, parent_span_id, :_}, :_}, [], [true]}
+      ])
+
+      :ok
+    end
+
     defp schedule_cleanup(interval) do
       Process.send_after(self(), :cleanup_stale_spans, interval)
     end
@@ -144,6 +274,20 @@ if Sentry.OpenTelemetry.VersionChecker.tracing_compatible?() do
       ]
 
       :ets.select_delete(table_name, child_match_spec)
+
+      # Cleanup stale pending children
+      pending_child_match_spec = [
+        {{{:pending_child, :_, :_}, :"$1"}, [{:<, :"$1", cutoff_time}], [true]}
+      ]
+
+      :ets.select_delete(table_name, pending_child_match_spec)
+
+      # Cleanup stale waiting parents
+      waiting_parent_match_spec = [
+        {{{:waiting_parent, :_}, :_, :"$1"}, [{:<, :"$1", cutoff_time}], [true]}
+      ]
+
+      :ets.select_delete(table_name, waiting_parent_match_spec)
     end
 
     defp default_table_name do

test/sentry/opentelemetry/span_storage_test.exs

@@ -93,6 +93,62 @@ defmodule Sentry.OpenTelemetry.SpanStorageTest do
     end
   end
 
+  describe "span_exists?" do
+    @tag span_storage: true
+    test "returns true for existing root span", %{table_name: table_name} do
+      root_span = %SpanRecord{
+        span_id: "root123",
+        parent_span_id: nil,
+        trace_id: "trace123",
+        name: "root_span"
+      }
+
+      SpanStorage.store_span(root_span, table_name: table_name)
+
+      assert SpanStorage.span_exists?("root123", table_name: table_name) == true
+    end
+
+    @tag span_storage: true
+    test "returns true for existing child span", %{table_name: table_name} do
+      child_span = %SpanRecord{
+        span_id: "child123",
+        parent_span_id: "parent123",
+        trace_id: "trace123",
+        name: "child_span"
+      }
+
+      SpanStorage.store_span(child_span, table_name: table_name)
+
+      assert SpanStorage.span_exists?("child123", table_name: table_name) == true
+    end
+
+    @tag span_storage: true
+    test "returns false for non-existent span", %{table_name: table_name} do
+      assert SpanStorage.span_exists?("nonexistent", table_name: table_name) == false
+    end
+
+    @tag span_storage: true
+    test "returns true for HTTP server span with remote parent (distributed tracing)", %{
+      table_name: table_name
+    } do
+      # HTTP server span with a remote parent (from distributed tracing)
+      # is stored as a child span, not a root span
+      http_server_span = %SpanRecord{
+        span_id: "http_span_123",
+        parent_span_id: "remote_parent_456",
+        trace_id: "trace123",
+        name: "GET /users"
+      }
+
+      SpanStorage.store_span(http_server_span, table_name: table_name)
+
+      # Should find the span even though it has a remote parent
+      assert SpanStorage.span_exists?("http_span_123", table_name: table_name) == true
+      # Should NOT find the remote parent
+      assert SpanStorage.span_exists?("remote_parent_456", table_name: table_name) == false
+    end
+  end
+
   describe "child spans" do
     @tag span_storage: true
     test "stores and retrieves child spans", %{table_name: table_name} do