Feature: allow querier and query frontend targets to run on same process #4301
Conversation
trevorwhitney
changed the title
| assert.Equal(t, "test handler", recorder.Body.String()) | ||
| }) | ||
|
|
||
| t.Run("wrap external handler with auth middleware", func(t *testing.T) { |
There was a problem hiding this comment.
This took me a few minutes to understand, the test name implied something was being done but after looking around I was able to see that every test actually has the auth middleware.
The other part of this I'm a little worried about is the use of a global variable, this feels a little brittle to me over time.
What if instead the testContext function took a config and a middleware.Interface which you could then provide a specific version of for this test which does what the current noop middleware does but it would be scoped within this test.
All the other tests could then just call testContext(config, nil) or you could make a separate testContextWithMiddleware(config, middleware.Interface) and have testContext call that with a nil middleware to save refactoring all your tests
WDYT?
There was a problem hiding this comment.
I like it! IMO a test should always be easier to understand than the code it's testing, so I love simplifications and clarifications.
|
Looks good @trevorwhitney! A couple suggestions, and the linter doesn't like a typo and I think we should be good to go! |
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
Co-authored-by: Ed Welch <edward.welch@grafana.com>
Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
trevorwhitney
force-pushed
the
4218-internal-querier
branch
from
f2bf318 to
98266b8
Compare
What this PR does / why we need it:
This PR allows both the Query and QueryFrontend targets to be enabled in the same loki process, as well as adding QueryFrontend to the "all" target. It follows a similar pattern as was done in cortex, where a query worker service is enabled with an internal router to handle query endpoints. Those endpoints are either a) exposed externally if the querier is running standalone, b) used internally by the query worker if the querier is running alongside the query frontend (since the query fronted will register those same routes externally).
Which issue(s) this PR fixes:
Fixes #4218
Special notes for your reviewer:
Checklist