Closed NLincoln closed 7 months ago
Thanks @NLincoln ! Noting here for reference per our internal slack thread, we're going to chat about this a bit more and circle back!
Hey @NLincoln - I think it might be worth updating this PR to use a similar Options pattern we used in beeline-go to provide extra options when setting a field. In this case, incase of a new AddFieldIfNotSet
function, we could use the same AddField
function and extend with a IfNotSet
option. What do you think?
@MikeGoldsmith I'd worry that since AddField is called relatively often, adding the options pattern might cause less-than-good performance. Since a libhoney Event is conceptually a wrapper around a map, I figured having dedicated methods that make sense for maps would be good
In beeline-go, trace-level fields take precedence over span-level fields. This is because we add the trace-level fields to the span at the very end of it's life, just before it's sent.
While we could move the point at which trace-level fields are added to spans, that feels... like too much backwards incompatibility, and instead I'd like to propose having trace-level fields added as "defaults" - so we won't override the fields that are added to the specific span.