diff --git a/internal/xds/translator/accesslog.go b/internal/xds/translator/accesslog.go index 991c8e342c..304f22da49 100644 --- a/internal/xds/translator/accesslog.go +++ b/internal/xds/translator/accesslog.go @@ -336,10 +336,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) ([ ResourceAttributes: convertToKeyValueList(otel.Resources, false), } - if otel.Text == nil { - return nil, errors.New("otel.Text is nil") + var format string + if otel.Text != nil && *otel.Text != "" { + format = *otel.Text } - format := *otel.Text if format != "" { al.Body = &otlpcommonv1.AnyValue{ @@ -349,9 +349,17 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) ([ } } - al.Attributes = convertToKeyValueList(otel.Attributes, true) + var attrs map[string]string + if len(otel.Attributes) != 0 { + attrs = otel.Attributes + } else if len(otel.Attributes) == 0 && format == "" { + // if there are no attributes and text format is unset, use the default EnvoyJSONLogFields + attrs = EnvoyJSONLogFields + } + + al.Attributes = convertToKeyValueList(attrs, true) + formatters := accessLogOpenTelemetryFormatters(format, attrs) - formatters := accessLogOpenTelemetryFormatters(format, otel.Attributes) if len(formatters) != 0 { al.Formatters = formatters } diff --git a/release-notes/current.yaml b/release-notes/current.yaml index b332fc8301..3b02988dcc 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -36,6 +36,7 @@ bug fixes: | Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. Retrigger reconciliation when backendRef of type ServiceImport is updated or when EndpointSlice(s) for a ServiceImport are updated. Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. + Fix allowing empty text field for opentelemetry sink when using JSON format. # Enhancements that improve performance. performance improvements: | diff --git a/test/e2e/testdata/accesslog-otel-default.yaml b/test/e2e/testdata/accesslog-otel-default.yaml new file mode 100644 index 0000000000..764e8e02e5 --- /dev/null +++ b/test/e2e/testdata/accesslog-otel-default.yaml @@ -0,0 +1,61 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: accesslog-gtw + namespace: gateway-conformance-infra +spec: + gatewayClassName: {GATEWAY_CLASS_NAME} + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: accesslog-otel +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyProxy +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + ipFamily: IPv4 + telemetry: + accessLog: + settings: + - matches: + - "'x-envoy-logged' in request.headers" + sinks: + - type: OpenTelemetry + openTelemetry: + backendRefs: + - name: otel-collector + namespace: monitoring + port: 4317 + resources: + k8s.cluster.name: "envoy-gateway" + shutdown: + drainTimeout: 5s + minDrainDuration: 1s +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: accesslog-gtw + rules: + - matches: + - path: + type: PathPrefix + value: /otel + backendRefs: + - name: infra-backend-v2 + port: 8080 diff --git a/test/e2e/testdata/accesslog-otel-json.yaml b/test/e2e/testdata/accesslog-otel-json.yaml new file mode 100644 index 0000000000..219315112c --- /dev/null +++ b/test/e2e/testdata/accesslog-otel-json.yaml @@ -0,0 +1,79 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: accesslog-gtw + namespace: gateway-conformance-infra +spec: + gatewayClassName: {GATEWAY_CLASS_NAME} + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: accesslog-otel +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyProxy +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + ipFamily: IPv4 + telemetry: + accessLog: + settings: + - format: + type: JSON + json: + time: "%START_TIME%" + method: "%REQ(:METHOD)%" + metadata: "%METADATA(ROUTE:envoy-gateway:resources)%" + x-envoy-original-path: "%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%" + protocol: "%PROTOCOL%" + responseCode: "%RESPONSE_CODE%" + responseFlags: "%RESPONSE_FLAGS%" + bytesReceived: "%BYTES_RECEIVED%" + bytesSent: "%BYTES_SENT%" + duration: "%DURATION%" + x-forwarded-for: "%REQ(X-FORWARDED-FOR)%" + user-agent: "%REQ(USER-AGENT)%" + x-request-id: "%REQ(X-REQUEST-ID)%" + authority: "%REQ(:AUTHORITY)%" + upstreamHost: "%UPSTREAM_HOST%" + matches: + - "'x-envoy-logged' in request.headers" + sinks: + - type: OpenTelemetry + openTelemetry: + backendRefs: + - name: otel-collector + namespace: monitoring + port: 4317 + resources: + k8s.cluster.name: "envoy-gateway" + shutdown: + drainTimeout: 5s + minDrainDuration: 1s +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: accesslog-gtw + rules: + - matches: + - path: + type: PathPrefix + value: /otel + backendRefs: + - name: infra-backend-v2 + port: 8080 diff --git a/test/e2e/tests/accesslog.go b/test/e2e/tests/accesslog.go index ea57734957..5df762868d 100644 --- a/test/e2e/tests/accesslog.go +++ b/test/e2e/tests/accesslog.go @@ -21,7 +21,7 @@ import ( ) func init() { - ConformanceTests = append(ConformanceTests, FileAccessLogTest, OpenTelemetryTest, ALSTest) + ConformanceTests = append(ConformanceTests, FileAccessLogTest, OpenTelemetryTestText, OpenTelemetryTestJSON, ALSTest, OpenTelemetryTestJSONAsDefault) } var FileAccessLogTest = suite.ConformanceTest{ @@ -116,9 +116,9 @@ var FileAccessLogTest = suite.ConformanceTest{ }, } -var OpenTelemetryTest = suite.ConformanceTest{ - ShortName: "OpenTelemetryAccessLog", - Description: "Make sure OpenTelemetry access log is working", +var OpenTelemetryTestText = suite.ConformanceTest{ + ShortName: "OpenTelemetryTextAccessLog", + Description: "Make sure OpenTelemetry text access log is working", Manifests: []string{"testdata/accesslog-otel.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { labels := map[string]string{ @@ -169,6 +169,112 @@ var OpenTelemetryTest = suite.ConformanceTest{ }, } +var OpenTelemetryTestJSONAsDefault = suite.ConformanceTest{ + ShortName: "OpenTelemetryAccessLogJSONAsDefault", + Description: "Make sure OpenTelemetry JSON access log is working as default when no format or type is specified", + Manifests: []string{"testdata/accesslog-otel-default.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + labels := map[string]string{ + "k8s_namespace_name": "envoy-gateway-system", + "exporter": "OTLP", + } + + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "accesslog-otel", Namespace: ns} + gwNN := types.NamespacedName{Name: "accesslog-gtw", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + t.Run("Positive", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + Headers: map[string]string{ + "x-envoy-logged": "1", + }, + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 1) + }) + + t.Run("Negative", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + // envoy will not log this request without the header x-envoy-logged + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 0) + }) + }, +} + +var OpenTelemetryTestJSON = suite.ConformanceTest{ + ShortName: "OpenTelemetryAccessLogJSON", + Description: "Make sure OpenTelemetry JSON access log is working with custom JSON attributes", + Manifests: []string{"testdata/accesslog-otel-json.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + labels := map[string]string{ + "k8s_namespace_name": "envoy-gateway-system", + "exporter": "OTLP", + } + + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "accesslog-otel", Namespace: ns} + gwNN := types.NamespacedName{Name: "accesslog-gtw", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + t.Run("Positive", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + Headers: map[string]string{ + "x-envoy-logged": "1", + }, + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 1) + }) + + t.Run("Negative", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + // envoy will not log this request without the header x-envoy-logged + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 0) + }) + }, +} + var ALSTest = suite.ConformanceTest{ ShortName: "ALS", Description: "Make sure ALS access log is working",