feat: Add baggage span processor

[MikeGoldsmith]

Summary of changes

Adds the baggage span processor. I've added under new root dir processors, not sure if that's the best place for it or it should go somewhere else. Happy to take direction.

• Closes Request for new component: Baggage Span Processor #934

[robbkidd]

Here's the first take on the long-awaited span processor to take current Baggage key/values and add them as attributes to spans as they start.

Not sure of the module namespace in the current state. This is a span processor, so should we nest it more deeply under the OpenTelemetry::Trace namespace? Is "Span" in its name sufficient? What do y'all think?

We can work on adding unit tests to this. The first edition of those would look a lot like the RSpec specs I wrote over here.

[arielvalentin]

Something that I want to confirm, this is still going to require that the user annotate the active/current span in addition to using baggage.

It's also going to transfer these attributes across service boundaries. Is that the effect that you want?

If so, I think it is important to highlight that this will potentially cause message and HTTP headers to impact performance since there isn't anything built in to say prevent the headers from growing unwieldy. (No per-header compression e.g.)

In addition to that baggage usage means library instrumentation will include all items in outbound headers to 3rd parties, which will raise concerns for privacy.

I realize that these concerns I am highlighting are not specific to this implementation but issues with Baggage; however, I think it would be important to highlight these concerns so that Baggage users are not caught off guard.

          span.add_attributes('user_id' => user.id, 'cart_id' => cart.id)
          ctx = OpenTelemetry::Baggage.build do |builder|

            builder.set_value('user_id', user.id)
            builder.set_value('cart_id', cart.id)
          end

          tracer.in_span('checkout') do |span|
              # yada yada yada
          end 

[MikeGoldsmith]

Something that I want to confirm, this is still going to require that the user annotate the active/current span in addition to using baggage.

It's also going to transfer these attributes across service boundaries. Is that the effect that you want?

If so, I think it is important to highlight that this will potentially cause message and HTTP headers to impact performance since there isn't anything built in to say prevent the headers from growing unwieldy. (No per-header compression e.g.)

In addition to that baggage usage means library instrumentation will include all items in outbound headers to 3rd parties, which will raise concerns for privacy.

I realize that these concerns I am highlighting are not specific to this implementation but issues with Baggage; however, I think it would be important to highlight these concerns so that Baggage users are not caught off guard.

          span.add_attributes('user_id' => user.id, 'cart_id' => cart.id)
          ctx = OpenTelemetry::Baggage.build do |builder|

            builder.set_value('user_id', user.id)
            builder.set_value('cart_id', cart.id)
          end

          tracer.in_span('checkout') do |span|
              # yada yada yada
          end 

Yes, it is known that baggage entries are transported to 3rd parties and you're correct this is a fault of baggage not this processor in particular. I haven't added a README yet, but will add warnings there, in the class implementation and any module functions to make it as clear as possible. I'll add a note regarding unbounded header size too.

[ericmustin]

Do any other languages have an implementation in contrib, or more broadly, do any other languages have non-spec processors that live in contrib?

(Fwiw I'm fine with this PR just kinda curious if y'all are setting precedent here, which, again, is fine if so)

[robbkidd]

Do any other languages have an implementation in contrib, or more broadly, do any other languages have non-spec processors that live in contrib?

We (Honeycomb) have started opening PRs to add BaggageSpanProcessor to contrib in other languages, so: technically "no" but also maybe "yes" soon.

[ericmustin]

Huzzah. Ty @robbkidd , Sounds good to me. Ping if this gets stuck in the mud happy to help w/approvals but otherwise will leave to the folks more tuned in with uh..the world (I'm on pto for a little while)

[robbkidd]

Well, I succeeded in adding a new set of jobs for Processors to the CI matrix! 🙌🏻

Now to wait for the matrix ... ⏳

[robbkidd]

Horrah! BaggageSpanProcessor tests pass. 🎉

[arielvalentin]

My only request for an actual change is to consolidate the CI jobs.

The other comments are optional changes/food for thought.

[arielvalentin]

💭 Since a span is essentially a struct as opposed to a service and you all are having to configure the SDK for some of tests below, I think it is enough to test this functionality using state-based test as opposed to using interaction-based testing.

[MikeGoldsmith]

I believe these tests were added to reflect a similar pattern used in other (instrumentation) packages. Is it worth replacing these or just a suggestion to reduce complexity?

[arielvalentin]

It was a suggestion but really, I leave the final decision up to you.

[MikeGoldsmith]

Added this follow-up enhancement issue to track adding a key predicate function to control what keys are copied to newly created span attributes.

• Baggage span processor - key predicate #956

[MikeGoldsmith]

My only request for an actual change is to consolidate the CI jobs.

The other comments are optional changes/food for thought.

Thanks for your feedback @arielvalentin. I've worked through and addressed your comments. Let me know if there's anything else.

[arielvalentin]

Thank you for your contribution!

[MikeGoldsmith]

@arielvalentin - I've pulled in the change jruby 9.4.6.0 update and updated the new processors workflow. Is there anything else you need / want before accepting this PR?