Skip to content

Deprecate TextMapGetter keys method#8531

Draft
arnabnandy7 wants to merge 4 commits into
open-telemetry:mainfrom
arnabnandy7:deprecate/textmap-keys
Draft

Deprecate TextMapGetter keys method#8531
arnabnandy7 wants to merge 4 commits into
open-telemetry:mainfrom
arnabnandy7:deprecate/textmap-keys

Conversation

@arnabnandy7

@arnabnandy7 arnabnandy7 commented Jun 24, 2026

Copy link
Copy Markdown

Description

Deprecates TextMapGetter.keys() and gives it a default empty implementation so TextMapGetter can be used as a functional interface.

This lets callers implement TextMapGetter with a lambda when only get() behavior is needed, while preserving existing carrier-key enumeration behavior for implementations that override keys().

Changes

  • Marked TextMapGetter as @FunctionalInterface.
  • Deprecated TextMapGetter.keys().
  • Added a default keys() implementation returning an empty iterable.
  • Added a regression test covering lambda-based TextMapGetter implementation.
  • Suppressed intentional deprecated keys() usage in deprecated Jaeger and OT trace baggage extraction paths.
  • Updated the opentelemetry-context API diff.

Testing

  • ./gradlew :context:test --tests io.opentelemetry.context.propagation.TextMapGetterTest
  • ./gradlew :extensions:trace-propagators:compileJava
  • ./gradlew :context:jApiCmp
  • ./gradlew :context:spotlessApply :extensions:trace-propagators:spotlessApply
  • git diff --check

Fixes #8470 and dependent on spec:5177

@arnabnandy7
arnabnandy7 requested a review from a team as a code owner June 24, 2026 17:57
@otelbot otelbot Bot added the api-change Changes to public API surface area label Jun 24, 2026
@otelbot

otelbot Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

⚠️ API changes detected — additional maintainer review required

@jack-berg @jkwatson

This PR modifies the public API surface area of the following module(s):

  • opentelemetry-context

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.63%. Comparing base (09d6c17) to head (882b6fd).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8531      +/-   ##
============================================
- Coverage     91.64%   91.63%   -0.02%     
+ Complexity    10348    10347       -1     
============================================
  Files          1013     1013              
  Lines         27380    27381       +1     
  Branches       3218     3218              
============================================
- Hits          25092    25090       -2     
- Misses         1558     1559       +1     
- Partials        730      732       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jack-berg jack-berg added the blocked:spec blocked on open or unresolved spec label Jun 24, 2026
@jack-berg

Copy link
Copy Markdown
Member

Added blocked:spec label. We can revisit if/when spec stabilizes. That might be a while. I suggest we convert to draft / close until then.

@arnabnandy7
arnabnandy7 marked this pull request as draft June 25, 2026 03:57
@arnabnandy7

Copy link
Copy Markdown
Author

I suggest we convert to draft / close until then.

@jack-berg I've converted it to a draft.

@cfredri4

cfredri4 commented Jul 3, 2026

Copy link
Copy Markdown

From the spec discussion:

open-telemetry/opentelemetry-specification#5150 (comment)

You can also consider making Keys an optional functionality (in OTel Java) that the Getter does not have to implement (or return some default implementation).

It is worth mentioning that the spec DOES NOT require implementing Keys.

One of the ways to implement it is Getter class with methods Get, Keys, and GetAll as described below. Languages may decide on alternative implementations and expose corresponding methods as delegates or other ways.

So until the spec discussion is settled, can the TextMapGetter.keys() get a default empty implementation, but not be deprecated?

@jkwatson jkwatson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no opinion one way or another on this one. I understand the appeal of being able to make this a functional interface, but it seems not good to have an empty iterable be the default, as that could end up being confusing for consumers. I'm fine with this either way.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 18, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-20 21:22:48 UTC.

  • Waiting on: Author
  • Next step: Move out of draft to request review.

This automated status or its linked feedback items may be incorrect. If something looks wrong, report it with the result you expected.

@cfredri4

Copy link
Copy Markdown

Given that it's not actually used anywhere by OpenTelemetry itself, should the default implementation throw an UnsupportedOperationException?

@arnabnandy7

Copy link
Copy Markdown
Author

Given that it's not actually used anywhere by OpenTelemetry itself, should the default implementation throw an UnsupportedOperationException?

I think returning an empty iterable is safer than throwing UnsupportedOperationException.

Although most propagators do not use keys(), the deprecated Jaeger and OT Trace propagators still call it for baggage extraction. If a lambda-based TextMapGetter were passed to either, throwing would cause extraction to fail at runtime. An empty iterable instead means that key enumeration is unavailable, so extraction can continue without enumerated baggage.

Given the ongoing spec discussion, I also think we should keep the default implementation but avoid deprecating keys() for now. That would make TextMapGetter a functional interface without prematurely deciding the broader deprecation question.

@cfredri4

Copy link
Copy Markdown

This would only happen on custom lambda propagators, and I would rather my custom propagator explode loudly with an exception than having to spend hours or days trying to debug why it doesn't propagate baggage. 🙂

Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
@arnabnandy7

Copy link
Copy Markdown
Author

@cfredri4 I’ve updated the default keys() implementation to throw UnsupportedOperationException, so custom lambda getters fail clearly when used with a propagator that requires key enumeration.

I also removed the deprecation for now, since that broader decision is still being discussed in the specification. TextMapGetter remains a functional interface, while implementations that need keys() must explicitly provide it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change Changes to public API surface area blocked:spec blocked on open or unresolved spec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate TextMapGetter.keys()

4 participants