Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Have a flag to report failed Extraction #684

Open
carlosalberto opened this issue Jul 1, 2020 · 1 comment
Open

Have a flag to report failed Extraction #684

carlosalberto opened this issue Jul 1, 2020 · 1 comment
Assignees
Labels
area:api Cross language API specification issue release:after-ga Not required before GA release, and not going to work on before GA spec:context Related to the specification/context directory

Comments

@carlosalberto
Copy link
Contributor

Consider adding a second return value for Extract(), that reports whether there was an invalid payload in the carrier or it simply was empty.

From #671 :

it is impossible to distinguish "error" from "no context at all". I could imagine one could use that info to e.g. stop trying more propagators if a propagator found its headers but they were invalid. I think there should be some way for propagators to indicate this. E.g. add a second return value (this is the spec, so it's up to languages how to implement this second return value).

@carlosalberto carlosalberto added area:api Cross language API specification issue spec:context Related to the specification/context directory labels Jul 1, 2020
@carlosalberto carlosalberto self-assigned this Jul 1, 2020
@Oberon00
Copy link
Member

Oberon00 commented Jul 1, 2020

Quoting myself from #671 (comment):

I would suggest updating Extract() as follows:

Return values:

  • [as before:] A new Context derived from the Context passed as argument,
    containing the extracted value, which can be a SpanContext,
    CorrelationContext or another cross-cutting concern context.
  • [new:] A status indicator object. It must be possible to programatically detect at least: "context successfully extracted" and "no context found" and to distinguish them from any other errors the propagator encountered (e.g. header was present but value was invalid).

Some languages might implement this with an out parameter, or a class containing both values, whatever is more idiomatic.

@carlosalberto carlosalberto added the release:required-for-ga Must be resolved before GA release, or nice to have before GA label Jul 10, 2020
@carlosalberto carlosalberto added priority:p2 Medium priority level priority:p3 Lowest priority level and removed priority:p2 Medium priority level labels Jul 24, 2020
@arminru arminru added release:after-ga Not required before GA release, and not going to work on before GA and removed priority:p3 Lowest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue release:after-ga Not required before GA release, and not going to work on before GA spec:context Related to the specification/context directory
Projects
None yet
Development

No branches or pull requests

3 participants