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

Context #35

Closed
tsloughter opened this issue Nov 24, 2017 · 8 comments
Closed

Context #35

tsloughter opened this issue Nov 24, 2017 · 8 comments

Comments

@tsloughter
Copy link
Member

Currently there is only the pdict as a way to track the context of a request (requestid, trace and span ids, etc) through middlewares to handlers.

I see two possibilities for expanding this, one which would be a major change to the elli interface and require a major version bump and the other being a simpler extension but feels sort of hacky.

The major change option would be to pass a context that is modifiable, like a map, by the middlewares. I've been starting on a lib for this: https://github.com/tsloughter/ctx

Alternatively the req could have a map or ctx added to it.

The advantage of using a context map as the request instead of req is it can be used and extended by other Erlang apps and code without depending on the Elli req record.

@yurrriq
Copy link
Member

yurrriq commented Nov 25, 2017

I support the idea of a modifiable context. Using ctx or similar as a layer of abstraction seems sound too, so it would be easier to patch for different scenarios, e.g. using some underlying structure other than maps.

I'm very fond of the way Ring works in Clojure, and their requests and responses are basically extensible maps as you describe above, with certain expectations outlined in the spec. As needed we could probably use Dialyzer to "enforce" those expectations on a per-library basis too.

@tsloughter
Copy link
Member Author

ctx is being used by https://github.com/tsloughter/grpcbox and https://github.com/census-instrumentation/opencensus-erlang

But I don't know how we would do this without breaking backwards compatibility and being forced on those who just want to keep using the req record.

@tsloughter
Copy link
Member Author

Hm, was just thinking I'd do it as an alternative to elli_middleware, so a elli_ctx_middleware. Only issue is this would mean each handler would have to support both :(. I suppose it could do some sort of check if it should pass req or ctx to the handler...

@tsloughter
Copy link
Member Author

Or not exactly... init can only return {ok, standard | handover} right now and I think it would be the right place to allow configuring the deadlines of a context for that request.

@yurrriq
Copy link
Member

yurrriq commented Mar 6, 2018

could we use a preprocessor directive to determine if we should use #req{} or a ctx map? I think most of the #req{} accessors are abstracted in elli_req already.

@yurrriq
Copy link
Member

yurrriq commented Mar 6, 2018

I'm nearly done implementing that in a branch.

@yurrriq
Copy link
Member

yurrriq commented Mar 6, 2018

See #44 for progress.

@yurrriq
Copy link
Member

yurrriq commented Feb 2, 2021

Closing in favor of #70

@yurrriq yurrriq closed this as completed Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants