fix proxy version check for built-in formatters (#58469)

* fix proxy version check for built-in formatters

Signed-off-by: Rama Chavali <[email protected]>

* remove proxy version

Signed-off-by: Rama Chavali <[email protected]>

* lint

Signed-off-by: Rama Chavali <[email protected]>

---------

Signed-off-by: Rama Chavali <[email protected]>

Author: ramaraochavali

pilot/pkg/model/telemetry_logging.go

@@ -23,8 +23,6 @@ import (
 	fileaccesslog "github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/file/v3"
 	grpcaccesslog "github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/grpc/v3"
 	otelaccesslog "github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/open_telemetry/v3"
-	celformatter "github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/cel/v3"
-	metadataformatter "github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/metadata/v3"
 	reqwithoutquery "github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/req_without_query/v3"
 	otlpcommon "go.opentelemetry.io/proto/otlp/common/v1"
 	"google.golang.org/protobuf/types/known/structpb"
@@ -119,20 +117,6 @@ var (
 		Name:        "envoy.formatter.req_without_query",
 		TypedConfig: protoconv.MessageToAny(&reqwithoutquery.ReqWithoutQuery{}),
 	}
-
-	// metadataFormatter configures additional formatters needed for some of the format strings like "METADATA"
-	// for more information, see https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/formatter/metadata/v3/metadata.proto
-	metadataFormatter = &core.TypedExtensionConfig{
-		Name:        "envoy.formatter.metadata",
-		TypedConfig: protoconv.MessageToAny(&metadataformatter.Metadata{}),
-	}
-
-	// celFormatter configures additional formatters needed for some of the format strings like "CEL"
-	// for more information, see https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/formatter/cel/v3/cel.proto
-	celFormatter = &core.TypedExtensionConfig{
-		Name:        "envoy.formatter.cel",
-		TypedConfig: protoconv.MessageToAny(&celformatter.Cel{}),
-	}
 )
 
 // configureFromProviderConfigHandled contains the number of providers we handle below.
@@ -141,27 +125,22 @@ var (
 const telemetryAccessLogHandled = 15
 
 func telemetryAccessLog(push *PushContext, proxy *Proxy, fp *meshconfig.MeshConfig_ExtensionProvider) *accesslog.AccessLog {
-	// Skip built-in formatter if Istio version is >= 1.26
-	// This is because if we send CEL/METADATA in the access log,
-	// envoy will report lots of warn message endlessly.
-	skipBuiltInFormatter := proxy.IstioVersion != nil && proxy.VersionGreaterOrEqual(&IstioVersion{Major: 1, Minor: 26})
-
 	var al *accesslog.AccessLog
 	switch prov := fp.Provider.(type) {
 	case *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLog:
 		// For built-in provider, fallback to MeshConfig for formatting options when LogFormat unset.
 		if fp.Name == builtinEnvoyAccessLogProvider &&
 			prov.EnvoyFileAccessLog.LogFormat == nil && !prov.EnvoyFileAccessLog.OmitEmptyValues {
-			al = FileAccessLogFromMeshConfig(prov.EnvoyFileAccessLog.Path, push.Mesh, skipBuiltInFormatter)
+			al = FileAccessLogFromMeshConfig(prov.EnvoyFileAccessLog.Path, push.Mesh)
 		} else {
-			al = fileAccessLogFromTelemetry(prov.EnvoyFileAccessLog, skipBuiltInFormatter)
+			al = fileAccessLogFromTelemetry(prov.EnvoyFileAccessLog)
 		}
 	case *meshconfig.MeshConfig_ExtensionProvider_EnvoyHttpAls:
 		al = httpGrpcAccessLogFromTelemetry(push, proxy, prov.EnvoyHttpAls)
 	case *meshconfig.MeshConfig_ExtensionProvider_EnvoyTcpAls:
 		al = tcpGrpcAccessLogFromTelemetry(push, proxy, prov.EnvoyTcpAls)
 	case *meshconfig.MeshConfig_ExtensionProvider_EnvoyOtelAls:
-		al = openTelemetryLog(push, proxy, prov.EnvoyOtelAls, skipBuiltInFormatter)
+		al = openTelemetryLog(push, proxy, prov.EnvoyOtelAls)
 	case *meshconfig.MeshConfig_ExtensionProvider_EnvoyExtAuthzHttp,
 		*meshconfig.MeshConfig_ExtensionProvider_EnvoyExtAuthzGrpc,
 		*meshconfig.MeshConfig_ExtensionProvider_Zipkin,
@@ -231,7 +210,7 @@ func tcpGrpcAccessLogFromTelemetry(push *PushContext, proxy *Proxy,
 	}
 }
 
-func fileAccessLogFromTelemetry(prov *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLogProvider, skipBuiltInFormatter bool) *accesslog.AccessLog {
+func fileAccessLogFromTelemetry(prov *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLogProvider) *accesslog.AccessLog {
 	p := prov.Path
 	if p == "" {
 		p = DevStdout
@@ -244,12 +223,12 @@ func fileAccessLogFromTelemetry(prov *meshconfig.MeshConfig_ExtensionProvider_En
 	if prov.LogFormat != nil {
 		switch logFormat := prov.LogFormat.LogFormat.(type) {
 		case *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLogProvider_LogFormat_Text:
-			fl.AccessLogFormat, needsFormatter = buildFileAccessTextLogFormat(logFormat.Text, prov.OmitEmptyValues, skipBuiltInFormatter)
+			fl.AccessLogFormat, needsFormatter = buildFileAccessTextLogFormat(logFormat.Text, prov.OmitEmptyValues)
 		case *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLogProvider_LogFormat_Labels:
-			fl.AccessLogFormat, needsFormatter = buildFileAccessJSONLogFormat(logFormat, prov.OmitEmptyValues, skipBuiltInFormatter)
+			fl.AccessLogFormat, needsFormatter = buildFileAccessJSONLogFormat(logFormat, prov.OmitEmptyValues)
 		}
 	} else {
-		fl.AccessLogFormat, needsFormatter = buildFileAccessTextLogFormat("", prov.OmitEmptyValues, skipBuiltInFormatter)
+		fl.AccessLogFormat, needsFormatter = buildFileAccessTextLogFormat("", prov.OmitEmptyValues)
 	}
 	if len(needsFormatter) != 0 {
 		fl.GetLogFormat().Formatters = needsFormatter
@@ -264,10 +243,10 @@ func fileAccessLogFromTelemetry(prov *meshconfig.MeshConfig_ExtensionProvider_En
 }
 
 func buildFileAccessTextLogFormat(
-	logFormatText string, omitEmptyValues bool, skipBuiltInFormatter bool,
+	logFormatText string, omitEmptyValues bool,
 ) (*fileaccesslog.FileAccessLog_LogFormat, []*core.TypedExtensionConfig) {
 	formatString := fileAccessLogFormat(logFormatText)
-	formatters := accessLogTextFormatters(formatString, skipBuiltInFormatter)
+	formatters := accessLogTextFormatters(formatString)
 
 	return &fileaccesslog.FileAccessLog_LogFormat{
 		LogFormat: &core.SubstitutionFormatString{
@@ -285,7 +264,7 @@ func buildFileAccessTextLogFormat(
 
 func buildFileAccessJSONLogFormat(
 	logFormat *meshconfig.MeshConfig_ExtensionProvider_EnvoyFileAccessLogProvider_LogFormat_Labels,
-	omitEmptyValues bool, skipBuiltInFormatter bool,
+	omitEmptyValues bool,
 ) (*fileaccesslog.FileAccessLog_LogFormat, []*core.TypedExtensionConfig) {
 	jsonLogStruct := EnvoyJSONLogFormatIstio
 	if logFormat.Labels != nil {
@@ -297,7 +276,7 @@ func buildFileAccessJSONLogFormat(
 		jsonLogStruct = EnvoyJSONLogFormatIstio
 	}
 
-	formatters := accessLogJSONFormatters(jsonLogStruct, skipBuiltInFormatter)
+	formatters := accessLogJSONFormatters(jsonLogStruct)
 	return &fileaccesslog.FileAccessLog_LogFormat{
 		LogFormat: &core.SubstitutionFormatString{
 			Format: &core.SubstitutionFormatString_JsonFormat{
@@ -309,50 +288,31 @@ func buildFileAccessJSONLogFormat(
 	}, formatters
 }
 
-func accessLogJSONFormatters(jsonLogStruct *structpb.Struct, skipBuiltInFormatter bool) []*core.TypedExtensionConfig {
+func accessLogJSONFormatters(jsonLogStruct *structpb.Struct) []*core.TypedExtensionConfig {
 	if jsonLogStruct == nil {
 		return nil
 	}
 
-	reqWithoutQuery, metadata, cel := false, false, false
+	reqWithoutQuery := false
 	for _, value := range jsonLogStruct.Fields {
 		if !reqWithoutQuery && strings.Contains(value.GetStringValue(), reqWithoutQueryCommandOperator) {
 			reqWithoutQuery = true
 		}
-		if !skipBuiltInFormatter && !metadata && strings.Contains(value.GetStringValue(), metadataCommandOperator) {
-			metadata = true
-		}
-		if !skipBuiltInFormatter && !cel && strings.Contains(value.GetStringValue(), celCommandOperator) {
-			cel = true
-		}
 	}
 
 	formatters := make([]*core.TypedExtensionConfig, 0, maxFormattersLength)
 	if reqWithoutQuery {
 		formatters = append(formatters, reqWithoutQueryFormatter)
 	}
-	if metadata {
-		formatters = append(formatters, metadataFormatter)
-	}
-	if cel {
-		formatters = append(formatters, celFormatter)
-	}
 
 	return formatters
 }
 
-func accessLogTextFormatters(text string, skipBuiltInFormatter bool) []*core.TypedExtensionConfig {
+func accessLogTextFormatters(text string) []*core.TypedExtensionConfig {
 	formatters := make([]*core.TypedExtensionConfig, 0, maxFormattersLength)
 	if strings.Contains(text, reqWithoutQueryCommandOperator) {
 		formatters = append(formatters, reqWithoutQueryFormatter)
 	}
-	// Start from Istio 1.26+(envoy 1.34), the formatter ``%CEL%`` and ``%METADATA%`` will be treated as built-in formatters
-	if !skipBuiltInFormatter && strings.Contains(text, metadataCommandOperator) {
-		formatters = append(formatters, metadataFormatter)
-	}
-	if !skipBuiltInFormatter && strings.Contains(text, celCommandOperator) {
-		formatters = append(formatters, celFormatter)
-	}
 
 	return formatters
 }
@@ -415,7 +375,7 @@ func fileAccessLogFormat(formatString string) string {
 	return EnvoyTextLogFormat
 }
 
-func FileAccessLogFromMeshConfig(path string, mesh *meshconfig.MeshConfig, skipBuiltInFormatter bool) *accesslog.AccessLog {
+func FileAccessLogFromMeshConfig(path string, mesh *meshconfig.MeshConfig) *accesslog.AccessLog {
 	// We need to build access log. This is needed either on first access or when mesh config changes.
 	fl := &fileaccesslog.FileAccessLog{
 		Path: path,
@@ -424,7 +384,7 @@ func FileAccessLogFromMeshConfig(path string, mesh *meshconfig.MeshConfig, skipB
 	switch mesh.AccessLogEncoding {
 	case meshconfig.MeshConfig_TEXT:
 		formatString := fileAccessLogFormat(mesh.AccessLogFormat)
-		formatters = accessLogTextFormatters(formatString, skipBuiltInFormatter)
+		formatters = accessLogTextFormatters(formatString)
 		fl.AccessLogFormat = &fileaccesslog.FileAccessLog_LogFormat{
 			LogFormat: &core.SubstitutionFormatString{
 				Format: &core.SubstitutionFormatString_TextFormatSource{
@@ -446,7 +406,7 @@ func FileAccessLogFromMeshConfig(path string, mesh *meshconfig.MeshConfig, skipB
 				jsonLogStruct = &parsedJSONLogStruct
 			}
 		}
-		formatters = accessLogJSONFormatters(jsonLogStruct, skipBuiltInFormatter)
+		formatters = accessLogJSONFormatters(jsonLogStruct)
 		fl.AccessLogFormat = &fileaccesslog.FileAccessLog_LogFormat{
 			LogFormat: &core.SubstitutionFormatString{
 				Format: &core.SubstitutionFormatString_JsonFormat{
@@ -471,7 +431,7 @@ func FileAccessLogFromMeshConfig(path string, mesh *meshconfig.MeshConfig, skipB
 }
 
 func openTelemetryLog(pushCtx *PushContext, proxy *Proxy,
-	provider *meshconfig.MeshConfig_ExtensionProvider_EnvoyOpenTelemetryLogProvider, skipBuiltInFormatter bool,
+	provider *meshconfig.MeshConfig_ExtensionProvider_EnvoyOpenTelemetryLogProvider,
 ) *accesslog.AccessLog {
 	hostname, cluster, err := clusterLookupFn(pushCtx, provider.Service, int(provider.Port))
 	if err != nil {
@@ -495,7 +455,7 @@ func openTelemetryLog(pushCtx *PushContext, proxy *Proxy,
 		labels = provider.LogFormat.Labels
 	}
 
-	cfg := buildOpenTelemetryAccessLogConfig(proxy, logName, hostname, cluster, f, labels, skipBuiltInFormatter)
+	cfg := buildOpenTelemetryAccessLogConfig(proxy, logName, hostname, cluster, f, labels)
 
 	return &accesslog.AccessLog{
 		Name:       OtelEnvoyALSName,
@@ -504,7 +464,7 @@ func openTelemetryLog(pushCtx *PushContext, proxy *Proxy,
 }
 
 func buildOpenTelemetryAccessLogConfig(proxy *Proxy,
-	logName, hostname, clusterName, format string, labels *structpb.Struct, skipBuiltInFormatter bool,
+	logName, hostname, clusterName, format string, labels *structpb.Struct,
 ) *otelaccesslog.OpenTelemetryAccessLogConfig {
 	cfg := &otelaccesslog.OpenTelemetryAccessLogConfig{
 		CommonConfig: &grpcaccesslog.CommonGrpcAccessLogConfig{
@@ -537,20 +497,20 @@ func buildOpenTelemetryAccessLogConfig(proxy *Proxy,
 		}
 	}
 
-	cfg.Formatters = accessLogFormatters(format, labels, skipBuiltInFormatter)
+	cfg.Formatters = accessLogFormatters(format, labels)
 
 	return cfg
 }
 
-func accessLogFormatters(text string, labels *structpb.Struct, skipBuiltInFormatter bool) []*core.TypedExtensionConfig {
+func accessLogFormatters(text string, labels *structpb.Struct) []*core.TypedExtensionConfig {
 	formatters := make([]*core.TypedExtensionConfig, 0, maxFormattersLength)
 	defer func() {
 		slices.SortBy(formatters, func(f *core.TypedExtensionConfig) string {
 			return f.Name
 		})
 	}()
 
-	formatters = append(formatters, accessLogTextFormatters(text, skipBuiltInFormatter)...)
+	formatters = append(formatters, accessLogTextFormatters(text)...)
 	if len(formatters) >= maxFormattersLength {
 		// all formatters are added, return if we have reached the limit
 		return formatters
@@ -561,7 +521,7 @@ func accessLogFormatters(text string, labels *structpb.Struct, skipBuiltInFormat
 		names.Insert(f.Name)
 	}
 
-	for _, f := range accessLogJSONFormatters(labels, skipBuiltInFormatter) {
+	for _, f := range accessLogJSONFormatters(labels) {
 		if !names.Contains(f.Name) {
 			formatters = append(formatters, f)
 			names.Insert(f.Name)