add signal->span-attrs-fn option to opentelemetry handler

[benalbrecht]

this PR adds a :signal->span-attrs-fn option to the opentelemetry handler for adding custom signal attributes to the corresponding opentelemetry span.

the existing signal->attrs fn seems to be intended for log records and i'm not sure whether the result would be a good default for spans as well, so i've just made it public to have it available as an option.

i've made the helper functions public as well to aid in creating a custom signal->span-attrs-fn

[ptaoussanis]

Thanks for this Benjamin!

I'd like to understand your use-case a little better. Could you please give me some idea of what one would typically use signal->span-attrs-fn for?

What would your fn look like?

Also, just checking if you're familiar with the :otel/attrs feature (currently undocumented).

[benalbrecht]

my use-case is adding attributes to otel traces i'm sending to honeycomb.io.

my signal->span-attrs-fn would look similar to signal->attrs

yes, i've noticed :otel/attrs but they aren't being added to trace spans right now 😄 just adding :otel/attrs might also work for me, since they could be populated by middleware.

[ptaoussanis]

my use-case is adding attributes to otel traces i'm sending to honeycomb.io.
my signal->span-attrs-fn would look similar to signal->attrs

Could you be a little more specific? E.g. are the attributes common to all signals, or signal specific?

Can you give me an example of just a few of the attributes you plan to add? It'd be helpful to get an understanding of what problem you're actually trying to solve, and therefore what our best options might be.

i've noticed :otel/attrs but they aren't being added to trace spans right now 😄 just adding :otel/attrs might also work for me, since they could be populated by middleware.

Just to confirm: you want your custom attributes on the trace and not the log record?

What if the handler supported the following?:

• :otel/log-attrs -> handled as here.
• :otel/trace-attrs -> handled the same way, but for trace attributes
• :otel/attrs -> attached to both trace and log

Or if we stick to your idea of an signal->span-attrs-fn option, how would you feel if it took a map like this?

In principle I'd prefer to avoid making low-levels fns public unless really necessary. We may still want to make adjustments later.

[benalbrecht]

Could you be a little more specific? E.g. are the attributes common to all signals, or signal specific?
Can you give me an example of just a few of the attributes you plan to add? It'd be helpful to get an understanding of what problem you're actually trying to solve, and therefore what our best options might be.

Some attributes are common to all signals, some are singal specific.
An example:

• ring handler traces get attributes like http.request.method or url.path
• database query traces get attributes like db.namespace or db.query.text
• both get attributes like thread.id or service.version

What if the handler supported the following?:
:otel/log-attrs -> handled as here.
:otel/trace-attrs -> handled the same way, but for trace attributes
:otel/attrs -> attached to both trace and log

i'm not using the otel log handler, so i don't have a preference here 😄 a single :otel/attrs key that is applied to both would work for me.

Or if we stick to your idea of an signal->span-attrs-fn option, how would you feel if it took a map like this?
In principle I'd prefer to avoid making low-levels fns public unless really necessary. We may still want to make adjustments later.

using something like :otel/attrs seems to be the better approach. i don't see anything that this version of signal->span-attrs-fn could do that can't be handled with middleware as well.

[ptaoussanis]

Great, thanks for the extra info 👍 In principle how does this look to you?

Also, we could hypothetically add the same custom attributes here, for Span Events.

Any thoughts on that?

[benalbrecht]

looks good, thanks!
it would make sense to add the attributes to span events as well, I totally missed the extra handling for those.

[ptaoussanis]

it would make sense to add the attributes to span events as well,

Great, makes sense. I've updated the commit.

If you have no other suggested improvements, I'll document the :otel/ options tomorrow and clean up the commit 👍

[benalbrecht]

i'm closing this one in the meantime :) looking forward to the next release!

[ptaoussanis]

I just updated v1.0.0-SNAPSHOT on Clojars.

If you get an opportunity, would you please test this and confirm that it's working as expected?

In that case, I expect to be able to release RC1 before the end of this month (Oct).

Cheers!