Skip to content

Commit

Permalink
Move Tenancy Handler To Server
Browse files Browse the repository at this point in the history 
Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
@mahadzaryab1
mahadzaryab1 committed Oct 27, 2024
1 parent a8feb37 commit a1beee7
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 191 deletions.
9 changes: 1 addition & 8 deletions cmd/query/app/apiv3/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/jaegertracing/jaeger/cmd/query/app/internal/api_v3"
"github.com/jaegertracing/jaeger/model"
_ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/storage/spanstore"
spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
)
Expand All @@ -42,15 +41,12 @@ type testGateway struct {
reader *spanstoremocks.Reader
url string
router *mux.Router
// used to set a tenancy header when executing requests
setupRequest func(*http.Request)
}

func (gw *testGateway) execRequest(t *testing.T, url string) ([]byte, int) {
req, err := http.NewRequest(http.MethodGet, gw.url+url, nil)
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")
gw.setupRequest(req)
response, err := http.DefaultClient.Do(req)
require.NoError(t, err)
body, err := io.ReadAll(response.Body)
Expand Down Expand Up @@ -101,11 +97,8 @@ func makeTestTrace() (*model.Trace, model.TraceID) {
func runGatewayTests(
t *testing.T,
basePath string,
tenancyOptions tenancy.Options,
setupRequest func(*http.Request),
) {
gw := setupHTTPGateway(t, basePath, tenancyOptions)
gw.setupRequest = setupRequest
gw := setupHTTPGateway(t, basePath)
t.Run("GetServices", gw.runGatewayGetServices)
t.Run("GetOperations", gw.runGatewayGetOperations)
t.Run("GetTrace", gw.runGatewayGetTrace)
Expand Down
7 changes: 1 addition & 6 deletions cmd/query/app/apiv3/http_gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/jaegertracing/jaeger/cmd/query/app/internal/api_v3"
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/storage/spanstore"
)

Expand All @@ -46,7 +45,6 @@ const (
// HTTPGateway exposes APIv3 HTTP endpoints.
type HTTPGateway struct {
QueryService *querysvc.QueryService
TenancyMgr *tenancy.Manager
Logger *zap.Logger
Tracer trace.TracerProvider
}
Expand All @@ -62,16 +60,13 @@ func (h *HTTPGateway) RegisterRoutes(router *mux.Router) {

// addRoute adds a new endpoint to the router with given path and handler function.
// This code is mostly copied from ../http_handler.
func (h *HTTPGateway) addRoute(
func (*HTTPGateway) addRoute(
router *mux.Router,
f func(http.ResponseWriter, *http.Request),
route string,
_ ...any, /* args */
) *mux.Route {
var handler http.Handler = http.HandlerFunc(f)
if h.TenancyMgr.Enabled {
handler = tenancy.ExtractTenantHTTPHandler(h.TenancyMgr, handler)
}
handler = otelhttp.WithRouteTag(route, handler)
return router.HandleFunc(route, handler.ServeHTTP)
}
Expand Down
72 changes: 7 additions & 65 deletions cmd/query/app/apiv3/http_gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package apiv3
import (
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -22,7 +21,6 @@ import (
"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/pkg/testutils"
dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
"github.com/jaegertracing/jaeger/storage/spanstore"
Expand All @@ -32,7 +30,6 @@ import (
func setupHTTPGatewayNoServer(
_ *testing.T,
basePath string,
tenancyOptions tenancy.Options,
) *testGateway {
gw := &testGateway{
reader: &spanstoremocks.Reader{},
Expand All @@ -45,7 +42,6 @@ func setupHTTPGatewayNoServer(

hgw := &HTTPGateway{
QueryService: q,
TenancyMgr: tenancy.NewManager(&tenancyOptions),
Logger: zap.NewNop(),
Tracer: jtracer.NoOp().OTEL,
}
Expand All @@ -61,9 +57,8 @@ func setupHTTPGatewayNoServer(
func setupHTTPGateway(
t *testing.T,
basePath string,
tenancyOptions tenancy.Options,
) *testGateway {
gw := setupHTTPGatewayNoServer(t, basePath, tenancyOptions)
gw := setupHTTPGatewayNoServer(t, basePath)

httpServer := httptest.NewServer(gw.router)
t.Cleanup(func() { httpServer.Close() })
Expand All @@ -76,22 +71,7 @@ func setupHTTPGateway(
}

func TestHTTPGateway(t *testing.T) {
for _, ten := range []bool{false, true} {
t.Run(fmt.Sprintf("tenancy=%v", ten), func(t *testing.T) {
tenancyOptions := tenancy.Options{
Enabled: ten,
}
tm := tenancy.NewManager(&tenancyOptions)
runGatewayTests(t, "/",
tenancyOptions,
func(req *http.Request) {
if ten {
// Add a tenancy header on outbound requests
req.Header.Add(tm.Header, "dummy")
}
})
})
}
runGatewayTests(t, "/")
}

func TestHTTPGatewayTryHandleError(t *testing.T) {
Expand Down Expand Up @@ -126,7 +106,7 @@ func TestHTTPGatewayOTLPError(t *testing.T) {
}

func TestHTTPGatewayGetTraceErrors(t *testing.T) {
gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{})
gw := setupHTTPGatewayNoServer(t, "")

// malformed trace id
r, err := http.NewRequest(http.MethodGet, "/api/v3/traces/xyz", nil)
Expand Down Expand Up @@ -233,7 +213,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) {
require.NoError(t, err)
w := httptest.NewRecorder()

gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{})
gw := setupHTTPGatewayNoServer(t, "")
gw.router.ServeHTTP(w, r)
assert.Contains(t, w.Body.String(), tc.expErr)
})
Expand All @@ -245,7 +225,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) {
require.NoError(t, err)
w := httptest.NewRecorder()

gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{})
gw := setupHTTPGatewayNoServer(t, "")
gw.reader.
On("FindTraces", matchContext, qp).
Return(nil, errors.New(simErr)).Once()
Expand All @@ -256,7 +236,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) {
}

func TestHTTPGatewayGetServicesErrors(t *testing.T) {
gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{})
gw := setupHTTPGatewayNoServer(t, "")

const simErr = "simulated error"
gw.reader.
Expand All @@ -271,7 +251,7 @@ func TestHTTPGatewayGetServicesErrors(t *testing.T) {
}

func TestHTTPGatewayGetOperationsErrors(t *testing.T) {
gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{})
gw := setupHTTPGatewayNoServer(t, "")

qp := spanstore.OperationQueryParameters{ServiceName: "foo", SpanKind: "server"}
const simErr = "simulated error"
Expand All @@ -285,41 +265,3 @@ func TestHTTPGatewayGetOperationsErrors(t *testing.T) {
gw.router.ServeHTTP(w, r)
assert.Contains(t, w.Body.String(), simErr)
}

func TestHTTPGatewayTenancyRejection(t *testing.T) {
basePath := "/"
tenancyOptions := tenancy.Options{Enabled: true}
gw := setupHTTPGateway(t, basePath, tenancyOptions)

traceID := model.NewTraceID(150, 160)
gw.reader.On("GetTrace", matchContext, matchTraceID).Return(
&model.Trace{
Spans: []*model.Span{
{
TraceID: traceID,
SpanID: model.NewSpanID(180),
OperationName: "foobar",
},
},
}, nil).Once()

req, err := http.NewRequest(http.MethodGet, gw.url+"/api/v3/traces/123", nil)
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")
// We don't set tenant header
response, err := http.DefaultClient.Do(req)
require.NoError(t, err)
body, err := io.ReadAll(response.Body)
require.NoError(t, err)
require.NoError(t, response.Body.Close())
require.Equal(t, http.StatusUnauthorized, response.StatusCode, "response=%s", string(body))

// Try again with tenant header set
tm := tenancy.NewManager(&tenancyOptions)
req.Header.Set(tm.Header, "acme")
response, err = http.DefaultClient.Do(req)
require.NoError(t, err)
require.NoError(t, response.Body.Close())
require.Equal(t, http.StatusOK, response.StatusCode)
// Skip unmarshal of response; it is enough that it succeeded
}
8 changes: 1 addition & 7 deletions cmd/query/app/http_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
uiconv "github.com/jaegertracing/jaeger/model/converter/json"
ui "github.com/jaegertracing/jaeger/model/json"
"github.com/jaegertracing/jaeger/pkg/jtracer"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
"github.com/jaegertracing/jaeger/storage/metricsstore"
Expand Down Expand Up @@ -76,22 +75,20 @@ type APIHandler struct {
queryService *querysvc.QueryService
metricsQueryService querysvc.MetricsQueryService
queryParser queryParser
tenancyMgr *tenancy.Manager
basePath string
apiPrefix string
logger *zap.Logger
tracer trace.TracerProvider
}

// NewAPIHandler returns an APIHandler
func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, options ...HandlerOption) *APIHandler {
func NewAPIHandler(queryService *querysvc.QueryService, options ...HandlerOption) *APIHandler {
aH := &APIHandler{
queryService: queryService,
queryParser: queryParser{
traceQueryLookbackDuration: defaultTraceQueryLookbackDuration,
timeNow: time.Now,
},
tenancyMgr: tm,
}

for _, option := range options {
Expand Down Expand Up @@ -135,9 +132,6 @@ func (aH *APIHandler) handleFunc(
) *mux.Route {
route := aH.formatRoute(routeFmt, args...)
var handler http.Handler = http.HandlerFunc(f)
if aH.tenancyMgr.Enabled {
handler = tenancy.ExtractTenantHTTPHandler(aH.tenancyMgr, handler)
}
handler = traceResponseHandler(handler)
handler = otelhttp.WithRouteTag(route, handler)
return router.HandleFunc(route, handler.ServeHTTP)
Expand Down
Loading

0 comments on commit a1beee7

Please sign in to comment.