Skip to content

Conversation

@tombruijn
Copy link
Member

@tombruijn tombruijn commented Jun 17, 2025

Use nif module attribute in Span module

Ensure we use the @nif module attribute everywhere, so that when we test something, to see if the Nif is called, we can actually verify it. I ran into this testing the set_attribute function calls.

Add namespace via attributes

In issue #972 it was reported it was difficult to override the default set namespace because our Absinthe instrumentation also sets the namespace to 'graphql', which is called later than some plugs. We have no 'only set namespace if not already set' logic, because the namespace is always set upon root span creation, so it would be difficult to determine when the namespace value was changed.

In the linked agent PR (private link) we added logic for the Node.js and Python OpenTelemetry integrations to set namespaces with the appsignal.namespace and appsignal.namespace_if_nil attributes. https://github.com/appsignal/appsignal-agent/pull/1274

This implementation also works for the Elixir integration, because they all use the Span API.

If the appsignal.namespace attribute is set, it will use the value as the namespace, overwriting the (default) namespace. If only the appsignal.namespace_if_nil attribute is set, it will use the value as the namespace, overwriting the default namespace. If both are set, it will use the appsignal.namespace attribute value.

Since this uses the internal hoisting mechanism, it means the namespace doesn't need to be set on the root span, but any span in the trace will work. Not sure if we want to update the docs on this, or just keep recommending to use the Span.set_namespace helper on the root span.

The Absinthe instrumentation (and potentially others) need to be updated to use the set_namespace_if_nil helper so that it is easier to overwrite the namespace for our users.

Allow customization of Absinthe trace namespace

Previously, it was not possible to customize the namespace of an Absinthe trace as detailed in issue #972. This was because the plug that set the namespace was run before our Absinthe instrumentation, which would then overwrite the namespace again.

We now use the new Span.set_namespace_if_nil helper in our Absinthe instrumentation, so that it only set the namespace from the instrumentation if it's not already set by the app (using the Span.set_namespace helper).

Closes #972

@tombruijn tombruijn self-assigned this Jun 17, 2025
@tombruijn tombruijn added the enhancement An improvement to an existing feature. label Jun 17, 2025
Ensure we use the `@nif` module attribute everywhere, so that when we
test something, to see if the Nif is called, we can actually verify it.
I ran into this testing the `set_attribute` function calls.
@tombruijn tombruijn changed the title Use nif module attribute in Span module Allow customization of trace namespace during the entire trace lifetime Jun 17, 2025
@tombruijn tombruijn requested a review from unflxw June 17, 2025 15:27
@tombruijn tombruijn marked this pull request as ready for review June 17, 2025 15:27
In issue #972 it was reported it was difficult to override the default
set namespace because our Absinthe instrumentation also sets the
namespace to 'graphql', which is called later than some plugs.
We have no 'only set namespace if not already set' logic, because the
namespace is always set upon root span creation, so it would be
difficult to determine when the namespace value was changed.

In the linked agent PR (private link) we added logic for the Node.js and
Python OpenTelemetry integrations to set namespaces with the
`appsignal.namespace` and `appsignal.namespace_if_nil` attributes.
appsignal/appsignal-agent#1274

This implementation also works for the Elixir integration, because they
all use the Span API.

If the `appsignal.namespace` attribute is set, it will use the value as
the namespace, overwriting the (default) namespace.
If only the `appsignal.namespace_if_nil` attribute is set, it will
use the value as the namespace, overwriting the default namespace.
If both are set, it will use the `appsignal.namespace` attribute value.

Since this uses the internal hoisting mechanism, it means the namespace
doesn't need to be set on the root span, but any span in the trace will
work. Not sure if we want to update the docs on this, or just keep
recommending to use the `Span.set_namespace` helper on the root span.

The Absinthe instrumentation (and potentially others) need to be updated
to use the `set_namespace_if_nil` helper so that it is easier to
overwrite the namespace for our users.
Previously, it was not possible to customize the namespace of an
Absinthe trace as detailed in issue #972.
This was because the plug that set the namespace was run before our
Absinthe instrumentation, which would then overwrite the namespace
again.

We now use the new `Span.set_namespace_if_nil` helper in our Absinthe
instrumentation, so that it only set the namespace from the
instrumentation if it's not already set by the app (using the
`Span.set_namespace` helper).

Closes #972
@unflxw
Copy link
Contributor

unflxw commented Jun 18, 2025

Not sure if we want to update the docs on this, or just keep recommending to use the Span.set_namespace helper on the root span.

I'd leave it as-is for consistency. It's probably confusing to learn that some trace-level attributes must be set on the root span, but not others.

We don't need to explicitly return span from these helpers. The
`set_attribute` helper already does that.
@tombruijn tombruijn merged commit 5eef756 into main Jun 18, 2025
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement to an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple absinthe namespaces

2 participants