Skip to content

Commit 1bd9541

Browse files
authored
FrontendService: Add tracing and logging middleware (#107956)
* FrontendService: Add tracing and logging middleware * tests! * middleware tests * context middleware test * revert http_server back to previous version * fix lint * fix test * use http.NotFound instead of custom http handler * use existing tracer for package * use otel/trace.Tracer in request_tracing middleware * tidy up tracing in contextMiddleware * fix 404 test * remove spans from contextMiddleware * comment
1 parent 92404d9 commit 1bd9541

File tree

8 files changed

+403
-19
lines changed

8 files changed

+403
-19
lines changed

pkg/api/http_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
596596
m := hs.web
597597

598598
m.Use(requestmeta.SetupRequestMetadata())
599-
m.Use(middleware.RequestTracing(hs.tracer))
599+
m.Use(middleware.RequestTracing(hs.tracer, middleware.SkipTracingPaths))
600600
m.Use(middleware.RequestMetrics(hs.Features, hs.Cfg, hs.promRegister))
601601

602602
m.UseMiddleware(hs.LoggerMiddleware.Middleware())

pkg/middleware/request_tracing.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,23 @@ func RouteOperationName(req *http.Request) (string, bool) {
7171
return "", false
7272
}
7373

74-
func RequestTracing(tracer tracing.Tracer) web.Middleware {
74+
// Paths that don't need tracing spans applied to them because of the
75+
// little value that would provide us
76+
func SkipTracingPaths(req *http.Request) bool {
77+
return strings.HasPrefix(req.URL.Path, "/public/") ||
78+
req.URL.Path == "/robots.txt" ||
79+
req.URL.Path == "/favicon.ico" ||
80+
req.URL.Path == "/api/health"
81+
}
82+
83+
func TraceAllPaths(req *http.Request) bool {
84+
return true
85+
}
86+
87+
func RequestTracing(tracer trace.Tracer, shouldTrace func(*http.Request) bool) web.Middleware {
7588
return func(next http.Handler) http.Handler {
7689
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
77-
// skip tracing for a few endpoints
78-
if strings.HasPrefix(req.URL.Path, "/public/") ||
79-
req.URL.Path == "/robots.txt" ||
80-
req.URL.Path == "/favicon.ico" ||
81-
req.URL.Path == "/api/health" {
90+
if !shouldTrace(req) {
8291
next.ServeHTTP(w, req)
8392
return
8493
}

pkg/server/module_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func (s *ModuleServer) Run() error {
193193
})
194194

195195
m.RegisterModule(modules.FrontendServer, func() (services.Service, error) {
196-
return frontend.ProvideFrontendService(s.cfg, s.promGatherer, s.license)
196+
return frontend.ProvideFrontendService(s.cfg, s.features, s.promGatherer, s.registerer, s.license)
197197
})
198198

199199
m.RegisterModule(modules.All, nil)
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package frontend
2+
3+
import (
4+
"context"
5+
"net/http"
6+
7+
"github.com/grafana/grafana/pkg/infra/log"
8+
"github.com/grafana/grafana/pkg/infra/tracing"
9+
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
10+
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
11+
"github.com/grafana/grafana/pkg/web"
12+
)
13+
14+
// Minimal copy of contextHandler.Middleware for frontend-service
15+
// frontend-service doesn't handle authentication or know what signed in users are
16+
func (s *frontendService) contextMiddleware() web.Middleware {
17+
return func(next http.Handler) http.Handler {
18+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
19+
ctx := r.Context()
20+
21+
reqContext := &contextmodel.ReqContext{
22+
Context: web.FromContext(ctx),
23+
Logger: log.New("context"),
24+
}
25+
26+
// inject ReqContext in the context
27+
ctx = context.WithValue(ctx, ctxkey.Key{}, reqContext)
28+
29+
// Set the context for the http.Request.Context
30+
// This modifies both r and reqContext.Req since they point to the same value
31+
*reqContext.Req = *reqContext.Req.WithContext(ctx)
32+
33+
traceID := tracing.TraceIDFromContext(ctx, false)
34+
if traceID != "" {
35+
reqContext.Logger = reqContext.Logger.New("traceID", traceID)
36+
}
37+
38+
next.ServeHTTP(w, r.WithContext(ctx))
39+
})
40+
}
41+
}

pkg/services/frontend/frontend_service.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,17 @@ import (
99
"github.com/prometheus/client_golang/prometheus"
1010
"github.com/prometheus/client_golang/prometheus/promhttp"
1111
"go.opentelemetry.io/otel"
12+
"go.opentelemetry.io/otel/trace"
1213

1314
"github.com/grafana/dskit/services"
1415
"github.com/grafana/grafana/pkg/infra/log"
16+
"github.com/grafana/grafana/pkg/middleware"
17+
"github.com/grafana/grafana/pkg/middleware/loggermw"
18+
"github.com/grafana/grafana/pkg/middleware/requestmeta"
19+
"github.com/grafana/grafana/pkg/services/featuremgmt"
1520
"github.com/grafana/grafana/pkg/services/licensing"
1621
"github.com/grafana/grafana/pkg/setting"
22+
"github.com/grafana/grafana/pkg/web"
1723
)
1824

1925
var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/frontend")
@@ -22,23 +28,31 @@ type frontendService struct {
2228
*services.BasicService
2329
cfg *setting.Cfg
2430
httpServ *http.Server
31+
features featuremgmt.FeatureToggles
2532
log log.Logger
2633
errChan chan error
2734
promGatherer prometheus.Gatherer
35+
promRegister prometheus.Registerer
36+
tracer trace.Tracer
37+
license licensing.Licensing
2838

2939
index *IndexProvider
3040
}
3141

32-
func ProvideFrontendService(cfg *setting.Cfg, promGatherer prometheus.Gatherer, license licensing.Licensing) (*frontendService, error) {
42+
func ProvideFrontendService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, promGatherer prometheus.Gatherer, promRegister prometheus.Registerer, license licensing.Licensing) (*frontendService, error) {
3343
index, err := NewIndexProvider(cfg, license)
3444
if err != nil {
3545
return nil, err
3646
}
3747

3848
s := &frontendService{
3949
cfg: cfg,
50+
features: features,
4051
log: log.New("frontend-server"),
4152
promGatherer: promGatherer,
53+
promRegister: promRegister,
54+
tracer: tracer,
55+
license: license,
4256
index: index,
4357
}
4458
s.BasicService = services.NewBasicService(s.start, s.running, s.stop)
@@ -65,6 +79,7 @@ func (s *frontendService) running(ctx context.Context) error {
6579

6680
func (s *frontendService) stop(failureReason error) error {
6781
s.log.Info("stopping frontend server", "reason", failureReason)
82+
6883
if err := s.httpServ.Shutdown(context.Background()); err != nil {
6984
s.log.Error("failed to shutdown frontend server", "error", err)
7085
return err
@@ -75,17 +90,48 @@ func (s *frontendService) stop(failureReason error) error {
7590
func (s *frontendService) newFrontendServer(ctx context.Context) *http.Server {
7691
s.log.Info("starting frontend server", "addr", ":"+s.cfg.HTTPPort)
7792

78-
router := http.NewServeMux()
79-
router.Handle("/metrics", promhttp.HandlerFor(s.promGatherer, promhttp.HandlerOpts{EnableOpenMetrics: true}))
80-
router.HandleFunc("/", s.index.HandleRequest)
93+
// Use the same web.Mux as the main grafana server for consistency + middleware reuse
94+
handler := web.New()
95+
s.addMiddlewares(handler)
96+
s.registerRoutes(handler)
8197

8298
server := &http.Server{
8399
// 5s timeout for header reads to avoid Slowloris attacks (https://thetooth.io/blog/slowloris-attack/)
84100
ReadHeaderTimeout: 5 * time.Second,
85101
Addr: ":" + s.cfg.HTTPPort,
86-
Handler: router,
102+
Handler: handler,
87103
BaseContext: func(_ net.Listener) context.Context { return ctx },
88104
}
89105

90106
return server
91107
}
108+
109+
func (s *frontendService) routeGet(m *web.Mux, pattern string, h ...web.Handler) {
110+
handlers := append([]web.Handler{middleware.ProvideRouteOperationName(pattern)}, h...)
111+
m.Get(pattern, handlers...)
112+
}
113+
114+
// Apply the same middleware patterns as the main HTTP server
115+
func (s *frontendService) addMiddlewares(m *web.Mux) {
116+
loggermiddleware := loggermw.Provide(s.cfg, s.features)
117+
118+
m.Use(requestmeta.SetupRequestMetadata())
119+
m.UseMiddleware(s.contextMiddleware())
120+
121+
m.Use(middleware.RequestTracing(s.tracer, middleware.TraceAllPaths))
122+
m.Use(middleware.RequestMetrics(s.features, s.cfg, s.promRegister))
123+
m.UseMiddleware(loggermiddleware.Middleware())
124+
125+
m.UseMiddleware(middleware.Recovery(s.cfg, s.license))
126+
}
127+
128+
func (s *frontendService) registerRoutes(m *web.Mux) {
129+
s.routeGet(m, "/metrics", promhttp.HandlerFor(s.promGatherer, promhttp.HandlerOpts{EnableOpenMetrics: true}))
130+
131+
// Frontend service doesn't (yet?) serve any assets, so explicitly 404
132+
// them so we can get logs for them
133+
s.routeGet(m, "/public/*", http.NotFound)
134+
135+
// All other requests return index.html
136+
s.routeGet(m, "/*", s.index.HandleRequest)
137+
}
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
package frontend
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"strings"
8+
"testing"
9+
10+
"github.com/prometheus/client_golang/prometheus"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
14+
"github.com/grafana/grafana/pkg/services/contexthandler"
15+
"github.com/grafana/grafana/pkg/services/featuremgmt"
16+
"github.com/grafana/grafana/pkg/services/licensing"
17+
"github.com/grafana/grafana/pkg/setting"
18+
"github.com/grafana/grafana/pkg/web"
19+
)
20+
21+
// Helper function to create a test service with minimal configuration
22+
func createTestService(t *testing.T, cfg *setting.Cfg) *frontendService {
23+
t.Helper()
24+
25+
features := featuremgmt.WithFeatures()
26+
license := &licensing.OSSLicensingService{}
27+
28+
var promRegister prometheus.Registerer = prometheus.NewRegistry()
29+
promGatherer := promRegister.(*prometheus.Registry)
30+
31+
service, err := ProvideFrontendService(cfg, features, promGatherer, promRegister, license)
32+
require.NoError(t, err)
33+
34+
return service
35+
}
36+
37+
func TestFrontendService_ServerCreation(t *testing.T) {
38+
t.Run("should create HTTP server with correct configuration", func(t *testing.T) {
39+
publicDir := setupTestWebAssets(t)
40+
cfg := &setting.Cfg{
41+
HTTPPort: "1234",
42+
StaticRootPath: publicDir,
43+
}
44+
service := createTestService(t, cfg)
45+
46+
ctx := context.Background()
47+
server := service.newFrontendServer(ctx)
48+
49+
assert.NotNil(t, server)
50+
assert.Equal(t, ":1234", server.Addr)
51+
assert.NotNil(t, server.Handler)
52+
assert.NotNil(t, server.BaseContext)
53+
})
54+
}
55+
56+
func TestFrontendService_Routes(t *testing.T) {
57+
publicDir := setupTestWebAssets(t)
58+
cfg := &setting.Cfg{
59+
HTTPPort: "3000",
60+
StaticRootPath: publicDir,
61+
}
62+
service := createTestService(t, cfg)
63+
64+
// Create a test mux to verify route registration
65+
mux := web.New()
66+
service.addMiddlewares(mux)
67+
service.registerRoutes(mux)
68+
69+
t.Run("should handle frontend wildcard routes", func(t *testing.T) {
70+
// Test that routes are registered by making requests
71+
testCases := []struct {
72+
path string
73+
description string
74+
}{
75+
// Metrics isn't registered in the test service?
76+
{"/", "index route should return HTML"},
77+
{"/dashboards", "browse dashboards route should return HTML"},
78+
{"/d/de773f33s8qgwf/fep-homepage", "dashboard route should return HTML"},
79+
}
80+
81+
for _, tc := range testCases {
82+
t.Run(tc.description, func(t *testing.T) {
83+
req := httptest.NewRequest("GET", tc.path, nil)
84+
recorder := httptest.NewRecorder()
85+
86+
mux.ServeHTTP(recorder, req)
87+
88+
assert.Equal(t, 200, recorder.Code)
89+
assert.Contains(t, recorder.Body.String(), "<div id=\"reactRoot\"></div>")
90+
})
91+
}
92+
})
93+
94+
t.Run("should handle assets 404 correctly", func(t *testing.T) {
95+
req := httptest.NewRequest("GET", "/public/build/app.js", nil)
96+
recorder := httptest.NewRecorder()
97+
98+
mux.ServeHTTP(recorder, req)
99+
100+
assert.Equal(t, 404, recorder.Code)
101+
assert.Equal(t, "404 page not found", strings.TrimSpace(recorder.Body.String()))
102+
})
103+
104+
t.Run("should return prometheus metrics", func(t *testing.T) {
105+
testCounter := prometheus.NewCounter(prometheus.CounterOpts{
106+
Name: "shrimp_count",
107+
})
108+
err := service.promRegister.Register(testCounter)
109+
require.NoError(t, err)
110+
testCounter.Inc()
111+
112+
req := httptest.NewRequest("GET", "/metrics", nil)
113+
recorder := httptest.NewRecorder()
114+
115+
mux.ServeHTTP(recorder, req)
116+
117+
assert.Contains(t, recorder.Body.String(), "\nshrimp_count 1\n")
118+
})
119+
}
120+
121+
func TestFrontendService_Middleware(t *testing.T) {
122+
publicDir := setupTestWebAssets(t)
123+
cfg := &setting.Cfg{
124+
HTTPPort: "3000",
125+
StaticRootPath: publicDir,
126+
}
127+
128+
t.Run("should register route prom metrics", func(t *testing.T) {
129+
service := createTestService(t, cfg)
130+
mux := web.New()
131+
service.addMiddlewares(mux)
132+
service.registerRoutes(mux)
133+
134+
req := httptest.NewRequest("GET", "/dashboards", nil)
135+
recorder := httptest.NewRecorder()
136+
mux.ServeHTTP(recorder, req)
137+
138+
req = httptest.NewRequest("GET", "/public/build/app.js", nil)
139+
mux.ServeHTTP(recorder, req)
140+
141+
req = httptest.NewRequest("GET", "/metrics", nil)
142+
mux.ServeHTTP(recorder, req)
143+
144+
metricsBody := recorder.Body.String()
145+
assert.Contains(t, metricsBody, "# TYPE grafana_http_request_duration_seconds histogram")
146+
assert.Contains(t, metricsBody, "grafana_http_request_duration_seconds_bucket{handler=\"public-assets\"") // assets 404
147+
assert.Contains(t, metricsBody, "grafana_http_request_duration_seconds_bucket{handler=\"/*\"") // index route
148+
})
149+
150+
t.Run("should add context middleware", func(t *testing.T) {
151+
service := createTestService(t, cfg)
152+
mux := web.New()
153+
service.addMiddlewares(mux)
154+
155+
mux.Get("/test-route", func(w http.ResponseWriter, r *http.Request) {
156+
ctx := contexthandler.FromContext(r.Context())
157+
assert.NotNil(t, ctx)
158+
assert.NotNil(t, ctx.Context)
159+
assert.NotNil(t, ctx.Logger)
160+
161+
w.WriteHeader(200)
162+
_, err := w.Write([]byte("ok"))
163+
require.NoError(t, err)
164+
})
165+
166+
req := httptest.NewRequest("GET", "/test-route", nil)
167+
recorder := httptest.NewRecorder()
168+
mux.ServeHTTP(recorder, req)
169+
})
170+
}

pkg/services/frontend/index.html

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,7 @@
9999
</script>
100100

101101
[[range $asset := .Assets.JSFiles]]
102-
<script
103-
nonce="[[$.Nonce]]"
104-
src="[[$asset.FilePath]]"
105-
type="text/javascript"
106-
defer
107-
></script>
102+
<script nonce="[[$.Nonce]]" src="[[$asset.FilePath]]" type="text/javascript" defer></script>
108103
[[end]]
109104
</body>
110105
</html>

0 commit comments

Comments
 (0)