Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

access log: add support for command formatter extensions #14512

Merged
merged 34 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
53f4bc1
access log: add support for command formatter extensions
Dec 22, 2020
0857a9e
Fix spelling
Dec 23, 2020
15606e6
Fix another spelling issue
Dec 23, 2020
e29138b
Use InjectFactory instead of REGISTER_FACTORY
Dec 23, 2020
238ccce
Use default constructor per clang-tidy
Dec 23, 2020
d69116a
Init bool to false
Dec 23, 2020
777423e
Merge remote-tracking branch 'upstream/master' into access-log-format…
Dec 24, 2020
8e74c99
Make parsing helpers public
Dec 24, 2020
c8e03ae
Update CommandParser's parse() to use const std::string&
Dec 24, 2020
e2b4035
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 4, 2021
6ddcea4
Pass more params to CommandParser::parse
Jan 4, 2021
f7d0c83
In-line comment for when/how to use formatter extensions
Jan 4, 2021
c936d03
CommandParser interface: improve comment docs
Jan 4, 2021
b841350
Docs for parseBuiltinCommand
Jan 4, 2021
a4afd3a
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 4, 2021
ffad3c5
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 4, 2021
fc127e8
Use TypedExtensionConfig and lookup extensions by type
Jan 5, 2021
28f7ab3
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 7, 2021
46fbee6
Fix bad merge
Jan 7, 2021
7d76e5b
Use Envoy::Config::Utility::getFactory
Jan 7, 2021
aa1a6e1
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 7, 2021
d0fc904
Add integration test
Jan 8, 2021
9f39f7e
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 8, 2021
61e3cff
Fix format
Jan 8, 2021
41f9b59
Fix factory registration
Jan 8, 2021
4538e77
Avoid REGISTER_FACTORY from tests
Jan 8, 2021
0e91778
clang-tidy
Jan 8, 2021
ac9f06e
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 8, 2021
a5d166c
Update changelog
Jan 8, 2021
48e1f8d
Use size_t
Jan 8, 2021
83eefaf
Explain how pos and command_end_position work
Jan 8, 2021
68660d3
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 12, 2021
6f5d55b
Fix changelog
Jan 12, 2021
4284ce7
Merge remote-tracking branch 'upstream/master' into access-log-format…
Jan 12, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions REPO_LAYOUT.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ code/extensions, and allows us specify extension owners in [CODEOWNERS](CODEOWNE
`Envoy::Extensions::ListenerFilters` namespace.
* [filters/network/](/source/extensions/filters/network): L4 network filters which use the
`Envoy::Extensions::NetworkFilters` namespace.
* [formatters](/source/extensions/formatters): Access log formatters which use the
`Envoy::Extensions::Formatters` namespace.
* [grpc_credentials/](/source/extensions/grpc_credentials): Custom gRPC credentials which use the
`Envoy::Extensions::GrpcCredentials` namespace.
* [health_checker/](/source/extensions/health_checker): Custom health checkers which use the
Expand Down
8 changes: 7 additions & 1 deletion api/envoy/config/core/v3/substitution_format_string.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ syntax = "proto3";
package envoy.config.core.v3;

import "envoy/config/core/v3/base.proto";
import "envoy/config/core/v3/extension.proto";

import "google/protobuf/any.proto";
import "google/protobuf/struct.proto";

import "udpa/annotations/status.proto";
Expand All @@ -18,7 +20,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;

// Configuration to use multiple :ref:`command operators <config_access_log_command_operators>`
// to generate a new string in either plain text or JSON format.
// [#next-free-field: 6]
// [#next-free-field: 7]
message SubstitutionFormatString {
oneof format {
option (validate.required) = true;
Expand Down Expand Up @@ -103,4 +105,8 @@ message SubstitutionFormatString {
// content_type: "text/html; charset=UTF-8"
//
string content_type = 4;

// Specifies a collection of Formatter plugins that can be called from the access log configuration.
// See the formatters extensions documentation for details.
repeated TypedExtensionConfig formatters = 6;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/root/extending/extending.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ types including:
* :ref:`Compression libraries <arch_overview_compression_libraries>`
* :ref:`Bootstrap extensions <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.bootstrap_extensions>`
* :ref:`Fatal actions <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.fatal_actions>`
* :ref:`Formatters <config_access_log_command_operators>`

As of this writing there is no high level extension developer documentation. The
:repo:`existing extensions <source/extensions>` are a good way to learn what is possible.
Expand Down
3 changes: 2 additions & 1 deletion docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Removed Config or Runtime

New Features
------------
* access log: added the :ref:`formatters <envoy_v3_api_field_config.core.v3.SubstitutionFormatString.formatters>` extension point for custom formatters (command operators).

Deprecated
----------
----------

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 52 additions & 0 deletions include/envoy/formatter/substitution_formatter.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <string>

#include "envoy/common/pure.h"
#include "envoy/config/typed_config.h"
#include "envoy/http/header_map.h"
#include "envoy/stream_info/stream_info.h"

Expand Down Expand Up @@ -78,5 +79,56 @@ class FormatterProvider {

using FormatterProviderPtr = std::unique_ptr<FormatterProvider>;

/**
* Interface for command parser.
* CommandParser returns a FormatterProviderPtr after successfully parsing an access log format
* token, nullptr otherwise.
*/
class CommandParser {
public:
virtual ~CommandParser() = default;

/**
* Return a FormatterProviderPtr if this command is parsed from the token.
* @param token the token to parse
* @param pos current position in the entire format string
* @param command_end_position position at the end of the command token
rgs1 marked this conversation as resolved.
Show resolved Hide resolved
*
* Given the following format line using an extension called %CMD()%:
*
* %CMD()% %START_TIME(%Y/%m/%d)% ...
*
* The call to parse() for that extension would look like this:
*
* parse("CMD()", 1, 5)
*
* @return FormattterProviderPtr substitution provider for the parsed command
*/
virtual FormatterProviderPtr parse(const std::string& token, size_t pos,
size_t command_end_position) const PURE;
};

using CommandParserPtr = std::unique_ptr<CommandParser>;

/**
* Implemented by each custom CommandParser and registered via Registry::registerFactory()
* or the convenience class RegisterFactory.
*/
class CommandParserFactory : public Config::TypedFactory {
public:
~CommandParserFactory() override = default;

/**
* Creates a particular CommandParser implementation.
*
* @param config supplies the configuration for the command parser.
* @return CommandParserPtr the CommandParser which will be used in
* SubstitutionFormatParser::parse() when evaluating an access log format string.
*/
virtual CommandParserPtr createCommandParserFromProto(const Protobuf::Message& config) PURE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want a message validation context here as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean within createCommandParserFromProto() something like ProtobufMessage::getStrictValidationVisitor() should be used or should it be a param for createCommandParserFromProto() itself?

Looking at this other extension point (#10908), the validator isn't part of creator method that receives a config...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was looking into this recently here, and it seems that validator generally comes from the factory context. A strict validator might be OK for now, not sure what additional benefit the context validator provides.


std::string category() const override { return "envoy.formatter"; }
};

} // namespace Formatter
} // namespace Envoy
1 change: 1 addition & 0 deletions source/common/formatter/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ envoy_cc_library(
hdrs = ["substitution_format_string.h"],
deps = [
":substitution_formatter_lib",
"//source/common/config:utility_lib",
"//source/common/protobuf",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
Expand Down
17 changes: 15 additions & 2 deletions source/common/formatter/substitution_format_string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "envoy/api/api.h"

#include "common/config/datasource.h"
#include "common/config/utility.h"
#include "common/formatter/substitution_formatter.h"

namespace Envoy {
Expand All @@ -16,14 +17,26 @@ SubstitutionFormatStringUtils::createJsonFormatter(const ProtobufWkt::Struct& st

FormatterPtr SubstitutionFormatStringUtils::fromProtoConfig(
const envoy::config::core::v3::SubstitutionFormatString& config, Api::Api& api) {
// Instantiate formatter extensions.
std::vector<CommandParserPtr> commands;
for (const auto& formatter : config.formatters()) {
auto* factory = Envoy::Config::Utility::getFactory<CommandParserFactory>(formatter);
if (!factory) {
throw EnvoyException(absl::StrCat("Formatter not found: ", formatter.name()));
}
auto parser = factory->createCommandParserFromProto(formatter.typed_config());
commands.push_back(std::move(parser));
}

switch (config.format_case()) {
case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kTextFormat:
return std::make_unique<FormatterImpl>(config.text_format(), config.omit_empty_values());
return std::make_unique<FormatterImpl>(config.text_format(), config.omit_empty_values(),
commands);
case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kJsonFormat: {
return createJsonFormatter(config.json_format(), true, config.omit_empty_values());
case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kTextFormatSource:
return std::make_unique<FormatterImpl>(
Config::DataSource::read(config.text_format_source(), true, api));
Config::DataSource::read(config.text_format_source(), true, api), false, commands);
}
default:
NOT_REACHED_GCOVR_EXCL_LINE;
Expand Down
Loading