route: Allow per-request custom context injection #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**Motivation** Currently, Prometheus's web API knows of two different contexts: - The main Prometheus server context which is passed into the web API and through to the query layer. - The per-request context which is generated by the route.Router. This is currently only used for storing and retrieving URL-path parameters, and it's not possible to influence this context at the top of the chain. Since merging of contexts is not a concept, we ultimately want the router to support a per-request context which: a) can be derived from the main server context, b) can be decorated with custom values/timeouts/etc. per-request The concrete motivation for b) is for Weaveworks Frankenstein to be able to extract multi-tenancy authentication headers and store user information in the context. This will then be passed on into the query layers and decide what data a user gets to see. Next step: update Prometheus to make use of this, get rid of the context in the web API itself. This will make the API package reusable by Frankenstein.
juliusv
force-pushed
the
context-injection
branch
from
9b69d62 to
ee21c31
Compare
juliusv
added a commit
to prometheus/prometheus
that referenced
this pull request
Sep 21, 2016
This is based on the common/route changes in prometheus/common#61.
fabxc
reviewed
Sep 22, 2016
| } | ||
|
|
||
| // New returns a new Router. | ||
| func New(ctxFn contextFn) *Router { |
There was a problem hiding this comment.
Do you envision a use case where a context function here would be useful instead of a base context?
Seems like values passed into the context would be static to a base context anyway.
Seems like only a timeout for all requests applied to a new context would be sane.
Could be more explicit to configure as a directly. But both approaches seem equally fine.
There was a problem hiding this comment.
Yes, I need a context function exactly for the Frankenstein case: to pull out per-request multi-tenancy information from the HTTP request headers and stuff that user info into the context.
Or did I misunderstand your question?
|
@fabxc Any chance to get this and the related prometheus/prometheus#2012 merged? |
|
Yea, seems fine in general and doesn't break anything 👍 go ahead on both. |
|
Thanks! |
gouthamve
pushed a commit
to gouthamve/common
that referenced
this pull request
Jul 22, 2018
03cc598 Don't lint generated protobuf code. 2b55c2d Merge pull request prometheus#66 from weaveworks/reduce-test-timeout d4e163c Make timeout a flag 49a8609 Reduce test timeout 8fa15cb Merge pull request prometheus#63 from weaveworks/test-defaults b783528 Tweak test script so it can be run on a mca a3b18bf Merge pull request prometheus#65 from weaveworks/fix-integration-tests ecb5602 Fix integration tests f9dcbf6 ... without tab (clearly not my day) a6215c3 Add break I forgot 0e6832d Remove incorrectly added tab eb26c68 Merge pull request prometheus#64 from weaveworks/remove-test-package-linting f088e83 Review feedback 2c6e83e Remove test package linting 2b3a1bb Merge pull request prometheus#62 from weaveworks/revert-61-test-defaults 8c3883a Revert "Make no-go-get the default, and don't assume -tags netgo" e75c226 Fix bug in GC of firewall rules. e49754e Merge pull request prometheus#51 from weaveworks/gc-firewall-rules 191f487 Add flag to enale/disable firewall rules' GC. 567905c Add GC of firewall rules for weave-net-tests to scheduler. 03119e1 Fix typo in GC of firewall rules. bbe3844 Fix regular expression for firewall rules. c5c23ce Pre-change refactoring: splitted gc_project function into smaller methods for better readability. ed5529f GC firewall rules ed8e757 Merge pull request prometheus#61 from weaveworks/test-defaults 57856e6 Merge pull request prometheus#56 from weaveworks/remove-wcloud dd5f3e6 Add -p flag to test, run test in parallel 62f6f94 Make no-go-get the default, and don't assume -tags netgo 8946588 Merge pull request prometheus#60 from weaveworks/2647-gc-weave-net-tests 4085df9 Scheduler now also garbage-collects VMs from weave-net-tests. 4b7d5c6 Merge pull request prometheus#59 from weaveworks/57-fix-lint-properly b7f0e69 Merge pull request prometheus#58 from weaveworks/fix-lint 794702c Pin version of shfmt ab1b11d Fix lint d1a5e46 Remove wcloud cli tool 81d80f3 Merge pull request prometheus#55 from weaveworks/lint-tf 05ad5f2 Review feedback 4c0d046 Use hclfmt to lint terraform. git-subtree-dir: tools git-subtree-split: 03cc5989769d93aa03f8aed3784ddfdb1fffc1c6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Currently, Prometheus's web API knows of two different contexts:
and through to the query layer.
is currently only used for storing and retrieving URL-path parameters,
and it's not possible to influence this context at the top of the
chain.
Since merging of contexts is not a concept, we ultimately want the
router to support a per-request context which:
a) can be derived from the main server context,
b) can be decorated with custom values/timeouts/etc. per-request
The concrete motivation for b) is for Weaveworks Frankenstein to be able
to extract multi-tenancy authentication headers and store user
information in the context. This will then be passed on into the query
layers and decide what data a user gets to see.
Next step: update Prometheus to make use of this, get rid of the context
in the web API itself. This will make the API package reusable by
Frankenstein.