Skip to content

Commit

Permalink
Restore tempo_request_duration_seconds metrics for querier_api_* requ…
Browse files Browse the repository at this point in the history 
…ests (grafana#3403)

* Restore tempo_request_duration_seconds metrics for querier_api_* requests

* Update CHANGELOG.md
  • Loading branch information
@kvrhdn
kvrhdn committed Feb 26, 2024
1 parent 0d79a1e commit 047c5e7
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 56 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* [BUGFIX] Move waitgroup handling for poller error condition [#3224](https://github.com/grafana/tempo/pull/3224) (@zalegrala)
* [BUGFIX] Fix head block excessive locking in ingester search [#3328](https://github.com/grafana/tempo/pull/3328) (@mdisibio)
* [BUGFIX] Fix issue with ingester failed to cut traces no such file or directory [#3346](https://github.com/grafana/tempo/issues/3346) (@mdisibio)
* [BUGFIX] Restore `tempo_request_duration_seconds` metrics for `querier_api_*` requests [#3403](https://github.com/grafana/tempo/pull/3403) (@kvrhdn)
* [ENHANCEMENT] Introduced `AttributePolicyMatch` & `IntrinsicPolicyMatch` structures to match span attributes based on strongly typed values & precompiled regexp [#3025](https://github.com/grafana/tempo/pull/3025) (@andriusluk)
* [CHANGE] TraceQL/Structural operators performance improvement. [#3088](https://github.com/grafana/tempo/pull/3088) (@joe-elliott)
* [CHANGE] Merge the processors overrides set through runtime overrides and user-configurable overrides [#3125](https://github.com/grafana/tempo/pull/3125) (@kvrhdn)
Expand Down
10 changes: 5 additions & 5 deletions cmd/tempo/app/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,11 @@ func (t *App) Run() error {
t.InternalServer.HTTP.Path("/ready").Methods("GET").Handler(t.readyHandler(sm, shutdownRequested))
}

t.Server.HTTP().Path(addHTTPAPIPrefix(&t.cfg, api.PathBuildInfo)).Handler(t.buildinfoHandler()).Methods("GET")
t.Server.HTTPRouter().Path(addHTTPAPIPrefix(&t.cfg, api.PathBuildInfo)).Handler(t.buildinfoHandler()).Methods("GET")

t.Server.HTTP().Path("/ready").Handler(t.readyHandler(sm, shutdownRequested))
t.Server.HTTP().Path("/status").Handler(t.statusHandler()).Methods("GET")
t.Server.HTTP().Path("/status/{endpoint}").Handler(t.statusHandler()).Methods("GET")
t.Server.HTTPRouter().Path("/ready").Handler(t.readyHandler(sm, shutdownRequested))
t.Server.HTTPRouter().Path("/status").Handler(t.statusHandler()).Methods("GET")
t.Server.HTTPRouter().Path("/status/{endpoint}").Handler(t.statusHandler()).Methods("GET")
grpc_health_v1.RegisterHealthServer(t.Server.GRPC(),
grpcutil.NewHealthCheckFrom(
grpcutil.WithShutdownRequested(shutdownRequested),
Expand Down Expand Up @@ -497,7 +497,7 @@ func (t *App) writeStatusEndpoints(w io.Writer) error {

endpoints := []endpoint{}

err := t.Server.HTTP().Walk(func(route *mux.Route, router *mux.Router, ancestors []*mux.Route) error {
err := t.Server.HTTPRouter().Walk(func(route *mux.Route, router *mux.Router, ancestors []*mux.Route) error {
e := endpoint{}

pathTemplate, err := route.GetPathTemplate()
Expand Down
68 changes: 34 additions & 34 deletions cmd/tempo/app/modules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (t *App) initReadRing(cfg ring.Config, name, key string) (*ring.Ring, error
return nil, fmt.Errorf("failed to create ring %s: %w", name, err)
}

t.Server.HTTP().Handle("/"+name+"/ring", ring)
t.Server.HTTPRouter().Handle("/"+name+"/ring", ring)
t.readRings[name] = ring

return ring, nil
Expand All @@ -182,8 +182,8 @@ func (t *App) initOverrides() (services.Service, error) {
prometheus.MustRegister(t.Overrides)
}

t.Server.HTTP().Path("/status/overrides").HandlerFunc(overrides.TenantsHandler(t.Overrides)).Methods("GET")
t.Server.HTTP().Path("/status/overrides/{tenant}").HandlerFunc(overrides.TenantStatusHandler(t.Overrides)).Methods("GET")
t.Server.HTTPRouter().Path("/status/overrides").HandlerFunc(overrides.TenantsHandler(t.Overrides)).Methods("GET")
t.Server.HTTPRouter().Path("/status/overrides/{tenant}").HandlerFunc(overrides.TenantStatusHandler(t.Overrides)).Methods("GET")

return t.Overrides, nil
}
Expand All @@ -205,10 +205,10 @@ func (t *App) initOverridesAPI() (services.Service, error) {
return t.HTTPAuthMiddleware.Wrap(h)
}

t.Server.HTTP().Path(overridesPath).Methods(http.MethodGet).Handler(wrapHandler(userConfigOverridesAPI.GetHandler))
t.Server.HTTP().Path(overridesPath).Methods(http.MethodPost).Handler(wrapHandler(userConfigOverridesAPI.PostHandler))
t.Server.HTTP().Path(overridesPath).Methods(http.MethodPatch).Handler(wrapHandler(userConfigOverridesAPI.PatchHandler))
t.Server.HTTP().Path(overridesPath).Methods(http.MethodDelete).Handler(wrapHandler(userConfigOverridesAPI.DeleteHandler))
t.Server.HTTPRouter().Path(overridesPath).Methods(http.MethodGet).Handler(wrapHandler(userConfigOverridesAPI.GetHandler))
t.Server.HTTPRouter().Path(overridesPath).Methods(http.MethodPost).Handler(wrapHandler(userConfigOverridesAPI.PostHandler))
t.Server.HTTPRouter().Path(overridesPath).Methods(http.MethodPatch).Handler(wrapHandler(userConfigOverridesAPI.PatchHandler))
t.Server.HTTPRouter().Path(overridesPath).Methods(http.MethodDelete).Handler(wrapHandler(userConfigOverridesAPI.DeleteHandler))

return userConfigOverridesAPI, nil
}
Expand All @@ -229,7 +229,7 @@ func (t *App) initDistributor() (services.Service, error) {
t.distributor = distributor

if distributor.DistributorRing != nil {
t.Server.HTTP().Handle("/distributor/ring", distributor.DistributorRing)
t.Server.HTTPRouter().Handle("/distributor/ring", distributor.DistributorRing)
}

return t.distributor, nil
Expand All @@ -247,8 +247,8 @@ func (t *App) initIngester() (services.Service, error) {

tempopb.RegisterPusherServer(t.Server.GRPC(), t.ingester)
tempopb.RegisterQuerierServer(t.Server.GRPC(), t.ingester)
t.Server.HTTP().Path("/flush").Handler(http.HandlerFunc(t.ingester.FlushHandler))
t.Server.HTTP().Path("/shutdown").Handler(http.HandlerFunc(t.ingester.ShutdownHandler))
t.Server.HTTPRouter().Path("/flush").Handler(http.HandlerFunc(t.ingester.FlushHandler))
t.Server.HTTPRouter().Path("/shutdown").Handler(http.HandlerFunc(t.ingester.ShutdownHandler))
return t.ingester, nil
}

Expand All @@ -265,10 +265,10 @@ func (t *App) initGenerator() (services.Service, error) {
t.generator = genSvc

spanStatsHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.generator.SpanMetricsHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixGenerator, addHTTPAPIPrefix(&t.cfg, api.PathSpanMetrics)), spanStatsHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixGenerator, addHTTPAPIPrefix(&t.cfg, api.PathSpanMetrics)), spanStatsHandler)

queryRangeHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.generator.QueryRangeHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixGenerator, addHTTPAPIPrefix(&t.cfg, api.PathMetricsQueryRange)), queryRangeHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixGenerator, addHTTPAPIPrefix(&t.cfg, api.PathMetricsQueryRange)), queryRangeHandler)

tempopb.RegisterMetricsGeneratorServer(t.Server.GRPC(), t.generator)

Expand Down Expand Up @@ -319,30 +319,30 @@ func (t *App) initQuerier() (services.Service, error) {
)

tracesHandler := middleware.Wrap(http.HandlerFunc(t.querier.TraceByIDHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathTraces)), tracesHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathTraces)), tracesHandler)

searchHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.SearchHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearch)), searchHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearch)), searchHandler)

searchTagsHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.SearchTagsHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTags)), searchTagsHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTags)), searchTagsHandler)

searchTagsV2Handler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.SearchTagsV2Handler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTagsV2)), searchTagsV2Handler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTagsV2)), searchTagsV2Handler)

searchTagValuesHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.SearchTagValuesHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValues)), searchTagValuesHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValues)), searchTagValuesHandler)

searchTagValuesV2Handler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.SearchTagValuesV2Handler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValuesV2)), searchTagValuesV2Handler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValuesV2)), searchTagValuesV2Handler)

spanMetricsSummaryHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.SpanMetricsSummaryHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSpanMetricsSummary)), spanMetricsSummaryHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathSpanMetricsSummary)), spanMetricsSummaryHandler)

queryRangeHandler := t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(t.querier.QueryRangeHandler))
t.Server.HTTP().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathMetricsQueryRange)), queryRangeHandler)
t.Server.HTTPRouter().Handle(path.Join(api.PathPrefixQuerier, addHTTPAPIPrefix(&t.cfg, api.PathMetricsQueryRange)), queryRangeHandler)

return t.querier, t.querier.CreateAndRegisterWorker(t.Server.HTTP())
return t.querier, t.querier.CreateAndRegisterWorker(t.Server.HTTPHandler())
}

func (t *App) initQueryFrontend() (services.Service, error) {
Expand Down Expand Up @@ -373,28 +373,28 @@ func (t *App) initQueryFrontend() (services.Service, error) {
)

// http trace by id endpoint
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathTraces), base.Wrap(queryFrontend.TraceByIDHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathTraces), base.Wrap(queryFrontend.TraceByIDHandler))

// http search endpoints
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearch), base.Wrap(queryFrontend.SearchHandler))
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTags), base.Wrap(queryFrontend.SearchTagsHandler))
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTagsV2), base.Wrap(queryFrontend.SearchTagsV2Handler))
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValues), base.Wrap(queryFrontend.SearchTagsValuesHandler))
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValuesV2), base.Wrap(queryFrontend.SearchTagsValuesV2Handler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearch), base.Wrap(queryFrontend.SearchHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTags), base.Wrap(queryFrontend.SearchTagsHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTagsV2), base.Wrap(queryFrontend.SearchTagsV2Handler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValues), base.Wrap(queryFrontend.SearchTagsValuesHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSearchTagValuesV2), base.Wrap(queryFrontend.SearchTagsValuesV2Handler))

// http metrics endpoints
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSpanMetricsSummary), base.Wrap(queryFrontend.SpanMetricsSummaryHandler))
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathMetricsQueryRange), base.Wrap(queryFrontend.QueryRangeHandler))
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathPromQueryRange), base.Wrap(queryFrontend.QueryRangeHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathSpanMetricsSummary), base.Wrap(queryFrontend.SpanMetricsSummaryHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathMetricsQueryRange), base.Wrap(queryFrontend.QueryRangeHandler))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathPromQueryRange), base.Wrap(queryFrontend.QueryRangeHandler))

// the query frontend needs to have knowledge of the blocks so it can shard search jobs
t.store.EnablePolling(context.Background(), nil)

// http query echo endpoint
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathEcho), echoHandler())
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathEcho), echoHandler())

// http endpoint to see usage stats data
t.Server.HTTP().Handle(addHTTPAPIPrefix(&t.cfg, api.PathUsageStats), usageStatsHandler(t.cfg.UsageReport))
t.Server.HTTPRouter().Handle(addHTTPAPIPrefix(&t.cfg, api.PathUsageStats), usageStatsHandler(t.cfg.UsageReport))

// todo: queryFrontend should implement service.Service and take the cortex frontend a submodule
return t.frontend, nil
Expand All @@ -412,7 +412,7 @@ func (t *App) initCompactor() (services.Service, error) {
t.compactor = compactor

if t.compactor.Ring != nil {
t.Server.HTTP().Handle("/compactor/ring", t.compactor.Ring)
t.Server.HTTPRouter().Handle("/compactor/ring", t.compactor.Ring)
}

return t.compactor, nil
Expand Down Expand Up @@ -452,7 +452,7 @@ func (t *App) initMemberlistKV() (services.Service, error) {
t.cfg.Distributor.DistributorRing.KVStore.MemberlistKV = t.MemberlistKV.GetMemberlistKV
t.cfg.Compactor.ShardingRing.KVStore.MemberlistKV = t.MemberlistKV.GetMemberlistKV

t.Server.HTTP().Handle("/memberlist", t.MemberlistKV)
t.Server.HTTPRouter().Handle("/memberlist", t.MemberlistKV)

return t.MemberlistKV, nil
}
Expand Down
47 changes: 30 additions & 17 deletions cmd/tempo/app/server_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (
)

type TempoServer interface {
HTTP() *mux.Router
HTTPRouter() *mux.Router
HTTPHandler() http.Handler
GRPC() *grpc.Server
Log() log.Logger
EnableHTTP2()
Expand All @@ -32,7 +33,8 @@ type TempoServer interface {

// todo: evaluate whether the internal server should be included as part of this
type tempoServer struct {
mux *mux.Router // all tempo http routes are added here
mux *mux.Router // all tempo http routes are added here
handler http.Handler // the final handler which includes the router and any middleware

externalServer *server.Server // the standard server that all HTTP/GRPC requests are served on
enableHTTP2Once sync.Once
Expand All @@ -45,10 +47,14 @@ func newTempoServer() *tempoServer {
}
}

func (s *tempoServer) HTTP() *mux.Router {
func (s *tempoServer) HTTPRouter() *mux.Router {
return s.mux
}

func (s *tempoServer) HTTPHandler() http.Handler {
return s.handler
}

func (s *tempoServer) GRPC() *grpc.Server {
return s.externalServer.GRPC
}
Expand All @@ -71,22 +77,28 @@ func (s *tempoServer) StartAndReturnService(cfg server.Config, supportGRPCOnHTTP
var err error

metrics := server.NewServerMetrics(cfg)
// use tempo's mux unless we are doing grpc over http, then we will let the library instantiate its own
// router and piggy back on it to route grpc requests
cfg.Router = s.mux
if supportGRPCOnHTTP {
DisableSignalHandling(&cfg)

if !supportGRPCOnHTTP {
// We don't do any GRPC handling, let the library handle all routing for us
cfg.Router = s.mux

s.externalServer, err = server.NewWithMetrics(cfg, metrics)
if err != nil {
return nil, fmt.Errorf("failed to create server: %w", err)
}
s.handler = s.externalServer.HTTPServer.Handler
} else {
// We want to route both GRPC and HTTP requests on the same endpoint
cfg.Router = nil
cfg.DoNotAddDefaultHTTPMiddleware = true // we don't want instrumentation on the "root" router, we want it on our mux. it will be added below.
}

DisableSignalHandling(&cfg)
s.externalServer, err = server.NewWithMetrics(cfg, metrics)
if err != nil {
return nil, fmt.Errorf("failed to create server: %w", err)
}
s.externalServer, err = server.NewWithMetrics(cfg, metrics)
if err != nil {
return nil, fmt.Errorf("failed to create server: %w", err)
}

// now that we have created the server and service let's setup our grpc/http router if necessary
if supportGRPCOnHTTP {
// now that we have created the server and service let's setup our grpc/http router if necessary
// for grpc to work we must enable h2c on the external server
s.EnableHTTP2()

Expand All @@ -96,7 +108,8 @@ func (s *tempoServer) StartAndReturnService(cfg server.Config, supportGRPCOnHTTP
if err != nil {
return nil, fmt.Errorf("failed to create http middleware: %w", err)
}
router := middleware.Merge(httpMiddleware...).Wrap(s.mux)

s.handler = middleware.Merge(httpMiddleware...).Wrap(s.mux)
s.externalServer.HTTP.PathPrefix("/").HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
// route to GRPC server if it's a GRPC request
if req.ProtoMajor == 2 && strings.Contains(req.Header.Get("Content-Type"), "application/grpc") {
Expand All @@ -105,7 +118,7 @@ func (s *tempoServer) StartAndReturnService(cfg server.Config, supportGRPCOnHTTP
}

// default to standard http server
router.ServeHTTP(w, req)
s.handler.ServeHTTP(w, req)
})
}

Expand Down

0 comments on commit 047c5e7

Please sign in to comment.