Skip to content

Commit

Permalink
Fix(query-frontend): /label/<name>/values endpoint to use right set…
Browse files Browse the repository at this point in the history
… of middlewares (#6072)

* Fix(query-frontend): `/label/<name>/values` endpoint to use right middleware

Previously, only `/label/` and `/labels` endpoint were using correct tripperware middleware
on the query frontend(e.g: stats collector middleware)

This PR fixes it for `/label/<name>/values` endpoint as well.

This is useful to support `metrics.go` stats middleware for `label/{name}/values` endpoint as well.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>

* Remove `unhandledpath` test

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>

* Remove unused test helper

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
  • Loading branch information
kavirajk authored May 2, 2022
1 parent 00e210a commit bb2e952
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 36 deletions.
2 changes: 1 addition & 1 deletion pkg/querier/queryrange/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func getOperation(path string) string {
return QueryRangeOp
case strings.HasSuffix(path, "/series"):
return SeriesOp
case strings.HasSuffix(path, "/labels") || strings.HasSuffix(path, "/label"):
case strings.HasSuffix(path, "/labels") || strings.HasSuffix(path, "/label") || strings.HasSuffix(path, "/values"):
return LabelNamesOp
case strings.HasSuffix(path, "/v1/query"):
return InstantQueryOp
Expand Down
107 changes: 72 additions & 35 deletions pkg/querier/queryrange/roundtrip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/promql"
"github.com/prometheus/prometheus/promql/parser"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/weaveworks/common/httpgrpc"
"github.com/weaveworks/common/middleware"
Expand Down Expand Up @@ -377,30 +378,6 @@ func TestLogNoRegex(t *testing.T) {
require.NoError(t, err)
}

func TestUnhandledPath(t *testing.T) {
tpw, stopper, err := NewTripperware(testConfig, util_log.Logger, fakeLimits{}, config.SchemaConfig{}, nil, nil)
if stopper != nil {
defer stopper.Stop()
}
require.NoError(t, err)
rt, err := newfakeRoundTripper()
require.NoError(t, err)
defer rt.Close()

ctx := user.InjectOrgID(context.Background(), "1")
req, err := http.NewRequest(http.MethodGet, "/loki/api/v1/labels/foo/values", nil)
require.NoError(t, err)
req = req.WithContext(ctx)
err = user.InjectOrgIDIntoHTTPRequest(ctx, req)
require.NoError(t, err)

count, h := errorResult()
rt.setHandler(h)
_, err = tpw(rt).RoundTrip(req)
require.Equal(t, 1, *count)
require.NoError(t, err)
}

func TestRegexpParamsSupport(t *testing.T) {
l := WithSplitByLimits(fakeLimits{maxSeries: 1, maxQueryParallelism: 2}, 4*time.Hour)
tpw, stopper, err := NewTripperware(testConfig, util_log.Logger, l, config.SchemaConfig{}, nil, nil)
Expand Down Expand Up @@ -547,6 +524,77 @@ func TestEntriesLimitWithZeroTripperware(t *testing.T) {
require.NoError(t, err)
}

func Test_getOperation(t *testing.T) {
cases := []struct {
name string
path string
expectedOp string
}{
{
name: "instant_query",
path: "/loki/api/v1/query",
expectedOp: InstantQueryOp,
},
{
name: "range_query_prom",
path: "/prom/query",
expectedOp: QueryRangeOp,
},
{
name: "range_query",
path: "/loki/api/v1/query_range",
expectedOp: QueryRangeOp,
},
{
name: "series_query",
path: "/loki/api/v1/series",
expectedOp: SeriesOp,
},
{
name: "series_query_prom",
path: "/prom/series",
expectedOp: SeriesOp,
},
{
name: "labels_query",
path: "/loki/api/v1/labels",
expectedOp: LabelNamesOp,
},
{
name: "labels_query_prom",
path: "/prom/labels",
expectedOp: LabelNamesOp,
},
{
name: "label_query",
path: "/loki/api/v1/label",
expectedOp: LabelNamesOp,
},
{
name: "labels_query_prom",
path: "/prom/label",
expectedOp: LabelNamesOp,
},
{
name: "label_values_query",
path: "/loki/api/v1/label/__name__/values",
expectedOp: LabelNamesOp,
},
{
name: "label_values_query_prom",
path: "/prom/label/__name__/values",
expectedOp: LabelNamesOp,
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := getOperation(tc.path)
assert.Equal(t, tc.expectedOp, got)
})
}
}

type fakeLimits struct {
maxQueryLength time.Duration
maxQueryParallelism int
Expand Down Expand Up @@ -605,17 +653,6 @@ func counter() (*int, http.Handler) {
})
}

func errorResult() (*int, http.Handler) {
count := 0
var lock sync.Mutex
return &count, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
lock.Lock()
defer lock.Unlock()
count++
w.WriteHeader(http.StatusInternalServerError)
})
}

func promqlResult(v parser.Value) (*int, http.Handler) {
count := 0
var lock sync.Mutex
Expand Down

0 comments on commit bb2e952

Please sign in to comment.