Merge pull request #460 from newrelic/vince/backoff-sampler-faster

Speed up BackoffSampler with counters

examples/apps/test_support/lib/test_support.ex

@@ -44,7 +44,7 @@ defmodule TestSupport do
   def simulate_agent_run(_context) do
     reset_config = update(:nr_config, license_key: "dummy_key", harvest_enabled: true)
     reset_agent_run = update(:nr_agent_run, trusted_account_key: "190")
-    send(NewRelic.DistributedTrace.BackoffSampler, :reset)
+    NewRelic.DistributedTrace.BackoffSampler.reset()
 
     ExUnit.Callbacks.on_exit(fn ->
       reset_config.()

lib/new_relic/distributed_trace/backoff_sampler.ex

@@ -3,60 +3,66 @@ defmodule NewRelic.DistributedTrace.BackoffSampler do
   alias NewRelic.Harvest.Collector.AgentRun
 
   # This GenServer tracks the sampling rate across sampling periods,
-  # which is used to determine when to sample a Distributed Trace
+  # which is used to determine when to sample a Distributed Trace.
+  # State is stored in erlang `counters` which are super fast
 
   @moduledoc false
 
   def start_link(_) do
     GenServer.start_link(__MODULE__, :ok, name: __MODULE__)
   end
 
+  # Counter indexes
+  @size 5
+  @cycle_number 1
+  @sampled_true_count 2
+  @decided_count 3
+  @decided_count_last 4
+  @sampling_target 5
+
   def init(:ok) do
     NewRelic.sample_process()
-    trigger_next_cycle()
-    {:ok, init_state()}
-  end
 
-  def init_state() do
-    %{
-      sampling_target: AgentRun.lookup(:sampling_target) || 10,
-      cycle_number: 0,
-      sampled_true_count: 0,
-      decided_count: 0,
-      decided_count_last: 0
-    }
-  end
+    :persistent_term.put({__MODULE__, :counter}, new(@size, []))
+    put(@sampling_target, AgentRun.lookup(:sampling_target) || 10)
 
-  def sample?, do: GenServer.call(__MODULE__, :sample?)
+    {:ok, %{}}
+  end
 
-  def handle_call(:sample?, _from, state) do
-    {sampled, state} = calculate(state)
-    {:reply, sampled, state}
+  def sample? do
+    calculate(%{
+      cycle_number: get(@cycle_number),
+      sampled_true_count: get(@sampled_true_count),
+      decided_count: get(@decided_count),
+      decided_count_last: get(@decided_count_last),
+      sampling_target: get(@sampling_target)
+    })
   end
 
   def handle_info(:cycle, state) do
+    cycle()
     trigger_next_cycle()
-    {:noreply, cycle(state)}
+    {:noreply, state}
   end
 
-  def handle_info(:reset, _state) do
-    {:noreply, init_state()}
+  def reset() do
+    put(@cycle_number, 0)
+    put(@decided_count_last, 0)
+    put(@decided_count, 0)
+    put(@sampled_true_count, 0)
   end
 
-  def cycle(state) do
-    %{
-      state
-      | cycle_number: state.cycle_number + 1,
-        sampled_true_count: 0,
-        decided_count: 0,
-        decided_count_last: state.decided_count
-    }
+  def cycle() do
+    incr(@cycle_number)
+    put(@decided_count_last, get(@decided_count))
+    put(@decided_count, 0)
+    put(@sampled_true_count, 0)
   end
 
   def calculate(state) do
     sampled = do_sample?(state)
-    state = update_state(sampled, state)
-    {sampled, state}
+    update_state(sampled)
+    sampled
   end
 
   def do_sample?(%{
@@ -91,18 +97,22 @@ defmodule NewRelic.DistributedTrace.BackoffSampler do
     Process.send_after(self(), :cycle, cycle_period)
   end
 
-  def update_state(false = _sampled?, state) do
-    %{state | decided_count: state.decided_count + 1}
+  def update_state(false = _sampled?) do
+    incr(@decided_count)
   end
 
-  def update_state(true = _sampled?, state) do
-    %{
-      state
-      | decided_count: state.decided_count + 1,
-        sampled_true_count: state.sampled_true_count + 1
-    }
+  def update_state(true = _sampled?) do
+    incr(@decided_count)
+    incr(@sampled_true_count)
   end
 
   def random(0), do: 0
   def random(n), do: :rand.uniform(n)
+
+  @compile {:inline, new: 2, incr: 1, put: 2, get: 1, pt: 0}
+  defp new(size, opts), do: :counters.new(size, opts)
+  defp incr(index), do: :counters.add(pt(), index, 1)
+  defp put(index, value), do: :counters.put(pt(), index, value)
+  defp get(index), do: :counters.get(pt(), index)
+  def pt(), do: :persistent_term.get({__MODULE__, :counter})
 end

test/backoff_sampler_test.exs

@@ -5,8 +5,9 @@ defmodule BackoffSamplerTest do
   test "Backoff behavior as best as we can" do
     # This is testing an inherently random algorithm
 
-    send(BackoffSampler, :reset)
+    BackoffSampler.reset()
 
+    # Target is 10, so it will sample the first 10
     assert BackoffSampler.sample?()
     assert BackoffSampler.sample?()
     assert BackoffSampler.sample?()
@@ -18,6 +19,7 @@ defmodule BackoffSamplerTest do
     assert BackoffSampler.sample?()
     assert BackoffSampler.sample?()
 
+    # The rest will be ignored
     refute BackoffSampler.sample?()
     refute BackoffSampler.sample?()
     refute BackoffSampler.sample?()
@@ -29,8 +31,25 @@ defmodule BackoffSamplerTest do
     refute BackoffSampler.sample?()
     refute BackoffSampler.sample?()
 
-    send(BackoffSampler, :cycle)
+    BackoffSampler.cycle()
 
+    # Next cycle it will adjust and take some, but not all
+    decisions = [
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?(),
+      BackoffSampler.sample?()
+    ]
+
+    assert true in decisions
+    assert false in decisions
+
+    # Next cycle it will adjust and take some, but not all
     decisions = [
       BackoffSampler.sample?(),
       BackoffSampler.sample?(),

test/distributed_trace_test.exs

@@ -58,7 +58,7 @@ defmodule DistributedTraceTest do
         primary_application_id: 1441
       )
 
-    send(DistributedTrace.BackoffSampler, :reset)
+    NewRelic.DistributedTrace.BackoffSampler.reset()
 
     on_exit(fn ->
       reset_config.()

test/other_transaction_test.exs

@@ -6,7 +6,7 @@ defmodule OtherTransactionTest do
 
   setup do
     reset_config = TestHelper.update(:nr_config, license_key: "dummy_key", harvest_enabled: true)
-    send(NewRelic.DistributedTrace.BackoffSampler, :reset)
+    NewRelic.DistributedTrace.BackoffSampler.reset()
 
     on_exit(fn ->
       reset_config.()

test/span_event_test.exs

@@ -12,7 +12,7 @@ defmodule SpanEventTest do
     reset_agent_run = TestHelper.update(:nr_agent_run, trusted_account_key: "190")
     reset_config = TestHelper.update(:nr_config, license_key: "dummy_key", harvest_enabled: true)
 
-    send(DistributedTrace.BackoffSampler, :reset)
+    DistributedTrace.BackoffSampler.reset()
 
     on_exit(fn ->
       reset_agent_run.()

test/support/test_helper.ex

@@ -69,7 +69,7 @@ defmodule TestHelper do
   def simulate_agent_run(_context) do
     reset_config = TestHelper.update(:nr_config, license_key: "dummy_key", harvest_enabled: true)
     reset_agent_run = TestHelper.update(:nr_agent_run, trusted_account_key: "190")
-    send(NewRelic.DistributedTrace.BackoffSampler, :reset)
+    NewRelic.DistributedTrace.BackoffSampler.reset()
 
     ExUnit.Callbacks.on_exit(fn ->
       reset_config.()

test/transaction_trace_test.exs

@@ -7,11 +7,10 @@ defmodule TransactionTraceTest do
   alias NewRelic.Transaction.Trace
 
   setup do
-    send(NewRelic.DistributedTrace.BackoffSampler, :reset)
     TestHelper.restart_harvest_cycle(Collector.Metric.HarvestCycle)
     TestHelper.restart_harvest_cycle(Collector.TransactionTrace.HarvestCycle)
     TestHelper.restart_harvest_cycle(Collector.SpanEvent.HarvestCycle)
-    send(NewRelic.DistributedTrace.BackoffSampler, :reset)
+    NewRelic.DistributedTrace.BackoffSampler.reset()
 
     on_exit(fn ->
       TestHelper.pause_harvest_cycle(Collector.Metric.HarvestCycle)

test/wc3_trace_context_test.exs

@@ -33,7 +33,7 @@ defmodule W3CTraceContextTest do
 
     reset_config = TestHelper.update(:nr_config, license_key: "dummy_key", harvest_enabled: true)
 
-    send(DistributedTrace.BackoffSampler, :reset)
+    DistributedTrace.BackoffSampler.reset()
 
     on_exit(fn ->
       reset_agent_run.()