Skip to content

Commit

Permalink
Add set_name_if_nil helper
Browse files Browse the repository at this point in the history
Add helper to not set the name on a span if it's already set. We can use
this to only set the span name (and the root span's (action) name), to
not overwrite custom (action) names set by the app.

Add the name field to the span struct to avoid having to update the
extension and agent to handle this logic.

Part of #869
  • Loading branch information
tombruijn committed May 24, 2024
1 parent 0346f01 commit 0b84e60
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 1 deletion.
6 changes: 6 additions & 0 deletions .changesets/add-span-set_name_if_nil-helper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: add
---

Add `Appsignal.Span.set_name_if_nil` helper. This helper can be used to not overwrite previously set span names, and only set the span name if it wasn't set previously. This will used most commonly in AppSignal created integrations with other libraries to allow apps to set custom span names.
21 changes: 21 additions & 0 deletions c_src/appsignal_extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,26 @@ static ERL_NIF_TERM _set_span_name(ErlNifEnv* env, int argc, const ERL_NIF_TERM
return ok_atom;
}

static ERL_NIF_TERM _set_span_name_if_nil(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
span_ptr *ptr;
ErlNifBinary name;

if (argc != 2) {
return enif_make_badarg(env);
}
if(!enif_get_resource(env, argv[0], appsignal_span_type, (void**) &ptr)) {
return enif_make_badarg(env);
}
if(!enif_inspect_iolist_as_binary(env, argv[1], &name)) {
return enif_make_badarg(env);
}

appsignal_set_span_name_if_nil(ptr->span, make_appsignal_string(name));

return ok_atom;
}

static ERL_NIF_TERM _set_span_namespace(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
span_ptr *ptr;
Expand Down Expand Up @@ -1445,6 +1465,7 @@ static ErlNifFunc nif_funcs[] =
{"_create_child_span", 1, _create_child_span, 0},
{"_create_child_span_with_timestamp", 3, _create_child_span_with_timestamp, 0},
{"_set_span_name", 2, _set_span_name, 0},
{"_set_span_name_if_nil", 2, _set_span_name_if_nil, 0},
{"_set_span_namespace", 2, _set_span_namespace, 0},
{"_set_span_attribute_string", 3, _set_span_attribute_string, 0},
{"_set_span_attribute_int", 3, _set_span_attribute_int, 0},
Expand Down
8 changes: 8 additions & 0 deletions lib/appsignal/nif.ex
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ defmodule Appsignal.Nif do
_set_span_name(reference, name)
end

def set_span_name_if_nil(reference, name) do
_set_span_name_if_nil(reference, name)
end

def set_span_namespace(reference, namespace) do
_set_span_namespace(reference, namespace)
end
Expand Down Expand Up @@ -448,6 +452,10 @@ defmodule Appsignal.Nif do
:ok
end

def _set_span_name_if_nil(_reference, _name) do
:ok
end

def _set_span_namespace(_reference, _namespace) do
:ok
end
Expand Down
17 changes: 17 additions & 0 deletions lib/appsignal/span.ex
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,23 @@ defmodule Appsignal.Span do

def set_name(span, _name), do: span

@spec set_name_if_nil(t() | nil, String.t()) :: t() | nil
@doc """
Sets an `Appsignal.Span`'s name if it was not set before.
## Example
Appsignal.Tracer.root_span()
|> Appsignal.Span.set_name_if_nil("PageController#index")
"""
def set_name_if_nil(%Span{reference: reference} = span, name)
when is_reference(reference) and is_binary(name) do
:ok = @nif.set_span_name_if_nil(reference, name)
span
end

def set_name_if_nil(span, _name), do: span

@spec set_namespace(t() | nil, String.t()) :: t() | nil
@doc """
Sets an `Appsignal.Span`'s namespace. The namespace is `"http_request"` or
Expand Down
5 changes: 5 additions & 0 deletions lib/appsignal/test/span.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ defmodule Appsignal.Test.Span do
Span.set_name(span, name)
end

def set_name_if_nil(span, name) do
add(:set_name_if_nil, {span, name})
Span.set_name_if_nil(span, name)
end

def set_namespace(span, name) do
add(:set_namespace, {span, name})
Span.set_namespace(span, name)
Expand Down
45 changes: 44 additions & 1 deletion test/appsignal/span_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ defmodule AppsignalSpanTest do
[return: Span.set_name(span, "test")]
end

test "returns a span", %{span: span, return: return} do
test "returns a span with the name set", %{span: span, return: return} do
assert return == span
end

Expand Down Expand Up @@ -203,6 +203,49 @@ defmodule AppsignalSpanTest do
end
end

describe ".set_name_if_nil/2, with span that doesn't have a name yet" do
setup :create_root_span

setup %{span: span} do
[return: Span.set_name_if_nil(span, "original name")]
end

test "when no name is set it returns a span with the name set", %{span: span, return: return} do
assert return == span
end

@tag :skip_env_test_no_nif
test "sets the name through the Nif", %{span: span, return: return} do
assert %{"name" => "original name"} = Span.to_map(updated)
end

test "returns nil when passing a nil-span" do
assert Span.set_name(nil, "test") == nil
end
end

describe ".set_name_if_nil/2, with span that does have a name already" do
setup :create_root_span

setup %{span: span} do
span = Span.set_name_if_nil(span, "original name")
[return: Span.set_name_if_nil(span, "updated name")]
end

test "whereturns a span with the name set", %{span: span, return: return} do
assert return == span
end

@tag :skip_env_test_no_nif
test "doesn't update the name through the Nif", %{span: span, return: return} do
assert %{"name" => "original name"} = Span.to_map(updated)
end

test "returns nil when passing a nil-span" do
assert Span.set_name(nil, "test") == nil
end
end

describe ".set_namespace/2" do
setup :create_root_span

Expand Down

0 comments on commit 0b84e60

Please sign in to comment.