feat: add BaggageSpanProcessor to automatically copy baggage entries as span attributes #17
Conversation
…as span attributes - Introduced BaggageSpanProcessor that integrates baggage entries into span attributes upon span creation. - Updated CHANGELOG for version 0.9.2 to reflect this new feature. - Modified CONTRIBUTING.md to update repository links. - Added unit tests for BaggageSpanProcessor to ensure correct functionality and handling of various baggage scenarios.
|
@danexello This is very good! I appreciate the coverage of many test cases and the doc was very good. I have some updates coming. Minor things like removing the reference to inferior languages like Python. ;) (and not being tied to changes in pythons baggage implementation via doc) Thank you very much. |
|
Questions I have:
|
|
@danexello Please merge in my pr17-improvements branch |
|
@michaelbushe thanks for the feedback! and sounds good. i can get to this tomorrow |
|
@michaelbushe i "merged" in the changes indicated in you One thing -- I'm not sure if we actually want this proposed change. I think we want to prefer the Baggage on the current context, as it will inherit any Baggage present on the Parent, which I think addresses your comment:
I think we still want |
|
|
I started playing around with getting a perf test up, but ran into problems getting |
|
@danexello Apologies for the delay. I just came back to it wondering were you were and saw that I'm the blocker now. Yes, I agree with your comments. Let's get this in. We can worry about performance later for this, it's an opt-in. |
|
The performance test is acting oddly. It's failing intermittently. When I run it locally via ./test/coverage.sh these are the three consistent failures. I'm still working on this. I got of the three to pass, perhaps by setting concurrency to 1 (from 10) in coverage.sh . Or perhaps from fixing a bug where the test replaces the print for OTelLog to capture log output but OTel.initialize via OTelEnv.initializeLogging() was not respecting it. Let me look at this some more. It doesn't happen on main so though the feature is good, we want to ensure it's quality before merging. Here are the tests that fail locally. 00:12 +179 ~2: test/unit/context/context_propagator_test.dart: Context Propagation composite propagator combines multiple propagators package:matcher expect 00:26 +397 ~12 -1: test/unit/util/otel_log_test.dart: OTelLog Tests OTelLog functions respect isXxx() convenience methods Perhaps the third failure is this: 00:35 +514 ~12 -2: test/unit/export/console_exporter_test.dart: ConsoleExporter Unit Tests Debug logging shows processor notifications package:matcher expect |
|
This just needs a rebase to catch up with the latest main and will work and will (now) publish the coverage to GitHub pages. baggage_propagator had 100% test coverage - nice! Thank you. |
2 participants
Addresses #15