diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 98d09740ea29..18e6af6f8f06 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -175,7 +175,7 @@ func main() { // Create activation handler chain // Note: innermost handlers are specified first, ie. the last handler in the chain will be executed first - var ah http.Handler = activatorhandler.New(ctx, throttler, transport) + var ah http.Handler = activatorhandler.New(ctx, throttler, transport, logger) ah = concurrencyReporter.Handler(ah) ah = tracing.HTTPSpanMiddleware(ah) ah = configStore.HTTPMiddleware(ah) diff --git a/pkg/activator/handler/context_handler.go b/pkg/activator/handler/context_handler.go index e49676927a77..ac2ae18fef87 100644 --- a/pkg/activator/handler/context_handler.go +++ b/pkg/activator/handler/context_handler.go @@ -65,17 +65,15 @@ func (h *contextHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } revID := types.NamespacedName{Namespace: namespace, Name: name} - logger := h.logger.With(zap.String(logkey.Key, revID.String())) revision, err := h.revisionLister.Revisions(namespace).Get(name) if err != nil { - logger.Errorw("Error while getting revision", zap.Error(err)) + h.logger.Errorw("Error while getting revision", zap.String(logkey.Key, revID.String()), zap.Error(err)) sendError(err, w) return } ctx := r.Context() - ctx = logging.WithLogger(ctx, logger) ctx = WithRevision(ctx, revision) ctx = WithRevID(ctx, revID) diff --git a/pkg/activator/handler/handler.go b/pkg/activator/handler/handler.go index 5cf0857673bf..4e39a17e90a8 100644 --- a/pkg/activator/handler/handler.go +++ b/pkg/activator/handler/handler.go @@ -28,7 +28,7 @@ import ( "k8s.io/apimachinery/pkg/types" network "knative.dev/networking/pkg" - "knative.dev/pkg/logging" + "knative.dev/pkg/logging/logkey" pkgnet "knative.dev/pkg/network" tracingconfig "knative.dev/pkg/tracing/config" "knative.dev/pkg/tracing/propagation/tracecontextb3" @@ -50,10 +50,11 @@ type activationHandler struct { tracingTransport http.RoundTripper throttler Throttler bufferPool httputil.BufferPool + logger *zap.SugaredLogger } // New constructs a new http.Handler that deals with revision activation. -func New(_ context.Context, t Throttler, transport http.RoundTripper) http.Handler { +func New(_ context.Context, t Throttler, transport http.RoundTripper, logger *zap.SugaredLogger) http.Handler { return &activationHandler{ transport: transport, tracingTransport: &ochttp.Transport{ @@ -62,11 +63,11 @@ func New(_ context.Context, t Throttler, transport http.RoundTripper) http.Handl }, throttler: t, bufferPool: network.NewBufferPool(), + logger: logger, } } func (a *activationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - logger := logging.FromContext(r.Context()) tracingEnabled := activatorconfig.FromContext(r.Context()).Tracing.Backend != tracingconfig.None tryContext, trySpan := r.Context(), (*trace.Span)(nil) @@ -74,14 +75,15 @@ func (a *activationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { tryContext, trySpan = trace.StartSpan(r.Context(), "throttler_try") } - if err := a.throttler.Try(tryContext, RevIDFrom(r.Context()), func(dest string) error { + revID := RevIDFrom(r.Context()) + if err := a.throttler.Try(tryContext, revID, func(dest string) error { trySpan.End() proxyCtx, proxySpan := r.Context(), (*trace.Span)(nil) if tracingEnabled { proxyCtx, proxySpan = trace.StartSpan(r.Context(), "activator_proxy") } - a.proxyRequest(logger, w, r.WithContext(proxyCtx), dest, tracingEnabled) + a.proxyRequest(revID, w, r.WithContext(proxyCtx), dest, tracingEnabled) proxySpan.End() return nil @@ -90,7 +92,7 @@ func (a *activationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { trySpan.Annotate([]trace.Attribute{trace.StringAttribute("activator.throttler.error", err.Error())}, "ThrottlerTry") trySpan.End() - logger.Errorw("Throttler try error", zap.Error(err)) + a.logger.Errorw("Throttler try error", zap.String(logkey.Key, revID.String()), zap.Error(err)) if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, queue.ErrRequestQueueFull) { http.Error(w, err.Error(), http.StatusServiceUnavailable) @@ -100,7 +102,7 @@ func (a *activationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } -func (a *activationHandler) proxyRequest(logger *zap.SugaredLogger, w http.ResponseWriter, r *http.Request, target string, tracingEnabled bool) { +func (a *activationHandler) proxyRequest(revID types.NamespacedName, w http.ResponseWriter, r *http.Request, target string, tracingEnabled bool) { network.RewriteHostIn(r) r.Header.Set(network.ProxyHeaderName, activator.Name) @@ -112,7 +114,9 @@ func (a *activationHandler) proxyRequest(logger *zap.SugaredLogger, w http.Respo proxy.Transport = a.tracingTransport } proxy.FlushInterval = network.FlushInterval - proxy.ErrorHandler = pkgnet.ErrorHandler(logger) + proxy.ErrorHandler = func(w http.ResponseWriter, req *http.Request, err error) { + pkgnet.ErrorHandler(a.logger.With(zap.String(logkey.Key, revID.String())))(w, req, err) + } proxy.ServeHTTP(w, r) } diff --git a/pkg/activator/handler/handler_test.go b/pkg/activator/handler/handler_test.go index a706b4b4e189..b4efc33706b4 100644 --- a/pkg/activator/handler/handler_test.go +++ b/pkg/activator/handler/handler_test.go @@ -125,7 +125,7 @@ func TestActivationHandler(t *testing.T) { ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(t) defer cancel() - handler := New(ctx, test.throttler, rt) + handler := New(ctx, test.throttler, rt, logging.FromContext(ctx)) resp := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) @@ -164,7 +164,7 @@ func TestActivationHandlerProxyHeader(t *testing.T) { ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(t) defer cancel() - handler := New(ctx, fakeThrottler{}, rt) + handler := New(ctx, fakeThrottler{}, rt, logging.FromContext(ctx)) writer := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "http://example.com", nil) @@ -241,7 +241,7 @@ func TestActivationHandlerTraceSpans(t *testing.T) { oct.Finish() }() - handler := New(ctx, fakeThrottler{}, rt) + handler := New(ctx, fakeThrottler{}, rt, logging.FromContext(ctx)) // Set up config store to populate context. configStore := setupConfigStore(t, logging.FromContext(ctx)) @@ -311,7 +311,7 @@ func BenchmarkHandler(b *testing.B) { }, nil }) - handler := New(ctx, fakeThrottler{}, rt) + handler := New(ctx, fakeThrottler{}, rt, logging.FromContext(ctx)) request := func() *http.Request { req := httptest.NewRequest(http.MethodGet, "http://example.com", nil)