Skip to content
This repository was archived by the owner on Apr 23, 2021. It is now read-only.

Required context parameter considerations #8

Closed
kmahar opened this issue Jul 8, 2020 · 3 comments
Closed

Required context parameter considerations #8

kmahar opened this issue Jul 8, 2020 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@kmahar
Copy link

kmahar commented Jul 8, 2020

One of the proposed API guidelines is that the context parameter be required, to avoid the context being inadvertently dropped.

There are a couple of considerations to think about here:

  1. What a should a library do if they wish to integrate tracing but are not willing or able to make a breaking change yet? One solution would be to maintain both new and old versions of the method, one of which takes in the context, and deprecate the old context-free one to push users toward providing context. (Alternatively, the context parameter could be optional, but then we still have the issue of an inadvertently dropped context.)

  2. Some libraries may have users/use cases where tracing isn't needed or wanted. How can library developers support users who don't already have a context around to pass in? @ktoso suggested this could be handled by providing some default "no context" value users can pass.

@slashmo
Copy link
Owner

slashmo commented Jul 8, 2020

Thanks for bringing this up, @kmahar!

I think we should avoid defaulting the BaggageContext argument to anything, be it nil or @ktoso's suggested .none in order to avoid dropping traces.

I also get the point about this potentially being awkward for other use cases where tracing doesn't make as much sense. Here, in my opinion, the decision should be up to a specific library whether it's worth the trade-off in order to avoid dropping traces for the server-side use of their API.

@kmahar
Copy link
Author

kmahar commented Jul 14, 2020

That all makes sense to me! On a related note I am +1 to the proposed parameter ordering after reading through all of that, it feels intuitive and matches up with how I would have thought to add context parameters to the MongoDB driver APIs.

@slashmo
Copy link
Owner

slashmo commented Aug 6, 2020

@kmahar Not closely related to this issue, but version 0.2.0 of this package might be of interest to you. It adds BaggageContextCarrier, which allows you to pass framework contexts (holding a baggage) to user-facing APIs that need a BaggageContext.

@ktoso ktoso self-assigned this Sep 14, 2020
@ktoso ktoso added the documentation Improvements or additions to documentation label Sep 14, 2020
@slashmo slashmo closed this as completed Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants