From 0b80110e22fac0aa5b71bf36ce056434488930d9 Mon Sep 17 00:00:00 2001 From: Jeff Kreeftmeijer Date: Wed, 25 May 2022 16:09:42 +0200 Subject: [PATCH 1/2] 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. --- test/appsignal/span_test.exs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/appsignal/span_test.exs b/test/appsignal/span_test.exs index ff5a4260f..093c349a4 100644 --- a/test/appsignal/span_test.exs +++ b/test/appsignal/span_test.exs @@ -415,12 +415,16 @@ 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 + + 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, when passing a nil-span" do From b2204f52342071e82c7e5c1bff3189ddd9d1e745 Mon Sep 17 00:00:00 2001 From: Jeff Kreeftmeijer Date: Wed, 25 May 2022 16:27:39 +0200 Subject: [PATCH 2/2] 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. --- ...nd_params-configuration-is-set-to-false.md | 6 +++ lib/appsignal/span.ex | 12 +++++- test/appsignal/span_test.exs | 42 +++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 .changesets/don't-set-parameters-when-the-send_params-configuration-is-set-to-false.md diff --git a/.changesets/don't-set-parameters-when-the-send_params-configuration-is-set-to-false.md b/.changesets/don't-set-parameters-when-the-send_params-configuration-is-set-to-false.md new file mode 100644 index 000000000..b1fd3d38a --- /dev/null +++ b/.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 diff --git a/lib/appsignal/span.ex b/lib/appsignal/span.ex index bf2c7eca8..442930447 100644 --- a/lib/appsignal/span.ex +++ b/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 diff --git a/test/appsignal/span_test.exs b/test/appsignal/span_test.exs index 093c349a4..54299ac96 100644 --- a/test/appsignal/span_test.exs +++ b/test/appsignal/span_test.exs @@ -427,6 +427,48 @@ defmodule AppsignalSpanTest do 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 + + 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 + + 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 test "returns nil" do assert Span.set_sample_data(nil, "key", %{param: "value"}) == nil