Don't set params when send_params is false (#771)

* Add missing test to set parameter values

There was no test to make sure the params are actually set. This patch
adds a regression test to make sure this isn't broken through future
work.

* Don't set params when send_params is false

Currently, the send_params configuration option is checked in
appsignal_plug, in Appsignal.Plug.set_params/2-3.

Since parameters are now also added from appsignal_phoenix, and because
there's already a distinction between sample data and parameters in
Appsignal.Span, Appsignal.Span.set_sample_data/3-4 now checks the
send_params configuration if the passed key equals "params".

The implementation in appsignal_plug can be removed when depending on
the upcoming version of this library.

.changesets/don't-set-parameters-when-the-send_params-configuration-is-set-to-false.md

@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "add"
+---
+
+Don't set parameters when the send_params configuration is set to false

lib/appsignal/span.ex

@@ -190,11 +190,19 @@ defmodule Appsignal.Span do
       |> Appsignal.Span.set_sample_data("environment", %{"method" => "GET"})
 
   """
-  def set_sample_data(span, "params", value) do
+  def set_sample_data(span, key, value) do
+    set_sample_data(span, Application.get_env(:appsignal, :config), key, value)
+  end
+
+  defp set_sample_data(span, %{send_params: true}, "params", value) do
     do_set_sample_data(span, "params", Appsignal.Utils.MapFilter.filter(value))
   end
 
-  def set_sample_data(span, key, value) do
+  defp set_sample_data(span, _config, "params", _value) do
+    span
+  end
+
+  defp set_sample_data(span, _config, key, value) do
     do_set_sample_data(span, key, value)
   end
 

test/appsignal/span_test.exs

@@ -415,12 +415,61 @@ defmodule AppsignalSpanTest do
     setup :create_root_span
 
     setup %{span: span} do
-      [return: Span.set_sample_data(span, "key", %{param: "value"})]
+      [return: Span.set_sample_data(span, "key", %{foo: "bar"})]
     end
 
     test "returns the span", %{span: span, return: return} do
       assert return == span
     end
+
+    @tag :skip_env_test_no_nif
+    test "sets the sample data", %{span: span} do
+      assert %{"sample_data" => %{"key" => "{\"foo\":\"bar\"}"}} = Span.to_map(span)
+    end
+  end
+
+  describe ".set_sample_data/3, if send_params is set to false" do
+    setup :create_root_span
+
+    setup %{span: span} do
+      config = Application.get_env(:appsignal, :config)
+      Application.put_env(:appsignal, :config, %{config | send_params: false})
+
+      try do
+        Span.set_sample_data(span, "key", %{foo: "bar"})
+      after
+        Application.put_env(:appsignal, :config, config)
+      end
+
+      :ok
+    end
+
+    @tag :skip_env_test_no_nif
+    test "sets the sample data", %{span: span} do
+      assert %{"sample_data" => %{"key" => "{\"foo\":\"bar\"}"}} = Span.to_map(span)
+    end
+  end
+
+  describe ".set_sample_data/3, if send_params is set to false, when using 'params' as the key" do
+    setup :create_root_span
+
+    setup %{span: span} do
+      config = Application.get_env(:appsignal, :config)
+      Application.put_env(:appsignal, :config, %{config | send_params: false})
+
+      try do
+        Span.set_sample_data(span, "params", %{foo: "bar"})
+      after
+        Application.put_env(:appsignal, :config, config)
+      end
+
+      :ok
+    end
+
+    @tag :skip_env_test_no_nif
+    test "does not set the sample data", %{span: span} do
+      assert Span.to_map(span)["sample_data"] == %{}
+    end
   end
 
   describe ".set_sample_data/3, when passing a nil-span" do