Skip to content

Commit

Permalink
[query] Use OTEL's helpers for http server (#6121)
Browse files Browse the repository at this point in the history
  • Loading branch information
mahadzaryab1 authored Oct 30, 2024
1 parent 0af8d35 commit c5f34d9
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 29 deletions.
18 changes: 12 additions & 6 deletions cmd/query/app/apiv3/http_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,16 @@ 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)
traceMiddleware := otelhttp.NewHandler(
otelhttp.WithRouteTag(route, handler),
route,
otelhttp.WithTracerProvider(h.Tracer))
return router.HandleFunc(route, traceMiddleware.ServeHTTP)
handler = otelhttp.WithRouteTag(route, handler)
handler = spanNameHandler(route, handler)
return router.HandleFunc(route, handler.ServeHTTP)
}

// tryHandleError checks if the passed error is not nil and handles it by writing
Expand Down Expand Up @@ -242,3 +240,11 @@ func (h *HTTPGateway) getOperations(w http.ResponseWriter, r *http.Request) {
}
h.marshalResponse(&api_v3.GetOperationsResponse{Operations: apiOperations}, w)
}

func spanNameHandler(spanName string, handler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
span := trace.SpanFromContext(r.Context())
span.SetName(spanName)
handler.ServeHTTP(w, r)
})
}
17 changes: 12 additions & 5 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,10 @@ func (aH *APIHandler) handleFunc(
) *mux.Route {
route := aH.formatRoute(routeFmt, args...)
var handler http.Handler = http.HandlerFunc(f)
traceMiddleware := otelhttp.NewHandler(
otelhttp.WithRouteTag(route, traceResponseHandler(handler)),
route,
otelhttp.WithTracerProvider(aH.tracer))
return router.HandleFunc(route, traceMiddleware.ServeHTTP)
handler = traceResponseHandler(handler)
handler = otelhttp.WithRouteTag(route, handler)
handler = spanNameHandler(route, handler)
return router.HandleFunc(route, handler.ServeHTTP)
}

func (aH *APIHandler) formatRoute(route string, args ...any) string {
Expand Down Expand Up @@ -532,3 +531,11 @@ func traceResponseHandler(handler http.Handler) http.Handler {
handler.ServeHTTP(w, r)
})
}

func spanNameHandler(spanName string, handler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
span := trace.SpanFromContext(r.Context())
span.SetName(spanName)
handler.ServeHTTP(w, r)
})
}
3 changes: 0 additions & 3 deletions cmd/query/app/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,6 @@ func TestGetTrace(t *testing.T) {
require.NoError(t, err)
assert.Empty(t, response.Errors)

assert.Len(t, exporter.GetSpans(), 1, "HTTP request was traced and span reported")
assert.Equal(t, "/api/traces/{traceID}", exporter.GetSpans()[0].Name)

traces := extractTraces(t, &response)
assert.Len(t, traces[0].Spans, 2)
assert.Len(t, traces[0].Spans[1].References, testCase.numSpanRefs)
Expand Down
34 changes: 19 additions & 15 deletions cmd/query/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import (
"net/http"
"strings"
"sync"
"time"

"github.com/gorilla/handlers"
"github.com/soheilhy/cmux"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componentstatus"
Expand All @@ -23,7 +21,6 @@ import (
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/noop"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/health"
Expand Down Expand Up @@ -91,7 +88,6 @@ func NewServer(
return nil, err
}
registerGRPCHandlers(grpcServer, querySvc, metricsQuerySvc, telset)

httpServer, err := createHTTPServer(ctx, querySvc, metricsQuerySvc, options, tm, telset)
if err != nil {
return nil, err
Expand Down Expand Up @@ -262,20 +258,28 @@ func createHTTPServer(
telset telemetery.Setting,
) (*httpServer, error) {
handler, staticHandlerCloser := initRouter(querySvc, metricsQuerySvc, queryOpts, tm, telset)
handler = responseHeadersHandler(handler, queryOpts.HTTP.ResponseHeaders)
handler = handlers.CompressHandler(handler)
recoveryHandler := recoveryhandler.NewRecoveryHandler(telset.Logger, true)

errorLog, _ := zap.NewStdLogAt(telset.Logger, zapcore.ErrorLevel)
server := &httpServer{
Server: &http.Server{
Handler: recoveryHandler(handler),
ErrorLog: errorLog,
ReadHeaderTimeout: 2 * time.Second,
handler = recoveryhandler.NewRecoveryHandler(telset.Logger, true)(handler)
hs, err := queryOpts.HTTP.ToServer(
ctx,
telset.Host,
component.TelemetrySettings{
Logger: telset.Logger,
TracerProvider: telset.TracerProvider,
LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider {
return noop.NewMeterProvider()
},
},
handler,
)
if err != nil {
return nil, errors.Join(err, staticHandlerCloser.Close())
}
server := &httpServer{
Server: hs,
staticHandlerCloser: staticHandlerCloser,
}

// TODO why doesn't OTEL helper do that already?
if queryOpts.HTTP.TLSSetting != nil {
tlsCfg, err := queryOpts.HTTP.TLSSetting.LoadTLSConfig(ctx) // This checks if the certificates are correctly provided
if err != nil {
Expand Down Expand Up @@ -303,7 +307,7 @@ func (s *Server) initListener(ctx context.Context) (cmux.CMux, error) {
return nil, err
}

s.httpConn, err = net.Listen("tcp", s.queryOptions.HTTP.Endpoint)
s.httpConn, err = s.queryOptions.HTTP.ToListener(ctx)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit c5f34d9

Please sign in to comment.