diff --git a/cmd/activator/request_log.go b/cmd/activator/request_log.go index 1ae912a1f18c..75930f51273d 100644 --- a/cmd/activator/request_log.go +++ b/cmd/activator/request_log.go @@ -21,6 +21,7 @@ import ( "go.uber.org/zap" corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/metrics" "knative.dev/serving/pkg/activator" "knative.dev/serving/pkg/apis/serving" servinglisters "knative.dev/serving/pkg/client/listers/serving/v1" @@ -29,7 +30,16 @@ import ( func updateRequestLogFromConfigMap(logger *zap.SugaredLogger, h *pkghttp.RequestLogHandler) func(configMap *corev1.ConfigMap) { return func(configMap *corev1.ConfigMap) { - newTemplate := configMap.Data["logging.request-log-template"] + obsconfig, err := metrics.NewObservabilityConfigFromConfigMap(configMap) + if err != nil { + logger.Errorw("Failed to get observability configmap.", zap.Error(err), "configmap", configMap) + return + } + + var newTemplate string + if obsconfig.EnableRequestLog { + newTemplate = obsconfig.RequestLogTemplate + } if err := h.SetTemplate(newTemplate); err != nil { logger.Errorw("Failed to update the request log template.", zap.Error(err), "template", newTemplate) } else { diff --git a/cmd/activator/request_log_test.go b/cmd/activator/request_log_test.go index 10dcab74543f..4071a57ce1a7 100644 --- a/cmd/activator/request_log_test.go +++ b/cmd/activator/request_log_test.go @@ -56,48 +56,85 @@ func TestUpdateRequestLogFromConfigMap(t *testing.T) { } tests := []struct { - name string - url string - body string - template string - want string + name string + url string + body string + data map[string]string + want string }{{ - name: "empty template", - url: "http://example.com/testpage", - body: "test", - template: "", - want: "", + name: "empty template", + url: "http://example.com/testpage", + body: "test", + data: map[string]string{ + "logging.request-log-template": "", + }, + want: "", + }, { + name: "template with new line", + url: "http://example.com/testpage", + body: "test", + data: map[string]string{ + "logging.request-log-template": "{{.Request.URL}}\n", + }, + want: "http://example.com/testpage\n", + }, { + name: "invalid template", + url: "http://example.com", + body: "test", + data: map[string]string{ + "logging.request-log-template": "{{}}", + }, + want: "http://example.com\n", }, { - name: "template with new line", - url: "http://example.com/testpage", - body: "test", - template: "{{.Request.URL}}\n", - want: "http://example.com/testpage\n", + name: "revision info", + url: "http://example.com", + body: "test", + data: map[string]string{ + "logging.request-log-template": "{{.Revision.Name}}, {{.Revision.Namespace}}, {{.Revision.Service}}, {{.Revision.Configuration}}, {{.Revision.PodName}}, {{.Revision.PodIP}}", + }, + want: "testRevision, testNs, testSvc, testConfig, , \n", + }, { + name: "empty template 2", + url: "http://example.com/testpage", + body: "test", + data: map[string]string{ + "logging.request-log-template": "", + }, + want: "", }, { - name: "invalid template", - url: "http://example.com", - body: "test", - template: "{{}}", - want: "http://example.com\n", + name: "explicitly enable request logging", + url: "http://example.com/testpage", + body: "test", + data: map[string]string{ + "logging.request-log-template": "{{.Request.URL}}\n", + "logging.enable-request-log": "true", + }, + want: "http://example.com/testpage\n", }, { - name: "revision info", - url: "http://example.com", - body: "test", - template: "{{.Revision.Name}}, {{.Revision.Namespace}}, {{.Revision.Service}}, {{.Revision.Configuration}}, {{.Revision.PodName}}, {{.Revision.PodIP}}", - want: "testRevision, testNs, testSvc, testConfig, , \n", + name: "explicitly disable request logging", + url: "http://example.com/testpage", + body: "test", + data: map[string]string{ + "logging.request-log-template": "disable_request_log", + "logging.enable-request-log": "false", + }, + want: "", }, { - name: "empty template 2", - url: "http://example.com/testpage", - body: "test", - template: "", - want: "", + name: "explicitly enable request logging with empty template", + url: "http://example.com/testpage", + body: "test", + data: map[string]string{ + "logging.request-log-template": "", + "logging.enable-request-log": "true", + }, + want: "", }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { buf.Reset() cm := &corev1.ConfigMap{} - cm.Data = map[string]string{"logging.request-log-template": test.template} + cm.Data = test.data (updateRequestLogFromConfigMap(testing2.TestLogger(t), handler))(cm) resp := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, test.url, bytes.NewBufferString(test.body)) diff --git a/cmd/queue/main.go b/cmd/queue/main.go index dfbbee5b3db4..2cd95d4e6987 100644 --- a/cmd/queue/main.go +++ b/cmd/queue/main.go @@ -82,6 +82,7 @@ type config struct { ServingLoggingConfig string `split_words:"true" required:"true"` ServingLoggingLevel string `split_words:"true" required:"true"` ServingRequestLogTemplate string `split_words:"true"` // optional + ServingEnableRequestLog bool `split_words:"true"` // optional ServingEnableProbeRequestLog bool `split_words:"true"` // optional // Metrics configuration @@ -432,7 +433,7 @@ func buildMetricsServer(promStatReporter *queue.PrometheusStatsReporter, protobu } func pushRequestLogHandler(currentHandler http.Handler, env config) http.Handler { - if env.ServingRequestLogTemplate == "" { + if !env.ServingEnableRequestLog { return currentHandler } diff --git a/pkg/reconciler/revision/resources/deploy_test.go b/pkg/reconciler/revision/resources/deploy_test.go index ea04bc35d5d2..a46293aa4750 100644 --- a/pkg/reconciler/revision/resources/deploy_test.go +++ b/pkg/reconciler/revision/resources/deploy_test.go @@ -135,6 +135,9 @@ var ( }, { Name: "SERVING_REQUEST_LOG_TEMPLATE", Value: "", + }, { + Name: "SERVING_ENABLE_REQUEST_LOG", + Value: "false", }, { Name: "SERVING_REQUEST_METRICS_BACKEND", Value: "", diff --git a/pkg/reconciler/revision/resources/queue.go b/pkg/reconciler/revision/resources/queue.go index a4c16de51593..e215ade5ff63 100644 --- a/pkg/reconciler/revision/resources/queue.go +++ b/pkg/reconciler/revision/resources/queue.go @@ -311,6 +311,9 @@ func makeQueueContainer(rev *v1.Revision, loggingConfig *logging.Config, tracing }, { Name: "SERVING_REQUEST_LOG_TEMPLATE", Value: observabilityConfig.RequestLogTemplate, + }, { + Name: "SERVING_ENABLE_REQUEST_LOG", + Value: strconv.FormatBool(observabilityConfig.EnableRequestLog), }, { Name: "SERVING_REQUEST_METRICS_BACKEND", Value: observabilityConfig.RequestMetricsBackend, diff --git a/pkg/reconciler/revision/resources/queue_test.go b/pkg/reconciler/revision/resources/queue_test.go index e261c16f6832..8d2409a5e7ce 100644 --- a/pkg/reconciler/revision/resources/queue_test.go +++ b/pkg/reconciler/revision/resources/queue_test.go @@ -199,6 +199,23 @@ func TestMakeQueueContainer(t *testing.T) { "SERVING_ENABLE_PROBE_REQUEST_LOG": "true", }) }), + }, { + name: "disabled request log configuration as env var", + rev: revision("bar", "foo", + withContainers(containers), + withContainerConcurrency(1)), + oc: metrics.ObservabilityConfig{ + RequestLogTemplate: "test template", + EnableProbeRequestLog: false, + EnableRequestLog: false, + }, + want: queueContainer(func(c *corev1.Container) { + c.Env = env(map[string]string{ + "SERVING_REQUEST_LOG_TEMPLATE": "test template", + "SERVING_ENABLE_REQUEST_LOG": "false", + "SERVING_ENABLE_PROBE_REQUEST_LOG": "false", + }) + }), }, { name: "request metrics backend as env var", rev: revision("bar", "foo", @@ -756,6 +773,7 @@ var defaultEnv = map[string]string{ "TRACING_CONFIG_SAMPLE_RATE": "0", "TRACING_CONFIG_DEBUG": "false", "SERVING_REQUEST_LOG_TEMPLATE": "", + "SERVING_ENABLE_REQUEST_LOG": "false", "SERVING_REQUEST_METRICS_BACKEND": "", "USER_PORT": strconv.Itoa(v1.DefaultUserPort), "SYSTEM_NAMESPACE": system.Namespace(),