diff --git a/pilot/pkg/model/telemetry.go b/pilot/pkg/model/telemetry.go index 2e8c1f829d59..c29125adfb1a 100644 --- a/pilot/pkg/model/telemetry.go +++ b/pilot/pkg/model/telemetry.go @@ -269,7 +269,7 @@ func (t *Telemetries) AccessLogging(push *PushContext, proxy *Proxy, class netwo Disabled: v.Disabled, } - al := telemetryAccessLog(push, fp) + al := telemetryAccessLog(push, proxy, fp) if al == nil { // stackdriver will be handled in HTTPFilters/TCPFilters continue diff --git a/pilot/pkg/model/telemetry_logging.go b/pilot/pkg/model/telemetry_logging.go index c19c3c357914..9190fa57ea1a 100644 --- a/pilot/pkg/model/telemetry_logging.go +++ b/pilot/pkg/model/telemetry_logging.go @@ -110,6 +110,8 @@ var ( // We need to propagate these as part of access log service stream // Logging them by default on the console may be an issue as the base64 encoded string is bound to be a big one. // But end users can certainly configure it on their own via the meshConfig using the %FILTER_STATE% macro. + // Related to https://github.com/istio/proxy/pull/5825, start from 1.24, Istio use new filter state key. + envoyStateToLog = []string{"upstream_peer", "downstream_peer"} envoyWasmStateToLog = []string{"wasm.upstream_peer", "wasm.upstream_peer_id", "wasm.downstream_peer", "wasm.downstream_peer_id"} // reqWithoutQueryFormatter configures additional formatters needed for some of the format strings like "REQ_WITHOUT_QUERY" @@ -138,7 +140,7 @@ var ( // STOP. DO NOT UPDATE THIS WITHOUT UPDATING telemetryAccessLog. const telemetryAccessLogHandled = 14 -func telemetryAccessLog(push *PushContext, fp *meshconfig.MeshConfig_ExtensionProvider) *accesslog.AccessLog { +func telemetryAccessLog(push *PushContext, proxy *Proxy, fp *meshconfig.MeshConfig_ExtensionProvider) *accesslog.AccessLog { var al *accesslog.AccessLog switch prov := fp.Provider.(type) { case *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLog: @@ -149,11 +151,11 @@ func telemetryAccessLog(push *PushContext, fp *meshconfig.MeshConfig_ExtensionPr al = fileAccessLogFromTelemetry(prov.EnvoyFileAccessLog) } case *meshconfig.MeshConfig_ExtensionProvider_EnvoyHttpAls: - al = httpGrpcAccessLogFromTelemetry(push, prov.EnvoyHttpAls) + al = httpGrpcAccessLogFromTelemetry(push, proxy, prov.EnvoyHttpAls) case *meshconfig.MeshConfig_ExtensionProvider_EnvoyTcpAls: - al = tcpGrpcAccessLogFromTelemetry(push, prov.EnvoyTcpAls) + al = tcpGrpcAccessLogFromTelemetry(push, proxy, prov.EnvoyTcpAls) case *meshconfig.MeshConfig_ExtensionProvider_EnvoyOtelAls: - al = openTelemetryLog(push, prov.EnvoyOtelAls) + al = openTelemetryLog(push, proxy, prov.EnvoyOtelAls) case *meshconfig.MeshConfig_ExtensionProvider_EnvoyExtAuthzHttp, *meshconfig.MeshConfig_ExtensionProvider_EnvoyExtAuthzGrpc, *meshconfig.MeshConfig_ExtensionProvider_Zipkin, @@ -172,13 +174,23 @@ func telemetryAccessLog(push *PushContext, fp *meshconfig.MeshConfig_ExtensionPr return al } -func tcpGrpcAccessLogFromTelemetry(push *PushContext, prov *meshconfig.MeshConfig_ExtensionProvider_EnvoyTcpGrpcV3LogProvider) *accesslog.AccessLog { +func filterStateObjectsToLog(proxy *Proxy) []string { + if proxy.VersionGreaterAndEqual(&IstioVersion{Major: 1, Minor: 24, Patch: 0}) { + return envoyStateToLog + } + + return envoyWasmStateToLog +} + +func tcpGrpcAccessLogFromTelemetry(push *PushContext, proxy *Proxy, + prov *meshconfig.MeshConfig_ExtensionProvider_EnvoyTcpGrpcV3LogProvider, +) *accesslog.AccessLog { logName := TCPEnvoyAccessLogFriendlyName if prov != nil && prov.LogName != "" { logName = prov.LogName } - filterObjects := envoyWasmStateToLog + filterObjects := filterStateObjectsToLog(proxy) if len(prov.FilterStateObjectsToLog) != 0 { filterObjects = prov.FilterStateObjectsToLog } @@ -332,13 +344,15 @@ func accessLogTextFormatters(text string) []*core.TypedExtensionConfig { return formatters } -func httpGrpcAccessLogFromTelemetry(push *PushContext, prov *meshconfig.MeshConfig_ExtensionProvider_EnvoyHttpGrpcV3LogProvider) *accesslog.AccessLog { +func httpGrpcAccessLogFromTelemetry(push *PushContext, proxy *Proxy, + prov *meshconfig.MeshConfig_ExtensionProvider_EnvoyHttpGrpcV3LogProvider, +) *accesslog.AccessLog { logName := HTTPEnvoyAccessLogFriendlyName if prov != nil && prov.LogName != "" { logName = prov.LogName } - filterObjects := envoyWasmStateToLog + filterObjects := filterStateObjectsToLog(proxy) if len(prov.FilterStateObjectsToLog) != 0 { filterObjects = prov.FilterStateObjectsToLog } @@ -443,7 +457,7 @@ func FileAccessLogFromMeshConfig(path string, mesh *meshconfig.MeshConfig) *acce return al } -func openTelemetryLog(pushCtx *PushContext, +func openTelemetryLog(pushCtx *PushContext, proxy *Proxy, provider *meshconfig.MeshConfig_ExtensionProvider_EnvoyOpenTelemetryLogProvider, ) *accesslog.AccessLog { hostname, cluster, err := clusterLookupFn(pushCtx, provider.Service, int(provider.Port)) @@ -468,7 +482,7 @@ func openTelemetryLog(pushCtx *PushContext, labels = provider.LogFormat.Labels } - cfg := buildOpenTelemetryAccessLogConfig(logName, hostname, cluster, f, labels) + cfg := buildOpenTelemetryAccessLogConfig(proxy, logName, hostname, cluster, f, labels) return &accesslog.AccessLog{ Name: OtelEnvoyALSName, @@ -476,7 +490,9 @@ func openTelemetryLog(pushCtx *PushContext, } } -func buildOpenTelemetryAccessLogConfig(logName, hostname, clusterName, format string, labels *structpb.Struct) *otelaccesslog.OpenTelemetryAccessLogConfig { +func buildOpenTelemetryAccessLogConfig(proxy *Proxy, + logName, hostname, clusterName, format string, labels *structpb.Struct, +) *otelaccesslog.OpenTelemetryAccessLogConfig { cfg := &otelaccesslog.OpenTelemetryAccessLogConfig{ CommonConfig: &grpcaccesslog.CommonGrpcAccessLogConfig{ LogName: logName, @@ -489,7 +505,7 @@ func buildOpenTelemetryAccessLogConfig(logName, hostname, clusterName, format st }, }, TransportApiVersion: core.ApiVersion_V3, - FilterStateObjectsToLog: envoyWasmStateToLog, + FilterStateObjectsToLog: filterStateObjectsToLog(proxy), }, DisableBuiltinLabels: true, } diff --git a/pilot/pkg/model/telemetry_logging_test.go b/pilot/pkg/model/telemetry_logging_test.go index f831d3951544..c04380941c76 100644 --- a/pilot/pkg/model/telemetry_logging_test.go +++ b/pilot/pkg/model/telemetry_logging_test.go @@ -959,6 +959,13 @@ func TestAccessLoggingCache(t *testing.T) { } func TestBuildOpenTelemetryAccessLogConfig(t *testing.T) { + sidecar := &Proxy{ + ConfigNamespace: "default", + Labels: map[string]string{"app": "test"}, + Metadata: &NodeMetadata{}, + IstioVersion: &IstioVersion{Major: 1, Minor: 23}, + } + fakeCluster := "outbound|55680||otel-collector.monitoring.svc.cluster.local" fakeAuthority := "otel-collector.monitoring.svc.cluster.local" for _, tc := range []struct { @@ -1042,7 +1049,7 @@ func TestBuildOpenTelemetryAccessLogConfig(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - got := buildOpenTelemetryAccessLogConfig(tc.logName, tc.hostname, tc.clusterName, tc.body, tc.labels) + got := buildOpenTelemetryAccessLogConfig(sidecar, tc.logName, tc.hostname, tc.clusterName, tc.body, tc.labels) assert.Equal(t, tc.expected, got) }) } @@ -1430,6 +1437,13 @@ func TestTelemetryAccessLog(t *testing.T) { }, } + sidecar := &Proxy{ + ConfigNamespace: "default", + Labels: map[string]string{"app": "test"}, + Metadata: &NodeMetadata{}, + IstioVersion: &IstioVersion{Major: 1, Minor: 23}, + } + for _, tc := range []struct { name string ctx *PushContext @@ -1597,7 +1611,7 @@ func TestTelemetryAccessLog(t *testing.T) { } push.Mesh = tc.meshConfig - got := telemetryAccessLog(push, tc.fp) + got := telemetryAccessLog(push, sidecar, tc.fp) if got == nil { t.Fatal("get nil accesslog") } @@ -1912,3 +1926,36 @@ func TestAccessLogFormatters(t *testing.T) { }) } } + +func TestFilterStateObjectsToLog(t *testing.T) { + cases := []struct { + proxy *Proxy + expected []string + }{ + { + proxy: &Proxy{ + IstioVersion: &IstioVersion{Major: 1, Minor: 23}, + }, + expected: []string{"wasm.upstream_peer", "wasm.upstream_peer_id", "wasm.downstream_peer", "wasm.downstream_peer_id"}, + }, + { + proxy: &Proxy{ + IstioVersion: &IstioVersion{Major: 1, Minor: 24}, + }, + expected: []string{"upstream_peer", "downstream_peer"}, + }, + { + proxy: &Proxy{ + IstioVersion: &IstioVersion{Major: 1, Minor: 25}, + }, + expected: []string{"upstream_peer", "downstream_peer"}, + }, + } + + for _, tc := range cases { + t.Run("", func(t *testing.T) { + got := filterStateObjectsToLog(tc.proxy) + assert.Equal(t, tc.expected, got) + }) + } +} diff --git a/pilot/pkg/networking/core/accesslog.go b/pilot/pkg/networking/core/accesslog.go index b068a1fb2ef6..96baf8fbbf4f 100644 --- a/pilot/pkg/networking/core/accesslog.go +++ b/pilot/pkg/networking/core/accesslog.go @@ -49,7 +49,8 @@ var ( // We need to propagate these as part of access log service stream // Logging them by default on the console may be an issue as the base64 encoded string is bound to be a big one. // But end users can certainly configure it on their own via the meshConfig using the %FILTER_STATE% macro. - envoyWasmStateToLog = []string{"wasm.upstream_peer", "wasm.upstream_peer_id", "wasm.downstream_peer", "wasm.downstream_peer_id"} + envoyWasmStateToLog = []string{"upstream_peer", "downstream_peer", // start from 1.24.0 + "wasm.upstream_peer", "wasm.upstream_peer_id", "wasm.downstream_peer", "wasm.downstream_peer_id"} // accessLogBuilder is used to set accessLog to filters accessLogBuilder = newAccessLogBuilder() diff --git a/releasenotes/notes/53778.yaml b/releasenotes/notes/53778.yaml new file mode 100644 index 000000000000..ab6d9cff5c4e --- /dev/null +++ b/releasenotes/notes/53778.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: telemetry +releaseNotes: + - | + **Fixed** an issue that should use `upstream_peer` instead of `wasm.upstream_peer` in `filterStateObjectsToLog`.