Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Should the baggage propagator clear pre-existing baggage? #1526

Closed
MrAlias opened this issue Mar 10, 2021 · 1 comment · Fixed by #1575
Closed

Should the baggage propagator clear pre-existing baggage? #1526

MrAlias opened this issue Mar 10, 2021 · 1 comment · Fixed by #1575
Labels
question Question for discussion spec:baggage Related to the specification/baggage directory

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Mar 10, 2021

Based on the 1.0.1 version of the Baggage specification:

If the propagator is unable to parse the incoming baggage, extract MUST return a Context with no baggage entries in it.
If the incoming baggage is present, but contains no entries, extract MUST return a Context with no baggage entries in it.

The TextMap Extract specification states that the extract method is passed a context and returns a derived context.

Based on the existing specification if a context contains valid Baggage values and is passed to the baggage propagator to extract an unparssable or empty baggage into those existing values will be cleared. Is this intended?

@MrAlias MrAlias added the spec:baggage Related to the specification/baggage directory label Mar 10, 2021
@MrAlias MrAlias changed the title Should the baggage propagator clear pre-existing baggage on a failed extract? Should the baggage propagator clear pre-existing baggage? Mar 10, 2021
@arminru arminru added the question Question for discussion label Mar 11, 2021
@MrAlias
Copy link
Contributor Author

MrAlias commented Mar 23, 2021

From the specification SIG meeting the take-away is that if an extraction fails it needs to not touch what is in the existing context. This is specified here:

If a value can not be parsed from the carrier, for a cross-cutting concern, the implementation MUST NOT throw an exception and MUST NOT store a new value in the Context, in order to preserve any previously existing valid value.

I'm going to open a PR to correct this language.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
question Question for discussion spec:baggage Related to the specification/baggage directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants