Skip to content

Commit

Permalink
Support lists as sample data root arguments
Browse files Browse the repository at this point in the history
We documented that we support lists as sample data root values, but in
practice we did not.

This fix allows for setting lists as custom sample data (and other types
of sample data) as shown in the changeset.
  • Loading branch information
tombruijn committed Jan 12, 2024
1 parent a339fc1 commit 92d948f
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 1 deletion.
17 changes: 17 additions & 0 deletions .changesets/support-lists-as-root-sample-data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
bump: "patch"
type: "change"
---

Add support for lists in the sample data as root values on spans, as shown below. Previously we only supported lists as nested objects in maps.

```elixir
Appsignal.Span.set_sample_data(
Appsignal.Tracer.root_span,
"custom_data",
[
"value 1",
"value 2"
]
)
```
2 changes: 1 addition & 1 deletion lib/appsignal/span.ex
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ defmodule Appsignal.Span do
end

defp do_set_sample_data(%Span{reference: reference} = span, key, value, setter)
when is_binary(key) and is_map(value) do
when is_binary(key) and (is_map(value) or is_list(value)) do
data = Appsignal.Utils.DataEncoder.encode(value)

:ok = setter.(reference, key, data)
Expand Down
60 changes: 60 additions & 0 deletions test/appsignal/span_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,36 @@ defmodule AppsignalSpanTest do
end
end

describe ".set_sample_data/3, with a list" do
setup :create_root_span

setup %{span: span} do
Span.set_sample_data(span, "custom_data", ["abc", "def"])

:ok
end

@tag :skip_env_test_no_nif
test "sets the list as sample data", %{span: span} do
assert %{"sample_data" => %{"custom_data" => "[\"abc\",\"def\"]"}} = Span.to_map(span)
end
end

describe ".set_sample_data/3, with a keyword list" do
setup :create_root_span

setup %{span: span} do
Span.set_sample_data(span, "custom_data", abc: "def")

:ok
end

@tag :skip_env_test_no_nif
test "sets the keyword list as sample data", %{span: span} do
assert %{"sample_data" => %{"custom_data" => "[[\"abc\",\"def\"]]"}} = Span.to_map(span)
end
end

describe ".set_sample_data/3, when passing invalid data" do
setup :create_root_span

Expand Down Expand Up @@ -655,6 +685,36 @@ defmodule AppsignalSpanTest do
end
end

describe ".set_sample_data_if_nil/3, with a list" do
setup :create_root_span

setup %{span: span} do
Span.set_sample_data_if_nil(span, "custom_data", ["abc", "def"])

:ok
end

@tag :skip_env_test_no_nif
test "sets the list as sample data", %{span: span} do
assert %{"sample_data" => %{"custom_data" => "[\"abc\",\"def\"]"}} = Span.to_map(span)
end
end

describe ".set_sample_data_if_nil/3, with a keyword list" do
setup :create_root_span

setup %{span: span} do
Span.set_sample_data_if_nil(span, "custom_data", abc: "def")

:ok
end

@tag :skip_env_test_no_nif
test "sets the keyword list as sample data", %{span: span} do
assert %{"sample_data" => %{"custom_data" => "[[\"abc\",\"def\"]]"}} = Span.to_map(span)
end
end

describe ".set_sample_data_if_nil/3, when passing invalid data" do
setup :create_root_span

Expand Down

0 comments on commit 92d948f

Please sign in to comment.