Merge pull request #417 from griffitr/optionally-disable-stacktrace-args

Optionally disable stacktrace args

lib/new_relic/config.ex

@@ -159,6 +159,10 @@ defmodule NewRelic.Config do
     get(:features, :function_argument_collection)
   end
 
+  def feature?(:stacktrace_argument_collection) do
+    get(:features, :stacktrace_argument_collection)
+  end
+
   def feature?(:request_queuing_metrics) do
     get(:features, :request_queuing_metrics)
   end

lib/new_relic/init.ex

@@ -96,6 +96,11 @@ defmodule NewRelic.Init do
           "NEW_RELIC_FUNCTION_ARGUMENT_COLLECTION_ENABLED",
           :function_argument_collection_enabled
         ),
+      stacktrace_argument_collection:
+        determine_feature(
+          "NEW_RELIC_STACKTRACE_ARGUMENT_COLLECTION_ENABLED",
+          :stacktrace_argument_collection_enabled
+        ),
       request_queuing_metrics:
         determine_feature(
           "NEW_RELIC_REQUEST_QUEUING_METRICS_ENABLED",

lib/new_relic/util/error.ex

@@ -38,15 +38,37 @@ defmodule NewRelic.Util.Error do
 
   def format_stacktrace(stacktrace, initial_call),
     do:
-      List.wrap(stacktrace)
+      maybe_remove_args_from_stacktrace(stacktrace)
+      |> List.wrap()
       |> prepend_initial_call(initial_call)
       |> Enum.map(fn
         line when is_binary(line) -> line
         entry when is_tuple(entry) -> Exception.format_stacktrace_entry(entry)
       end)
 
-  defp prepend_initial_call(stacktrace, {mod, fun, args}),
-    do: stacktrace ++ [{mod, fun, args, []}]
+  defp prepend_initial_call(stacktrace, {mod, fun, args}) do
+    if NewRelic.Config.feature?(:stacktrace_argument_collection) do
+      stacktrace ++ [{mod, fun, args, []}]
+    else
+      stacktrace ++ [{mod, fun, ["DISABLED (arity: #{length(args)})"], []}]
+    end
+  end
+
+  defp prepend_initial_call(stacktrace, _) do
+    stacktrace
+  end
+
+  defp maybe_remove_args_from_stacktrace(stacktrace) do
+    if NewRelic.Config.feature?(:stacktrace_argument_collection) do
+      stacktrace
+    else
+      remove_args_from_stacktrace(stacktrace)
+    end
+  end
+
+  defp remove_args_from_stacktrace([{mod, fun, [_ | _] = args, info} | rest]),
+    do: [{mod, fun, ["DISABLED (arity: #{length(args)})"], info} | rest]
 
-  defp prepend_initial_call(stacktrace, _), do: stacktrace
+  defp remove_args_from_stacktrace(stacktrace) when is_list(stacktrace),
+    do: stacktrace
 end

test/error_test.exs

@@ -9,8 +9,10 @@ defmodule ErrorTest do
     def start_link, do: GenServer.start_link(__MODULE__, :ok, name: __MODULE__)
     @compile {:no_warn_undefined, Not}
     def handle_call(:nofun, _from, _state), do: Not.a_function(:one, :two)
+    def handle_call(:secretfun, _from, _state), do: Not.secretfun("secretvalue", "other_secret")
     def handle_call(:sleep, _from, _state), do: :timer.sleep(:infinity)
     def handle_call(:raise, _from, _state), do: raise("ERROR")
+    def handle_call(:erlang_error, _from, _state), do: raise(:erlang.error(:badarg))
   end
 
   test "Catch and harvest errors" do
@@ -31,6 +33,38 @@ defmodule ErrorTest do
            end)
   end
 
+  test "Redact arguments when not gathering stacktrace args" do
+    Process.flag(:trap_exit, true)
+    TestHelper.restart_harvest_cycle(Collector.TransactionErrorEvent.HarvestCycle)
+    TestHelper.restart_harvest_cycle(Collector.ErrorTrace.HarvestCycle)
+    ErrorDummy.start_link()
+
+    reset_features = TestHelper.update(:nr_features, stacktrace_argument_collection: false)
+
+    capture_log(fn ->
+      catch_exit do
+        GenServer.call(ErrorDummy, :secretfun)
+      end
+    end)
+
+    [[_, event_error, _]] = TestHelper.gather_harvest(Collector.TransactionErrorEvent.Harvester)
+    [[_, _, _, _, trace_error, _]] = TestHelper.gather_harvest(Collector.ErrorTrace.Harvester)
+
+    refute String.contains?(event_error.stacktrace, "secretvalue")
+    refute String.contains?(event_error.stacktrace, "other_secret")
+    assert String.contains?(event_error.stacktrace, "Not.secretfun(\"DISABLED (arity: 2)\")")
+
+    refute String.contains?(List.first(trace_error.stack_trace), "secretvalue")
+    refute String.contains?(List.first(trace_error.stack_trace), "other_secret")
+
+    assert String.contains?(
+             List.first(trace_error.stack_trace),
+             "Not.secretfun(\"DISABLED (arity: 2)\")"
+           )
+
+    reset_features.()
+  end
+
   test "Catch a raised Error" do
     Process.flag(:trap_exit, true)
     TestHelper.restart_harvest_cycle(Collector.TransactionErrorEvent.HarvestCycle)
@@ -83,6 +117,27 @@ defmodule ErrorTest do
            end)
   end
 
+  test "Handle Erlang exception when stacktrace arg colleection is turned off" do
+    Process.flag(:trap_exit, true)
+    TestHelper.restart_harvest_cycle(Collector.TransactionErrorEvent.HarvestCycle)
+    TestHelper.restart_harvest_cycle(Collector.ErrorTrace.HarvestCycle)
+    ErrorDummy.start_link()
+
+    reset_features = TestHelper.update(:nr_features, stacktrace_argument_collection: false)
+
+    capture_log(fn ->
+      catch_exit do
+        GenServer.call(ErrorDummy, :erlang_error)
+      end
+    end)
+
+    [[_, event_error, _]] = TestHelper.gather_harvest(Collector.TransactionErrorEvent.Harvester)
+
+    assert String.contains?(event_error.stacktrace, "init(\"DISABLED (arity: 1)\")")
+
+    reset_features.()
+  end
+
   test "Catch a simple raise" do
     TestHelper.restart_harvest_cycle(Collector.TransactionErrorEvent.HarvestCycle)