Skip to content

Commit

Permalink
Add %UPSTREAM_CLUSTER_RAW% formatter (envoyproxy#35265)
Browse files Browse the repository at this point in the history
The %UPSTREAM_CLUSTER% formatter is affected by alt_stat_name, but there
are times where a custom stat name may have special delimeters meant to
help with stat tag regex parsing but those delimeters shouldn't show up
in access logs. In these cases, %RAW_UPSTREAM_CLUSTER% could be used.

Commit Message: Add %RAW_UPSTREAM_CLUSTER% access log formatter
Additional Description:
Risk Level: no
Testing: unit
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Signed-off-by: asingh-g <abhisinghx@google.com>
  • Loading branch information
keithmattix authored and asingh-g committed Aug 20, 2024
1 parent 036a468 commit a132efa
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 3 deletions.
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,9 @@ new_features:
Added config field
:ref:`filter_metadata <envoy_v3_api_field_extensions.filters.http.ext_authz.v3.ExtAuthz.filter_metadata>`
for injecting arbitrary data to the filter state for logging.
- area: access_log
change: |
added %UPSTREAM_CLUSTER_RAW% access log formatter to log the original upstream cluster name, regadless of whether
``alt_stat_name`` is set.
deprecated:
4 changes: 4 additions & 0 deletions docs/root/configuration/observability/access_log/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,10 @@ UDP
Upstream cluster to which the upstream host belongs to. :ref:`alt_stat_name
<envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` will be used if provided.

%UPSTREAM_CLUSTER_RAW%
Upstream cluster to which the upstream host belongs to. :ref:`alt_stat_name
<envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` does NOT modify this value.

%UPSTREAM_LOCAL_ADDRESS%
Local address of the upstream connection. If the address is an IP address it includes both
address and port.
Expand Down
16 changes: 16 additions & 0 deletions source/common/formatter/stream_info_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,22 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide
stream_info.upstreamClusterInfo().value()->observabilityName();
}

return upstream_cluster_name.empty()
? absl::nullopt
: absl::make_optional<std::string>(upstream_cluster_name);
});
}}},
{"UPSTREAM_CLUSTER_RAW",
{CommandSyntaxChecker::COMMAND_ONLY,
[](const std::string&, absl::optional<size_t>) {
return std::make_unique<StreamInfoStringFormatterProvider>(
[](const StreamInfo::StreamInfo& stream_info) {
std::string upstream_cluster_name;
if (stream_info.upstreamClusterInfo().has_value() &&
stream_info.upstreamClusterInfo().value() != nullptr) {
upstream_cluster_name = stream_info.upstreamClusterInfo().value()->name();
}

return upstream_cluster_name.empty()
? absl::nullopt
: absl::make_optional<std::string>(upstream_cluster_name);
Expand Down
21 changes: 21 additions & 0 deletions test/common/formatter/substitution_formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,27 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) {
ProtoEq(ValueUtil::nullValue()));
}

{
StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER_RAW");
const std::string raw_cluster_name = "raw_name";
auto cluster_info_mock = std::make_shared<Upstream::MockClusterInfo>();
absl::optional<Upstream::ClusterInfoConstSharedPtr> cluster_info = cluster_info_mock;
EXPECT_CALL(stream_info, upstreamClusterInfo()).WillRepeatedly(Return(cluster_info));
EXPECT_CALL(*cluster_info_mock, name()).WillRepeatedly(ReturnRef(raw_cluster_name));
EXPECT_EQ("raw_name", upstream_format.formatWithContext({}, stream_info));
EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info),
ProtoEq(ValueUtil::stringValue("raw_name")));
}

{
StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER_RAW");
absl::optional<Upstream::ClusterInfoConstSharedPtr> cluster_info = nullptr;
EXPECT_CALL(stream_info, upstreamClusterInfo()).WillRepeatedly(Return(cluster_info));
EXPECT_EQ(absl::nullopt, upstream_format.formatWithContext({}, stream_info));
EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info),
ProtoEq(ValueUtil::nullValue()));
}

{
NiceMock<Api::MockOsSysCalls> os_sys_calls;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls(&os_sys_calls);
Expand Down
28 changes: 25 additions & 3 deletions test/common/router/router_upstream_log_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,11 @@ class RouterUpstreamLogTest : public testing::Test {
bool enable_periodic_upstream_log = false) {
envoy::extensions::filters::http::router::v3::Router router_proto;
static const std::string cluster_name = "cluster_0";
static const std::string observability_name = "cluster-0";

cluster_info_ = std::make_shared<NiceMock<Upstream::MockClusterInfo>>();
ON_CALL(*cluster_info_, name()).WillByDefault(ReturnRef(cluster_name));
ON_CALL(*cluster_info_, observabilityName()).WillByDefault(ReturnRef(cluster_name));
ON_CALL(*cluster_info_, observabilityName()).WillByDefault(ReturnRef(observability_name));
ON_CALL(callbacks_.stream_info_, upstreamClusterInfo()).WillByDefault(Return(cluster_info_));
callbacks_.stream_info_.downstream_bytes_meter_ = std::make_shared<StreamInfo::BytesMeter>();
EXPECT_CALL(callbacks_.dispatcher_, deferredDelete_).Times(testing::AnyNumber());
Expand Down Expand Up @@ -435,6 +436,27 @@ name: accesslog
init(absl::optional<envoy::config::accesslog::v3::AccessLog>(upstream_log));
run();

EXPECT_EQ(output_.size(), 1U);
EXPECT_EQ(output_.front(), "cluster-0");
}

// Test UPSTREAM_CLUSTER_RAW log formatter.
TEST_F(RouterUpstreamLogTest, RawUpstreamCluster) {
const std::string yaml = R"EOF(
name: accesslog
typed_config:
"@type": type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog
log_format:
text_format_source:
inline_string: "%UPSTREAM_CLUSTER_RAW%"
path: "/dev/null"
)EOF";

envoy::config::accesslog::v3::AccessLog upstream_log;
TestUtility::loadFromYaml(yaml, upstream_log);
init(absl::optional<envoy::config::accesslog::v3::AccessLog>(upstream_log));
run();

EXPECT_EQ(output_.size(), 1U);
EXPECT_EQ(output_.front(), "cluster_0");
}
Expand All @@ -461,9 +483,9 @@ name: accesslog
EXPECT_EQ(output_.size(), 2U);
EXPECT_EQ(
output_.front(),
absl::StrCat("cluster_0 ", AccessLogType_Name(AccessLog::AccessLogType::UpstreamPoolReady)));
absl::StrCat("cluster-0 ", AccessLogType_Name(AccessLog::AccessLogType::UpstreamPoolReady)));
EXPECT_EQ(output_.back(),
absl::StrCat("cluster_0 ", AccessLogType_Name(AccessLog::AccessLogType::UpstreamEnd)));
absl::StrCat("cluster-0 ", AccessLogType_Name(AccessLog::AccessLogType::UpstreamEnd)));
}

TEST_F(RouterUpstreamLogTest, PeriodicLog) {
Expand Down

0 comments on commit a132efa

Please sign in to comment.