diff --git a/src/core/lib/channel/promise_based_filter.h b/src/core/lib/channel/promise_based_filter.h index 3c0d7b86f86..9029a7480a7 100644 --- a/src/core/lib/channel/promise_based_filter.h +++ b/src/core/lib/channel/promise_based_filter.h @@ -318,6 +318,8 @@ class CallData : public BaseCallData { metadata->Set(GrpcStatusMetadata(), status_code); metadata->Set(GrpcMessageMetadata(), Slice::FromCopiedString(status_details)); + metadata->GetOrCreatePointer(GrpcStatusContext()) + ->emplace_back(grpc_error_string(error)); } // Wakeup and poll the promise if appropriate. diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h index 7fd9d4b48be..8b6904f2c4d 100644 --- a/src/core/lib/transport/metadata_batch.h +++ b/src/core/lib/transport/metadata_batch.h @@ -504,6 +504,7 @@ struct LbCostBinMetadata { // Annotation added by a transport to note whether a failed request was never // placed on the wire, or never seen by a server. struct GrpcStreamNetworkState { + static absl::string_view DebugKey() { return "GrpcStreamNetworkState"; } static constexpr bool kRepeatable = false; enum ValueType : uint8_t { kNotSentOnWire, @@ -519,6 +520,14 @@ struct GrpcStreamNetworkState { } }; +// Annotation added by various systems to describe the reason for a failure. +struct GrpcStatusContext { + static absl::string_view DebugKey() { return "GrpcStatusContext"; } + static constexpr bool kRepeatable = true; + using ValueType = std::string; + static const std::string& DisplayValue(const std::string& x) { return x; } +}; + namespace metadata_detail { // IsEncodable: Given a trait, determine if that trait is encodable, or is just @@ -733,6 +742,15 @@ class GetStringValueHelper { std::string* backing_; }; +// Sink for key value logs +using LogFn = absl::FunctionRef; + +template +GPR_ATTRIBUTE_NOINLINE void LogKeyValueTo(absl::string_view key, const T& value, + V (*display_value)(U), LogFn log_fn) { + log_fn(key, absl::StrCat(display_value(value))); +} + // Generate a strong type for metadata values per trait. template struct Value; @@ -756,6 +774,9 @@ struct ValueEncode(Which(), value); } + void LogTo(LogFn log_fn) const { + LogKeyValueTo(Which::key(), value, Which::DisplayValue, log_fn); + } using StorageType = typename Which::ValueType; GPR_NO_UNIQUE_ADDRESS StorageType value; }; @@ -777,12 +798,17 @@ struct Value void EncodeTo(Encoder*) const {} + void LogTo(LogFn log_fn) const { + LogKeyValueTo(Which::DebugKey(), value, Which::DisplayValue, log_fn); + } using StorageType = typename Which::ValueType; GPR_NO_UNIQUE_ADDRESS StorageType value; }; template -struct Value> { +struct Value::value, + void>> { Value() = default; explicit Value(const typename Which::ValueType& value) { this->value.push_back(value); @@ -803,28 +829,42 @@ struct Value> { encoder->Encode(Which(), v); } } + void LogTo(LogFn log_fn) const { + for (const auto& v : value) { + LogKeyValueTo(Which::key(), v, Which::DisplayValue, log_fn); + } + } using StorageType = absl::InlinedVector; StorageType value; }; -// Encoder to log some metadata -class LogEncoder { - public: - explicit LogEncoder( - absl::FunctionRef log_fn) - : log_fn_(log_fn) {} - - template - void Encode(Which, const typename Which::ValueType& value) { - log_fn_(Which::key(), absl::StrCat(Which::DisplayValue(value))); +template +struct Value::value, + void>> { + Value() = default; + explicit Value(const typename Which::ValueType& value) { + this->value.push_back(value); } - - void Encode(const Slice& key, const Slice& value) { - log_fn_(key.as_string_view(), value.as_string_view()); + explicit Value(typename Which::ValueType&& value) { + this->value.emplace_back(std::forward(value)); } - - private: - absl::FunctionRef log_fn_; + Value(const Value&) = delete; + Value& operator=(const Value&) = delete; + Value(Value&& other) noexcept : value(std::move(other.value)) {} + Value& operator=(Value&& other) noexcept { + value = std::move(other.value); + return *this; + } + template + void EncodeTo(Encoder*) const {} + void LogTo(LogFn log_fn) const { + for (const auto& v : value) { + LogKeyValueTo(Which::DebugKey(), v, Which::DisplayValue, log_fn); + } + } + using StorageType = absl::InlinedVector; + StorageType value; }; // Encoder to copy some metadata @@ -851,6 +891,55 @@ class CopySink { Output* dst_; }; +// Callable for the ForEach in Encode() -- for each value, call the +// appropriate encoder method. +template +struct EncodeWrapper { + Encoder* encoder; + template + void operator()(const Value& which) { + which.EncodeTo(encoder); + } +}; + +// Callable for the ForEach in Log() +struct LogWrapper { + LogFn log_fn; + template + void operator()(const Value& which) { + which.LogTo(log_fn); + } +}; + +// Encoder to compute TransportSize +class TransportSizeEncoder { + public: + void Encode(const Slice& key, const Slice& value) { + size_ += key.length() + value.length() + 32; + } + + template + void Encode(Which, const typename Which::ValueType& value) { + Add(Which(), value); + } + + void Encode(ContentTypeMetadata, + const typename ContentTypeMetadata::ValueType& value) { + if (value == ContentTypeMetadata::kInvalid) return; + Add(ContentTypeMetadata(), value); + } + + size_t size() const { return size_; } + + private: + template + void Add(Which, const typename Which::ValueType& value) { + size_ += Which::key().length() + Which::Encode(value).length() + 32; + } + + uint32_t size_ = 0; +}; + } // namespace metadata_detail // Helper function for encoders @@ -928,6 +1017,9 @@ MetadataValueAsSlice(typename Which::ValueType value) { // name): // // Traits for the GrpcXyz field: // struct GrpcXyz { +// // The string key that should be used for debug dumps - should not be a +// // valid http2 key (ie all lower case) +// static absl::string_view DebugKey() { return "GRPC_XYZ"; } // // Can this metadata field be repeated? // static constexpr bool kRepeatable = ...; // // The type that's stored on MetadataBatch @@ -985,7 +1077,7 @@ class MetadataMap { // transitions. template void Encode(Encoder* encoder) const { - table_.ForEach(EncodeWrapper{encoder}); + table_.ForEach(metadata_detail::EncodeWrapper{encoder}); for (const auto& unk : unknown_) { encoder->Encode(unk.first, unk.second); } @@ -993,8 +1085,12 @@ class MetadataMap { // Similar to Encode, but targeted at logging: for each metadatum, // call f(key, value) as absl::string_views. - void Log(absl::FunctionRef log_fn) - const; + void Log(metadata_detail::LogFn log_fn) const { + table_.ForEach(metadata_detail::LogWrapper{log_fn}); + for (const auto& unk : unknown_) { + log_fn(unk.first.as_string_view(), unk.second.as_string_view()); + } + } std::string DebugString() const { std::string out; @@ -1149,46 +1245,6 @@ class MetadataMap { template using Value = metadata_detail::Value; - // Callable for the ForEach in Encode() -- for each value, call the - // appropriate encoder method. - template - struct EncodeWrapper { - Encoder* encoder; - template - void operator()(const Value& which) { - which.EncodeTo(encoder); - } - }; - - // Encoder to compute TransportSize - class TransportSizeEncoder { - public: - void Encode(const Slice& key, const Slice& value) { - size_ += key.length() + value.length() + 32; - } - - template - void Encode(Which, const typename Which::ValueType& value) { - Add(Which(), value); - } - - void Encode(ContentTypeMetadata, - const typename ContentTypeMetadata::ValueType& value) { - if (value == ContentTypeMetadata::kInvalid) return; - Add(ContentTypeMetadata(), value); - } - - size_t size() const { return size_; } - - private: - template - void Add(Which, const typename Which::ValueType& value) { - size_ += Which::key().length() + Which::Encode(value).length() + 32; - } - - uint32_t size_ = 0; - }; - void AppendUnknown(absl::string_view key, Slice value) { unknown_.EmplaceBack(Slice::FromCopiedString(key), value.Ref()); } @@ -1258,7 +1314,7 @@ void MetadataMap::Clear() { template size_t MetadataMap::TransportSize() const { - TransportSizeEncoder enc; + metadata_detail::TransportSizeEncoder enc; Encode(&enc); return enc.size(); } @@ -1271,14 +1327,6 @@ Derived MetadataMap::Copy() const { return out; } -template -void MetadataMap::Log( - absl::FunctionRef log_fn) - const { - metadata_detail::LogEncoder enc(log_fn); - Encode(&enc); -} - } // namespace grpc_core struct grpc_metadata_batch; @@ -1300,7 +1348,8 @@ using grpc_metadata_batch_base = grpc_core::MetadataMap< grpc_core::GrpcServerStatsBinMetadata, grpc_core::GrpcTraceBinMetadata, grpc_core::GrpcTagsBinMetadata, grpc_core::GrpcLbClientStatsMetadata, grpc_core::LbCostBinMetadata, grpc_core::LbTokenMetadata, - grpc_core::GrpcStreamNetworkState>; + // Non-encodable things + grpc_core::GrpcStreamNetworkState, grpc_core::GrpcStatusContext>; struct grpc_metadata_batch : public grpc_metadata_batch_base { using grpc_metadata_batch_base::grpc_metadata_batch_base; diff --git a/test/core/transport/metadata_map_test.cc b/test/core/transport/metadata_map_test.cc index 75d1dec12c7..fc0cd2f8a55 100644 --- a/test/core/transport/metadata_map_test.cc +++ b/test/core/transport/metadata_map_test.cc @@ -117,6 +117,7 @@ TEST(MetadataMapTest, NonEncodableTrait) { GrpcStreamNetworkState::kNotSentOnWire); EncoderWithNoTraitEncodeFunctions encoder; map.Encode(&encoder); + EXPECT_EQ(map.DebugString(), "GrpcStreamNetworkState: not sent on wire"); } } // namespace testing