Skip to content

Commit 87f3033

Browse files
istio-testingPetrMc
istio-testing
and
PetrMc
authored
[release-1.29] Add FIELD accessor support for peer metadata in tracing (#6775)
* Add FIELD accessor support for peer metadata in tracing Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * lint Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * address PR comments, fix tests Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * fix test when labels are not present Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * test fixes Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * Store both CelState and WorkloadMetadataObject for CEL and FIELD accessor support Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * Use separate keys for CelState and WorkloadMetadataObject storage Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * Update istio_stats to use new peer metadata object keys Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> * Changed peerInfoRead to check the *PeerObj Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> --------- Signed-off-by: Petr McAllister <petr.mcallister@gmail.com> Co-authored-by: Petr McAllister <petr.mcallister@gmail.com>
1 parent 22eff7c commit 87f3033

4 files changed

Lines changed: 127 additions & 20 deletions

File tree

‎extensions/common/metadata_object.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,14 @@ namespace Istio {
2727
namespace Common {
2828

2929
// Filter state key to store the peer metadata under.
30+
// CelState is stored under these keys for CEL expression support.
3031
constexpr absl::string_view DownstreamPeer = "downstream_peer";
3132
constexpr absl::string_view UpstreamPeer = "upstream_peer";
3233

34+
// Filter state keys for WorkloadMetadataObject (FIELD accessor support).
35+
constexpr absl::string_view DownstreamPeerObj = "downstream_peer_obj";
36+
constexpr absl::string_view UpstreamPeerObj = "upstream_peer_obj";
37+
3338
// Special filter state key to indicate the filter is done looking for peer metadata.
3439
// This is used by network metadata exchange on failure.
3540
constexpr absl::string_view NoPeer = "peer_not_found";

‎source/extensions/filters/http/istio_stats/istio_stats.cc‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,27 +114,37 @@ enum class Reporter {
114114
};
115115

116116
// Detect if peer info read is completed by TCP metadata exchange.
117+
// Checks for WorkloadMetadataObject key (set atomically with CelState by peer_metadata filter).
117118
bool peerInfoRead(Reporter reporter, const StreamInfo::FilterState& filter_state) {
118119
const auto& filter_state_key =
119120
reporter == Reporter::ServerSidecar || reporter == Reporter::ServerGateway
120-
? Istio::Common::DownstreamPeer
121-
: Istio::Common::UpstreamPeer;
121+
? Istio::Common::DownstreamPeerObj
122+
: Istio::Common::UpstreamPeerObj;
122123
return filter_state.hasDataWithName(filter_state_key) ||
123124
filter_state.hasDataWithName(Istio::Common::NoPeer);
124125
}
125126

126127
std::optional<Istio::Common::WorkloadMetadataObject>
127128
peerInfo(Reporter reporter, const StreamInfo::FilterState& filter_state) {
128-
const auto& filter_state_key =
129+
const auto& cel_state_key =
129130
reporter == Reporter::ServerSidecar || reporter == Reporter::ServerGateway
130131
? Istio::Common::DownstreamPeer
131132
: Istio::Common::UpstreamPeer;
132-
// This's a workaround before FilterStateObject support operation like `.labels['role']`.
133-
// The workaround is to use CelState to store the peer metadata.
134-
// Rebuild the WorkloadMetadataObject from the CelState.
133+
const auto& obj_key = reporter == Reporter::ServerSidecar || reporter == Reporter::ServerGateway
134+
? Istio::Common::DownstreamPeerObj
135+
: Istio::Common::UpstreamPeerObj;
136+
137+
// Try reading as WorkloadMetadataObject first (new format, stored under *_obj key)
138+
const auto* peer_info =
139+
filter_state.getDataReadOnly<Istio::Common::WorkloadMetadataObject>(obj_key);
140+
if (peer_info) {
141+
return *peer_info;
142+
}
143+
144+
// Fall back to CelState for backward compatibility with older deployments
135145
const auto* cel_state =
136146
filter_state.getDataReadOnly<Envoy::Extensions::Filters::Common::Expr::CelState>(
137-
filter_state_key);
147+
cel_state_key);
138148
if (!cel_state) {
139149
return {};
140150
}
@@ -144,7 +154,7 @@ peerInfo(Reporter reporter, const StreamInfo::FilterState& filter_state) {
144154
return {};
145155
}
146156

147-
Istio::Common::WorkloadMetadataObject peer_info(
157+
Istio::Common::WorkloadMetadataObject result(
148158
extractString(obj, Istio::Common::InstanceNameToken),
149159
extractString(obj, Istio::Common::ClusterNameToken),
150160
extractString(obj, Istio::Common::NamespaceNameToken),
@@ -156,7 +166,17 @@ peerInfo(Reporter reporter, const StreamInfo::FilterState& filter_state) {
156166
Istio::Common::fromSuffix(extractString(obj, Istio::Common::WorkloadTypeToken)),
157167
extractString(obj, Istio::Common::IdentityToken));
158168

159-
return peer_info;
169+
// Extract labels from the "labels" field
170+
const auto& labels_it = obj.fields().find(Istio::Common::LabelsToken);
171+
if (labels_it != obj.fields().end() && labels_it->second.has_struct_value()) {
172+
std::vector<std::pair<std::string, std::string>> labels;
173+
for (const auto& label : labels_it->second.struct_value().fields()) {
174+
labels.push_back({std::string(label.first), std::string(label.second.string_value())});
175+
}
176+
result.setLabels(labels);
177+
}
178+
179+
return result;
160180
}
161181

162182
// Process-wide context shared with all filter instances.

‎source/extensions/filters/http/peer_metadata/filter.cc‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,13 +291,23 @@ void FilterConfig::setFilterState(StreamInfo::StreamInfo& info, bool downstream,
291291
const PeerInfo& value) const {
292292
const absl::string_view key =
293293
downstream ? Istio::Common::DownstreamPeer : Istio::Common::UpstreamPeer;
294+
const absl::string_view obj_key =
295+
downstream ? Istio::Common::DownstreamPeerObj : Istio::Common::UpstreamPeerObj;
294296
if (!info.filterState()->hasDataWithName(key)) {
295-
// Use CelState to allow operation filter_state.upstream_peer.labels['role']
297+
// Store CelState for CEL expressions like filter_state.downstream_peer.labels['role']
296298
auto pb = value.serializeAsProto();
297-
auto peer_info = std::make_unique<CelState>(FilterConfig::peerInfoPrototype());
298-
peer_info->setValue(absl::string_view(pb->SerializeAsString()));
299+
auto cel_state = std::make_unique<CelState>(FilterConfig::peerInfoPrototype());
300+
cel_state->setValue(absl::string_view(pb->SerializeAsString()));
299301
info.filterState()->setData(
300-
key, std::move(peer_info), StreamInfo::FilterState::StateType::Mutable,
302+
key, std::move(cel_state), StreamInfo::FilterState::StateType::Mutable,
303+
StreamInfo::FilterState::LifeSpan::FilterChain, sharedWithUpstream());
304+
305+
// Also store WorkloadMetadataObject under a separate key for FIELD accessor support.
306+
// WorkloadMetadataObject implements hasFieldSupport() + getField() for
307+
// formatters using %FILTER_STATE(downstream_peer_obj:FIELD:fieldname)% syntax.
308+
auto workload_metadata = std::make_unique<PeerInfo>(value);
309+
info.filterState()->setData(
310+
obj_key, std::move(workload_metadata), StreamInfo::FilterState::StateType::Mutable,
301311
StreamInfo::FilterState::LifeSpan::FilterChain, sharedWithUpstream());
302312
} else {
303313
ENVOY_LOG(debug, "Duplicate peer metadata, skipping");

‎source/extensions/filters/http/peer_metadata/filter_test.cc‎

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,10 @@ class PeerMetadataTest : public testing::Test {
7979
downstream ? Istio::Common::DownstreamPeer : Istio::Common::UpstreamPeer));
8080
}
8181
void checkPeerNamespace(bool downstream, const std::string& expected) {
82-
const auto* cel_state =
83-
stream_info_.filterState()
84-
->getDataReadOnly<Envoy::Extensions::Filters::Common::Expr::CelState>(
85-
downstream ? Istio::Common::DownstreamPeer : Istio::Common::UpstreamPeer);
86-
Protobuf::Struct obj;
87-
ASSERT_TRUE(obj.ParseFromString(cel_state->value().data()));
88-
EXPECT_EQ(expected, extractString(obj, "namespace"));
82+
const auto* peer_info = stream_info_.filterState()->getDataReadOnly<PeerInfo>(
83+
downstream ? Istio::Common::DownstreamPeerObj : Istio::Common::UpstreamPeerObj);
84+
ASSERT_NE(peer_info, nullptr);
85+
EXPECT_EQ(expected, peer_info->namespace_name_);
8986
}
9087

9188
absl::string_view extractString(const Protobuf::Struct& metadata, absl::string_view key) {
@@ -488,6 +485,81 @@ TEST_F(PeerMetadataTest, UpstreamMXPropagationSkipPassthrough) {
488485
checkNoPeer(false);
489486
}
490487

488+
TEST_F(PeerMetadataTest, FieldAccessorSupport) {
489+
const WorkloadMetadataObject pod("pod-foo-1234", "my-cluster", "default", "foo", "foo-service",
490+
"v1alpha3", "myapp", "v1", Istio::Common::WorkloadType::Pod, "");
491+
EXPECT_CALL(*metadata_provider_, GetMetadata(_))
492+
.WillRepeatedly(Invoke([&](const Network::Address::InstanceConstSharedPtr& address)
493+
-> std::optional<WorkloadMetadataObject> {
494+
if (absl::StartsWith(address->asStringView(), "127.0.0.1")) {
495+
return {pod};
496+
}
497+
return {};
498+
}));
499+
initialize(R"EOF(
500+
downstream_discovery:
501+
- workload_discovery: {}
502+
)EOF");
503+
504+
const auto* peer_info =
505+
stream_info_.filterState()->getDataReadOnly<PeerInfo>(Istio::Common::DownstreamPeerObj);
506+
ASSERT_NE(peer_info, nullptr);
507+
508+
// Test hasFieldSupport
509+
EXPECT_TRUE(peer_info->hasFieldSupport());
510+
511+
// Test getField() for all 9 fields
512+
EXPECT_EQ("foo", std::get<absl::string_view>(peer_info->getField("workload")));
513+
EXPECT_EQ("default", std::get<absl::string_view>(peer_info->getField("namespace")));
514+
EXPECT_EQ("my-cluster", std::get<absl::string_view>(peer_info->getField("cluster")));
515+
EXPECT_EQ("foo-service", std::get<absl::string_view>(peer_info->getField("service")));
516+
EXPECT_EQ("v1alpha3", std::get<absl::string_view>(peer_info->getField("revision")));
517+
EXPECT_EQ("myapp", std::get<absl::string_view>(peer_info->getField("app")));
518+
EXPECT_EQ("v1", std::get<absl::string_view>(peer_info->getField("version")));
519+
EXPECT_EQ("pod", std::get<absl::string_view>(peer_info->getField("type")));
520+
EXPECT_EQ("pod-foo-1234", std::get<absl::string_view>(peer_info->getField("name")));
521+
}
522+
523+
TEST_F(PeerMetadataTest, CelExpressionCompatibility) {
524+
const WorkloadMetadataObject pod("pod-bar-5678", "test-cluster", "production", "bar",
525+
"bar-service", "v2", "barapp", "v2",
526+
Istio::Common::WorkloadType::Pod, "");
527+
EXPECT_CALL(*metadata_provider_, GetMetadata(_))
528+
.WillRepeatedly(Invoke([&](const Network::Address::InstanceConstSharedPtr& address)
529+
-> std::optional<WorkloadMetadataObject> {
530+
if (absl::StartsWith(address->asStringView(), "127.0.0.1")) {
531+
return {pod};
532+
}
533+
return {};
534+
}));
535+
initialize(R"EOF(
536+
downstream_discovery:
537+
- workload_discovery: {}
538+
)EOF");
539+
540+
// Verify CelState is stored under downstream_peer for CEL expressions
541+
const auto* cel_state = stream_info_.filterState()
542+
->getDataReadOnly<Envoy::Extensions::Filters::Common::Expr::CelState>(
543+
Istio::Common::DownstreamPeer);
544+
ASSERT_NE(cel_state, nullptr);
545+
546+
// Verify WorkloadMetadataObject is stored under downstream_peer_obj for FIELD accessor
547+
const auto* peer_info =
548+
stream_info_.filterState()->getDataReadOnly<PeerInfo>(Istio::Common::DownstreamPeerObj);
549+
ASSERT_NE(peer_info, nullptr);
550+
551+
// Test that serializeAsProto still works for CEL compatibility
552+
auto proto = peer_info->serializeAsProto();
553+
ASSERT_NE(proto, nullptr);
554+
555+
// Verify the protobuf contains expected data
556+
const auto* struct_proto = dynamic_cast<const google::protobuf::Struct*>(proto.get());
557+
ASSERT_NE(struct_proto, nullptr);
558+
EXPECT_EQ("production", extractString(*struct_proto, "namespace"));
559+
EXPECT_EQ("bar", extractString(*struct_proto, "workload"));
560+
EXPECT_EQ("test-cluster", extractString(*struct_proto, "cluster"));
561+
}
562+
491563
} // namespace
492564
} // namespace PeerMetadata
493565
} // namespace HttpFilters

0 commit comments

Comments
 (0)