stream info: rename the extended response flag and response flag (env…

…oyproxy#32105) * stream info: rename the extended response flag and response flag Signed-off-by: wbpcode <[email protected]> * minor update Signed-off-by: wbpcode <[email protected]> * address comments Signed-off-by: wbpcode <[email protected]> --------- Signed-off-by: wbpcode <[email protected]>
Winbobob · Feb 3, 2024 · 0053634 · 0053634
1 parent d7fe4ae
commit 0053634
Show file tree

Hide file tree

Showing 62 changed files with 622 additions and 516 deletions.
diff --git a/contrib/client_ssl_auth/filters/network/source/client_ssl_auth.cc b/contrib/client_ssl_auth/filters/network/source/client_ssl_auth.cc
@@ -127,7 +127,7 @@ void ClientSslAuthFilter::onEvent(Network::ConnectionEvent event) {
   if (!config_->allowedPrincipals().allowed(
           read_callbacks_->connection().ssl()->sha256PeerCertificateDigest())) {
     read_callbacks_->connection().streamInfo().setResponseFlag(
-        StreamInfo::ResponseFlag::UpstreamProtocolError);
+        StreamInfo::CoreResponseFlag::UpstreamProtocolError);
     read_callbacks_->connection().streamInfo().setResponseCodeDetails(AuthDigestNoMatch);
     config_->stats().auth_digest_no_match_.inc();
     read_callbacks_->connection().close(Network::ConnectionCloseType::NoFlush);

diff --git a/contrib/client_ssl_auth/filters/network/test/client_ssl_auth_test.cc b/contrib/client_ssl_auth/filters/network/test/client_ssl_auth_test.cc
@@ -164,7 +164,7 @@ TEST_F(ClientSslAuthFilterTest, Ssl) {
   std::string expected_sha_1("digest");
   EXPECT_CALL(*ssl_, sha256PeerCertificateDigest()).WillOnce(ReturnRef(expected_sha_1));
   EXPECT_CALL(filter_callbacks_.connection_.stream_info_,
-              setResponseFlag(StreamInfo::ResponseFlag::UpstreamProtocolError));
+              setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamProtocolError));
   EXPECT_CALL(filter_callbacks_.connection_.stream_info_,
               setResponseCodeDetails("auth_digest_no_match"));
   EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush));

diff --git a/contrib/generic_proxy/filters/network/source/proxy.cc b/contrib/generic_proxy/filters/network/source/proxy.cc
@@ -31,15 +31,15 @@ Tracing::Decision tracingDecision(const Tracing::ConnectionManagerTracingConfig&
   return {Tracing::Reason::NotTraceable, false};
 }
 
-StreamInfo::ResponseFlag
+StreamInfo::CoreResponseFlag
 responseFlagFromDownstreamReasonReason(DownstreamStreamResetReason reason) {
   switch (reason) {
   case DownstreamStreamResetReason::ConnectionTermination:
-    return StreamInfo::ResponseFlag::DownstreamConnectionTermination;
+    return StreamInfo::CoreResponseFlag::DownstreamConnectionTermination;
   case DownstreamStreamResetReason::LocalConnectionTermination:
-    return StreamInfo::ResponseFlag::LocalReset;
+    return StreamInfo::CoreResponseFlag::LocalReset;
   case DownstreamStreamResetReason::ProtocolError:
-    return StreamInfo::ResponseFlag::DownstreamProtocolError;
+    return StreamInfo::CoreResponseFlag::DownstreamProtocolError;
   }
   PANIC("Unknown reset reason");
 }

diff --git a/contrib/generic_proxy/filters/network/source/router/router.cc b/contrib/generic_proxy/filters/network/source/router/router.cc
@@ -555,20 +555,21 @@ void RouterFilter::resetStream(StreamResetReason reason) {
     callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, resetReasonToStringView(reason)));
     break;
   case StreamResetReason::ProtocolError:
-    callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamProtocolError);
+    callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamProtocolError);
     callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, resetReasonToStringView(reason)));
     break;
   case StreamResetReason::ConnectionFailure:
-    callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionFailure);
+    callbacks_->streamInfo().setResponseFlag(
+        StreamInfo::CoreResponseFlag::UpstreamConnectionFailure);
     callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, resetReasonToStringView(reason)));
     break;
   case StreamResetReason::ConnectionTermination:
     callbacks_->streamInfo().setResponseFlag(
-        StreamInfo::ResponseFlag::UpstreamConnectionTermination);
+        StreamInfo::CoreResponseFlag::UpstreamConnectionTermination);
     callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, resetReasonToStringView(reason)));
     break;
   case StreamResetReason::Overflow:
-    callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamOverflow);
+    callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamOverflow);
     callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, resetReasonToStringView(reason)));
     break;
   }
@@ -580,7 +581,7 @@ void RouterFilter::kickOffNewUpstreamRequest() {
   auto thread_local_cluster = cluster_manager_.getThreadLocalCluster(cluster_name);
   if (thread_local_cluster == nullptr) {
     filter_complete_ = true;
-    callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoClusterFound);
+    callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::NoClusterFound);
     callbacks_->sendLocalReply(Status(StatusCode::kNotFound, "cluster_not_found"));
     return;
   }
@@ -612,7 +613,7 @@ void RouterFilter::kickOffNewUpstreamRequest() {
       auto pool_data = thread_local_cluster->tcpConnPool(Upstream::ResourcePriority::Default, this);
       if (!pool_data.has_value()) {
         filter_complete_ = true;
-        callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoHealthyUpstream);
+        callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::NoHealthyUpstream);
         callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, "no_healthy_upstream"));
         return;
       }
@@ -632,7 +633,7 @@ void RouterFilter::kickOffNewUpstreamRequest() {
     auto pool_data = thread_local_cluster->tcpConnPool(Upstream::ResourcePriority::Default, this);
     if (!pool_data.has_value()) {
       filter_complete_ = true;
-      callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoHealthyUpstream);
+      callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::NoHealthyUpstream);
       callbacks_->sendLocalReply(Status(StatusCode::kUnavailable, "no_healthy_upstream"));
       return;
     }
@@ -671,7 +672,7 @@ FilterStatus RouterFilter::onStreamDecoded(StreamRequest& request) {
 
   ENVOY_LOG(debug, "No route for current request and send local reply");
   filter_complete_ = true;
-  callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound);
+  callbacks_->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::NoRouteFound);
   callbacks_->sendLocalReply(Status(StatusCode::kNotFound, "route_not_found"));
   return FilterStatus::StopIteration;
 }

diff --git a/contrib/generic_proxy/filters/network/source/stats.cc b/contrib/generic_proxy/filters/network/source/stats.cc
@@ -18,8 +18,10 @@ CodeOrFlags::CodeOrFlags(Server::Configuration::ServerFactoryContext& context)
     code_stat_names_.push_back(pool_.add(std::to_string(i)));
   }
 
-  for (const auto& flag : StreamInfo::ResponseFlagUtils::CORE_RESPONSE_FLAGS) {
-    flag_stat_names_.emplace(flag.second, pool_.add(flag.first.short_string_));
+  flag_stat_names_.reserve(StreamInfo::ResponseFlagUtils::responseFlagsVec().size());
+
+  for (const auto& flag : StreamInfo::ResponseFlagUtils::responseFlagsVec()) {
+    flag_stat_names_.push_back(pool_.add(flag.short_string_));
   }
 
   unknown_code_or_flag_ = pool_.add("-");
@@ -33,27 +35,10 @@ Stats::StatName CodeOrFlags::statNameFromCode(uint32_t code) const {
 }
 
 Stats::StatName CodeOrFlags::statNameFromFlag(StreamInfo::ResponseFlag flag) const {
-  const auto iter = flag_stat_names_.find(flag);
-  if (iter != flag_stat_names_.end()) {
-    return iter->second;
-  }
-  return unknown_code_or_flag_;
-}
-
-absl::InlinedVector<StreamInfo::ResponseFlag, 2>
-getResponseFlags(const StreamInfo::StreamInfo& info) {
-  if (!info.hasAnyResponseFlag()) {
-    return {};
-  }
-
-  absl::InlinedVector<StreamInfo::ResponseFlag, 2> flags;
-
-  for (const auto& flag : StreamInfo::ResponseFlagUtils::CORE_RESPONSE_FLAGS) {
-    if (info.hasResponseFlag(flag.second)) {
-      flags.push_back(flag.second);
-    }
-  }
-  return flags;
+  // Any flag value should be less than the size of flag_stat_names_. Because flag_stat_names_
+  // is initialized with all possible flags.
+  ASSERT(flag.value() < flag_stat_names_.size());
+  return flag_stat_names_[flag.value()];
 }
 
 void GenericFilterStatsHelper::onRequestReset() { stats_.downstream_rq_reset_.inc(); }
@@ -93,7 +78,7 @@ void GenericFilterStatsHelper::onRequestComplete(const StreamInfo::StreamInfo& i
   }
 
   const auto response_code = info.responseCode().value_or(0);
-  const auto response_flags = getResponseFlags(info);
+  const auto response_flags = info.responseFlags();
 
   if (last_code_counter_.second.has_value() && response_code == last_code_counter_.first) {
     last_code_counter_.second->inc();

diff --git a/contrib/generic_proxy/filters/network/source/stats.h b/contrib/generic_proxy/filters/network/source/stats.h
@@ -28,7 +28,10 @@ class CodeOrFlags : public Singleton::Instance {
   Stats::StatNamePool pool_;
 
   std::vector<Stats::StatName> code_stat_names_;
-  absl::flat_hash_map<StreamInfo::ResponseFlag, Stats::StatName> flag_stat_names_;
+  // The flag_stat_names_ contains stat names of all response flags. The index of each flag
+  // is the same as the value of the flag. Size of this vector is the same as the size of
+  // StreamInfo::ResponseFlagUtils::responseFlagsVec().
+  std::vector<Stats::StatName> flag_stat_names_;
 
   Stats::StatName unknown_code_or_flag_;
 };

diff --git a/contrib/generic_proxy/filters/network/test/proxy_test.cc b/contrib/generic_proxy/filters/network/test/proxy_test.cc
@@ -906,7 +906,7 @@ TEST_F(FilterTest, NewStreamAndReplyNormallyWithDrainClose) {
 
   auto response = std::make_unique<FakeStreamCodecFactory::FakeResponse>();
   response->status_ = {234, false}; // Response non-OK.
-  active_stream->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamProtocolError);
+  active_stream->streamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamProtocolError);
   active_stream->onResponseStart(std::move(response));
 
   EXPECT_EQ(filter_config_->stats().downstream_rq_total_.value(), 1);

diff --git a/envoy/stream_info/stream_info.h b/envoy/stream_info/stream_info.h
@@ -36,7 +36,7 @@ using ClusterInfoConstSharedPtr = std::shared_ptr<const ClusterInfo>;
 
 namespace StreamInfo {
 
-enum ResponseFlag : uint16_t {
+enum CoreResponseFlag : uint16_t {
   // Local server healthcheck failed.
   FailedLocalHealthCheck,
   // No healthy upstream.
@@ -99,38 +99,34 @@ enum ResponseFlag : uint16_t {
 
 class ResponseFlagUtils;
 
-// TODO(wbpcode): rename the ExtendedResponseFlag to ResponseFlag and legacy
-// ResponseFlag to CoreResponseFlag.
-class ExtendedResponseFlag {
+class ResponseFlag {
 public:
-  ExtendedResponseFlag() = default;
+  constexpr ResponseFlag() = default;
 
   /**
    * Construct a response flag from the core response flag enum. The integer
    * value of the enum is used as the raw integer value of the flag.
    * @param flag the core response flag enum.
    */
-  ExtendedResponseFlag(ResponseFlag flag) : raw_value_(flag) {}
+  constexpr ResponseFlag(CoreResponseFlag flag) : value_(flag) {}
 
   /**
    * Get the raw integer value of the flag.
    * @return uint16_t the raw integer value.
    */
-  uint16_t value() const { return raw_value_; }
+  uint16_t value() const { return value_; }
 
-  bool operator==(const ExtendedResponseFlag& other) const {
-    return raw_value_ == other.raw_value_;
-  }
+  bool operator==(const ResponseFlag& other) const { return value_ == other.value_; }
 
 private:
   friend class ResponseFlagUtils;
 
   // This private constructor is used to create extended response flags from
   // uint16_t values. This can only be used by ResponseFlagUtils to ensure
   // only validated values are used.
-  ExtendedResponseFlag(uint16_t value) : raw_value_(value) {}
+  ResponseFlag(uint16_t value) : value_(value) {}
 
-  uint16_t raw_value_{};
+  uint16_t value_{};
 };
 
 /**
@@ -632,7 +628,7 @@ class StreamInfo {
    * @param response_flag the response flag. Each filter can set independent response flags. The
    * flags are accumulated.
    */
-  virtual void setResponseFlag(ExtendedResponseFlag response_flag) PURE;
+  virtual void setResponseFlag(ResponseFlag response_flag) PURE;
 
   /**
    * @param code the HTTP response code to set for this request.
@@ -787,7 +783,7 @@ class StreamInfo {
   /**
    * @return whether response flag is set or not.
    */
-  virtual bool hasResponseFlag(ExtendedResponseFlag response_flag) const PURE;
+  virtual bool hasResponseFlag(ResponseFlag response_flag) const PURE;
 
   /**
    * @return whether any response flag is set or not.
@@ -797,11 +793,11 @@ class StreamInfo {
   /**
    * @return all response flags that are set.
    */
-  virtual absl::Span<const ExtendedResponseFlag> responseFlags() const PURE;
+  virtual absl::Span<const ResponseFlag> responseFlags() const PURE;
 
   /**
    * @return response flags encoded as an integer. Every bit of the integer is used to represent a
-   * flag. Only flags that are declared in the enum ResponseFlag type are supported.
+   * flag. Only flags that are declared in the enum CoreResponseFlag type are supported.
    */
   virtual uint64_t legacyResponseFlags() const PURE;
 

diff --git a/mobile/library/common/types/c_types.h b/mobile/library/common/types/c_types.h
@@ -211,7 +211,7 @@ typedef struct {
   uint64_t received_byte_count;
   // The final response flags for the stream. See
   // https://github.com/envoyproxy/envoy/blob/main/envoy/stream_info/stream_info.h
-  // for the ResponseFlag enum.
+  // for the CoreResponseFlag enum.
   uint64_t response_flags;
   // The upstream protocol, if an upstream connection was established. Field
   // entries are based off of Envoy's Http::Protocol

diff --git a/mobile/test/common/http/filters/test_read/filter.cc b/mobile/test/common/http/filters/test_read/filter.cc
@@ -34,7 +34,7 @@ Http::FilterHeadersStatus TestReadFilter::decodeHeaders(Http::RequestHeaderMap&
   return Http::FilterHeadersStatus::Continue;
 }
 
-StreamInfo::ResponseFlag TestReadFilter::mapErrorToResponseFlag(uint64_t errorCode) {
+StreamInfo::CoreResponseFlag TestReadFilter::mapErrorToResponseFlag(uint64_t errorCode) {
   switch (errorCode) {
   case 0x4000000:
     return StreamInfo::DnsResolutionFailed;

diff --git a/mobile/test/common/http/filters/test_read/filter.h b/mobile/test/common/http/filters/test_read/filter.h
@@ -27,7 +27,7 @@ class TestReadFilter final : public Http::PassThroughFilter,
   /* A mapping of the envoymobile errors we care about for testing
    * From https://github.com/envoyproxy/envoy/blob/main/envoy/stream_info/stream_info.h
    */
-  StreamInfo::ResponseFlag mapErrorToResponseFlag(uint64_t errorCode);
+  StreamInfo::CoreResponseFlag mapErrorToResponseFlag(uint64_t errorCode);
 };
 
 } // namespace TestRead

diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc
@@ -114,7 +114,8 @@ void CodecClient::onEvent(Network::ConnectionEvent event) {
       reason = StreamResetReason::ConnectionTermination;
       if (protocol_error_) {
         reason = StreamResetReason::ProtocolError;
-        connection_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamProtocolError);
+        connection_->streamInfo().setResponseFlag(
+            StreamInfo::CoreResponseFlag::UpstreamProtocolError);
       }
     }
     while (!active_requests_.empty()) {