From e18d81f6427d01b5a8f91b39be7a3fcd52cf1582 Mon Sep 17 00:00:00 2001 From: Dave Freilich Date: Thu, 28 Nov 2024 16:29:18 +0200 Subject: [PATCH] update --- router/core/engine_loader_hooks.go | 65 +++++++++---------- router/core/graph_server.go | 7 +- router/core/graphql_handler.go | 7 -- .../internal/requestlogger/requestlogger.go | 6 +- .../internal/requestlogger/subgraphlogger.go | 10 +-- 5 files changed, 42 insertions(+), 53 deletions(-) diff --git a/router/core/engine_loader_hooks.go b/router/core/engine_loader_hooks.go index 40612bddd..44dd44767 100644 --- a/router/core/engine_loader_hooks.go +++ b/router/core/engine_loader_hooks.go @@ -16,7 +16,6 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.21.0" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" - "net/http" "slices" "time" ) @@ -44,7 +43,7 @@ type engineLoaderHooksRequestContext struct { type JointHooks interface { resolve.LoaderHooks - resolve.HttpLoaderHooks + //resolve.HttpLoaderHooks } func NewEngineRequestHooks(metricStore metric.Store, logger *requestlogger.SubgraphAccessLogger) JointHooks { @@ -83,7 +82,7 @@ func (f *engineLoaderHooks) OnLoad(ctx context.Context, ds resolve.DataSourceInf }) } -func (f *engineLoaderHooks) OnFinished(ctx context.Context, statusCode int, ds resolve.DataSourceInfo, err error) { +func (f *engineLoaderHooks) OnFinished(ctx context.Context, ds resolve.DataSourceInfo, responseInfo *resolve.ResponseInfo) { if resolve.IsIntrospectionDataSource(ds.ID) { return @@ -106,7 +105,7 @@ func (f *engineLoaderHooks) OnFinished(ctx context.Context, statusCode int, ds r defer span.End() commonAttrs := []attribute.KeyValue{ - semconv.HTTPStatusCode(statusCode), + semconv.HTTPStatusCode(responseInfo.StatusCode), rotel.WgSubgraphID.String(ds.ID), rotel.WgSubgraphName.String(ds.Name), } @@ -124,15 +123,37 @@ func (f *engineLoaderHooks) OnFinished(ctx context.Context, statusCode int, ds r metricAddOpt := otelmetric.WithAttributeSet(attribute.NewSet(metricAttrs...)) - if err != nil { + if f.accessLogger != nil { + fields := []zap.Field{ + zap.String("subgraph_name", ds.Name), + zap.String("subgraph_id", ds.ID), + zap.Int("status", responseInfo.StatusCode), + } + if responseInfo.Err != nil { + fields = append(fields, zap.Any("error", responseInfo.Err)) + } + if ctx != nil { + hookCtx, ok := ctx.Value(engineLoaderHooksContextKey).(*engineLoaderHooksRequestContext) + if !ok { + return + } + + latency := time.Since(hookCtx.startTime) + fields = append(fields, zap.Duration("latency", latency)) + } + + f.accessLogger.WriteRequestLog(responseInfo, fields) + } + + if responseInfo.Err != nil { // Set error status. This is the fetch error from the engine // Downstream errors are extracted from the subgraph response - span.SetStatus(codes.Error, err.Error()) - span.RecordError(err) + span.SetStatus(codes.Error, responseInfo.Err.Error()) + span.RecordError(responseInfo.Err) var errorCodesAttr []string - if unwrapped, ok := err.(multiError); ok { + if unwrapped, ok := responseInfo.Err.(multiError); ok { errs := unwrapped.Unwrap() for _, e := range errs { var subgraphError *resolve.SubgraphError @@ -188,31 +209,3 @@ func (f *engineLoaderHooks) OnFinished(ctx context.Context, statusCode int, ds r span.SetAttributes(traceAttrs...) } - -func (f *engineLoaderHooks) OnHttpFinished(ctx context.Context, ds resolve.DataSourceInfo, err error, request *http.Request, response *http.Response) { - if resolve.IsIntrospectionDataSource(ds.ID) { - return - } - - if f.accessLogger != nil { - fields := []zap.Field{ - zap.String("subgraph_name", ds.Name), - zap.String("subgraph_id", ds.ID), - zap.Int("status", response.StatusCode), - } - if err != nil { - fields = append(fields, zap.Any("error", err)) - } - if ctx != nil { - hookCtx, ok := ctx.Value(engineLoaderHooksContextKey).(*engineLoaderHooksRequestContext) - if !ok { - return - } - - latency := time.Since(hookCtx.startTime) - fields = append(fields, zap.Duration("latency", latency)) - } - - f.accessLogger.WriteRequestLog(request, response, fields) - } -} diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 63bb758a4..31f6b80df 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -760,7 +760,6 @@ func (s *graphServer) buildGraphMux(ctx context.Context, Authorizer: NewCosmoAuthorizer(authorizerOptions), SubgraphErrorPropagation: s.subgraphErrorPropagation, EngineLoaderHooks: hooks, - HttpLoaderHooks: hooks, } if s.redisClient != nil { @@ -875,7 +874,7 @@ func (s *graphServer) buildGraphMux(ctx context.Context, return gm, nil } -func (s *graphServer) accessLogsFieldHandler(attributes []config.CustomAttribute, panicError any, request *http.Request, response *http.Response) []zapcore.Field { +func (s *graphServer) accessLogsFieldHandler(attributes []config.CustomAttribute, panicError any, request *http.Request, responseHeader *http.Header) []zapcore.Field { reqContext := getRequestContext(request.Context()) if reqContext == nil { return nil @@ -884,8 +883,8 @@ func (s *graphServer) accessLogsFieldHandler(attributes []config.CustomAttribute resFields = append(resFields, logging.WithRequestID(middleware.GetReqID(request.Context()))) for _, field := range attributes { - if field.ValueFrom != nil && field.ValueFrom.ResponseHeader != "" && response != nil { - resFields = append(resFields, NewStringLogField(response.Header.Get(field.ValueFrom.ResponseHeader), field)) + if field.ValueFrom != nil && field.ValueFrom.ResponseHeader != "" && responseHeader != nil { + resFields = append(resFields, NewStringLogField(responseHeader.Get(field.ValueFrom.ResponseHeader), field)) } else if field.ValueFrom != nil && field.ValueFrom.RequestHeader != "" { resFields = append(resFields, NewStringLogField(request.Header.Get(field.ValueFrom.RequestHeader), field)) } else if field.ValueFrom != nil && field.ValueFrom.ContextField != "" && reqContext.operation != nil { diff --git a/router/core/graphql_handler.go b/router/core/graphql_handler.go index a32bc8c44..617284ddb 100644 --- a/router/core/graphql_handler.go +++ b/router/core/graphql_handler.go @@ -76,7 +76,6 @@ type HandlerOptions struct { RateLimitConfig *config.RateLimitConfiguration SubgraphErrorPropagation config.SubgraphErrorPropagationConfiguration EngineLoaderHooks resolve.LoaderHooks - HttpLoaderHooks resolve.HttpLoaderHooks } func NewGraphQLHandler(opts HandlerOptions) *GraphQLHandler { @@ -97,7 +96,6 @@ func NewGraphQLHandler(opts HandlerOptions) *GraphQLHandler { rateLimitConfig: opts.RateLimitConfig, subgraphErrorPropagation: opts.SubgraphErrorPropagation, engineLoaderHooks: opts.EngineLoaderHooks, - httpLoaderHooks: opts.HttpLoaderHooks, } return graphQLHandler } @@ -123,7 +121,6 @@ type GraphQLHandler struct { rateLimitConfig *config.RateLimitConfiguration subgraphErrorPropagation config.SubgraphErrorPropagationConfiguration engineLoaderHooks resolve.LoaderHooks - httpLoaderHooks resolve.HttpLoaderHooks enableExecutionPlanCacheResponseHeader bool enablePersistedOperationCacheResponseHeader bool @@ -162,10 +159,6 @@ func (h *GraphQLHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ctx.SetEngineLoaderHooks(h.engineLoaderHooks) } - if h.httpLoaderHooks != nil { - ctx.SetEngineHttpLoaderHooks(h.httpLoaderHooks) - } - ctx = h.configureRateLimiting(ctx) switch p := requestContext.operation.preparedPlan.preparedPlan.(type) { diff --git a/router/internal/requestlogger/requestlogger.go b/router/internal/requestlogger/requestlogger.go index 16f983da1..9060ce4af 100644 --- a/router/internal/requestlogger/requestlogger.go +++ b/router/internal/requestlogger/requestlogger.go @@ -17,7 +17,7 @@ import ( "go.uber.org/zap/zapcore" ) -type ContextFunc func(fields []config.CustomAttribute, panic any, r *http.Request, re *http.Response) []zapcore.Field +type ContextFunc func(fields []config.CustomAttribute, panic any, r *http.Request, rh *http.Header) []zapcore.Field // Option provides a functional approach to define // configuration for a handler; such as setting the logging @@ -169,6 +169,10 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func (al accessLogger) getRequestFields(r *http.Request) []zapcore.Field { + if r == nil { + return al.baseFields + } + start := time.Now() url := r.URL path := url.Path diff --git a/router/internal/requestlogger/subgraphlogger.go b/router/internal/requestlogger/subgraphlogger.go index 76a4f63b9..0d4c25246 100644 --- a/router/internal/requestlogger/subgraphlogger.go +++ b/router/internal/requestlogger/subgraphlogger.go @@ -2,9 +2,9 @@ package requestlogger import ( "github.com/wundergraph/cosmo/router/pkg/config" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" "go.uber.org/zap" "go.uber.org/zap/zapcore" - "net/http" ) type accessLogger struct { @@ -43,11 +43,11 @@ func NewSubgraphAccessLogger(logger *zap.Logger, opts SubgraphOptions) *Subgraph } } -func (h *SubgraphAccessLogger) WriteRequestLog(r *http.Request, rs *http.Response, subgraphFields []zap.Field) { - path := r.URL.Path - fields := h.accessLogger.getRequestFields(r) +func (h *SubgraphAccessLogger) WriteRequestLog(respInfo *resolve.ResponseInfo, subgraphFields []zap.Field) { + path := respInfo.Request.URL.Path + fields := h.accessLogger.getRequestFields(respInfo.Request) if h.accessLogger.fieldsHandler != nil { - fields = append(fields, h.accessLogger.fieldsHandler(h.accessLogger.attributes, nil, r, rs)...) + fields = append(fields, h.accessLogger.fieldsHandler(h.accessLogger.attributes, nil, respInfo.Request, &respInfo.ResponseHeaders)...) } fields = append(subgraphFields, fields...)