internal/envoy: configurable access log format #3694
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,29 +17,47 @@ import ( | |
| envoy_accesslog_v3 "github.com/envoyproxy/go-control-plane/envoy/config/accesslog/v3" | ||
| envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" | ||
| envoy_file_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/file/v3" | ||
| envoy_req_without_query_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/req_without_query/v3" | ||
| "github.com/envoyproxy/go-control-plane/pkg/wellknown" | ||
| _struct "github.com/golang/protobuf/ptypes/struct" | ||
| "github.com/projectcontour/contour/internal/protobuf" | ||
| "github.com/projectcontour/contour/pkg/config" | ||
| ) | ||
|
|
||
| // FileAccessLogEnvoy returns a new file based access log filter | ||
| func FileAccessLogEnvoy(path string, format string, extensions []string) []*envoy_accesslog_v3.AccessLog { | ||
| // Nil by default to defer to Envoy's default log format. | ||
| var logFormat *envoy_file_v3.FileAccessLog_LogFormat | ||
|
|
||
| if format != "" { | ||
| logFormat = &envoy_file_v3.FileAccessLog_LogFormat{ | ||
| LogFormat: &envoy_config_core_v3.SubstitutionFormatString{ | ||
| Format: &envoy_config_core_v3.SubstitutionFormatString_TextFormatSource{ | ||
| TextFormatSource: &envoy_config_core_v3.DataSource{ | ||
| Specifier: &envoy_config_core_v3.DataSource_InlineString{ | ||
| InlineString: format, | ||
|
There was a problem hiding this comment. Will Envoy blow up if the format is invalid? Or just not log properly? There was a problem hiding this comment. Oh I see there's a parser below. There was a problem hiding this comment. Yes, the same parser is now used for both JSON and text based logs. If for some reason invalid format would still get through parser, I've tested how Envoy would react, for results see #3576 (comment). There is also one very common error that could be validated: there needs to be There was a problem hiding this comment. Oof, yeah, that There was a problem hiding this comment. Yeah, I think I'm a 👍 to enforcing/adding the There was a problem hiding this comment. I've now added check for |
||
| }, | ||
| }, | ||
| }, | ||
| Formatters: extensionConfig(extensions), | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| return []*envoy_accesslog_v3.AccessLog{{ | ||
| Name: wellknown.FileAccessLog, | ||
| ConfigType: &envoy_accesslog_v3.AccessLog_TypedConfig{ | ||
| TypedConfig: protobuf.MustMarshalAny(&envoy_file_v3.FileAccessLog{ | ||
| Path: path, | ||
| AccessLogFormat: logFormat, | ||
| }), | ||
| }, | ||
| }} | ||
| } | ||
|
|
||
| // FileAccessLogJSON returns a new file based access log filter | ||
| // that will log in JSON format | ||
| func FileAccessLogJSON(path string, fields config.AccessLogFields, extensions []string) []*envoy_accesslog_v3.AccessLog { | ||
|
|
||
| jsonformat := &_struct.Struct{ | ||
| Fields: make(map[string]*_struct.Value), | ||
|
|
@@ -59,6 +77,7 @@ func FileAccessLogJSON(path string, fields config.AccessLogFields) []*envoy_acce | |
| Format: &envoy_config_core_v3.SubstitutionFormatString_JsonFormat{ | ||
| JsonFormat: jsonformat, | ||
| }, | ||
| Formatters: extensionConfig(extensions), | ||
| }, | ||
| }, | ||
| }), | ||
|
|
@@ -73,3 +92,23 @@ func sv(s string) *_struct.Value { | |
| }, | ||
| } | ||
| } | ||
|
|
||
| // extensionConfig returns a list of extension configs required by the access log format. | ||
| // | ||
| // Note: When adding support for new formatter, update the list of extensions here and | ||
| // add the corresponding extension in pkg/config/parameters.go AccessLogFormatterExtensions(). | ||
| // Currently only one extension exist in Envoy. | ||
| func extensionConfig(extensions []string) []*envoy_config_core_v3.TypedExtensionConfig { | ||
| var config []*envoy_config_core_v3.TypedExtensionConfig | ||
|
|
||
| for _, e := range extensions { | ||
| if e == "envoy.formatter.req_without_query" { | ||
tsaarni marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| config = append(config, &envoy_config_core_v3.TypedExtensionConfig{ | ||
| Name: "envoy.formatter.req_without_query", | ||
| TypedConfig: protobuf.MustMarshalAny(&envoy_req_without_query_v3.ReqWithoutQuery{ /* empty */ }), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| return config | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might need to be docs on the site. I worry we're getting too many comments in the config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've shortened the text by removing the link to Envoy docs and the full example (still leaving short example value
accesslog-format-string: "...\n")The config file is bit hard to read in general. Maybe one small improvement would be changing indentation of fields-vs-comments, see e.g. sshd_config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d'oh, it obviously fails anyways to be readable due to structs having indentations :-D
maybe double-hash for comments:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this formatting is not great. I filed #3843 to make some improvements here.