implement getAll function in TextMap Extract

[ycchuang99]

Closes: #1567

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.92%. Comparing base (e2f24eb) to head (26112d1).
Report is 4 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1570      +/-   ##
============================================
+ Coverage     70.89%   70.92%   +0.03%     
- Complexity     2756     2765       +9     
============================================
  Files           407      407              
  Lines          8328     8348      +20     
============================================
+ Hits           5904     5921      +17     
- Misses         2424     2427       +3     

Flag	Coverage Δ
8.1	70.61% <100.00%> (+0.03%)	⬆️
8.2	70.81% <100.00%> (+0.11%)	⬆️
8.3	70.85% <100.00%> (-0.03%)	⬇️
8.4	70.85% <100.00%> (+0.14%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...rc/Context/Propagation/ArrayAccessGetterSetter.php	94.36% <100.00%> (+1.50%)	⬆️
...ation/SanitizeCombinedHeadersPropagationGetter.php	56.00% <100.00%> (+56.00%)	⬆️

... and 17 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2f24eb...26112d1. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[brettmc]

Can you think about this part of the spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#getall

For many language implementations, the GetAll function will be added after the stable release of Getter. For these languages, requiring implementations of Getter to include GetAll constitutes a breaking change since instrumentation which previously functioned would fail. Language implementations should be cognizant of this, and add GetAll in a way that retains backwards compatibility. For example, by providing a default GetAll implementation based on Get, or by creating an extended Getter type.

It's usually helpful to check Java's implementation for inspiration :)

[ycchuang99]

Hi @brettmc , would you give me some advice.

I'm currently working on adding a getAll() method via ExtendedPropagationGetterInterface, but I've run into an issue with OpenTelemetry\Context\Propagation\SanitizeCombinedHeadersPropagationGetter.

This class uses PropagationGetterInterface in its constructor, and changing it to ExtendedPropagationGetterInterface could introduce a breaking change. At the same time, I can't guarantee that the provided instance will implement getAll().

final class SanitizeCombinedHeadersPropagationGetter implements PropagationGetterInterface
{
    public function __construct(private readonly PropagationGetterInterface $getter)
    {
    }
}

Would you recommend leaving SanitizeCombinedHeadersPropagationGetter as is, or adding a fallback in getAll() to return an empty array if the constructor argument doesn't implement ExtendedPropagationGetterInterface?

Or do you have any other suggestions?

[brettmc]

This class uses PropagationGetterInterface in its constructor, and changing it to ExtendedPropagationGetterInterface could introduce a breaking change.

Excellent question. I think the safest way is to continue to accept a PropagationGetterInterface, have the class implement ExtendedPropagationGetterInterface, and put a check in the getAll method: "if $this->getter implements ExtendedPropagationGetterInterface, call it, else return empty array`. WDYT?

[ycchuang99]

Thanks! I think that makes sense, I’ll do it this way.