From 8fe4518885f77cf0d7090e56c5f0d74efe4d5fcb Mon Sep 17 00:00:00 2001
From: Zhewei Hu <zhu@pinterest.com>
Date: Fri, 1 Sep 2023 09:29:42 -0700
Subject: [PATCH] Address comments

Signed-off-by: Zhewei Hu <zhu@pinterest.com>
---
 .../filters/network/zookeeper_proxy/decoder.h |  4 +--
 .../filters/network/zookeeper_proxy/filter.cc | 34 +++++--------------
 .../filters/network/zookeeper_proxy/filter.h  |  3 --
 3 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.h b/source/extensions/filters/network/zookeeper_proxy/decoder.h
index dd3f0ebedf3b..34ad9b4d32ff 100644
--- a/source/extensions/filters/network/zookeeper_proxy/decoder.h
+++ b/source/extensions/filters/network/zookeeper_proxy/decoder.h
@@ -80,7 +80,6 @@ class DecoderCallbacks {
   virtual ~DecoderCallbacks() = default;
 
   virtual void onDecodeError() PURE;
-  virtual void onDefaultRequestBytes(const OpCodes opcode, const uint64_t bytes) PURE;
   virtual void onRequestBytes(const absl::optional<OpCodes> opcode, const uint64_t bytes) PURE;
   virtual void onConnect(bool readonly) PURE;
   virtual void onPing() PURE;
@@ -105,7 +104,6 @@ class DecoderCallbacks {
   virtual void onCheckWatchesRequest(const std::string& path, int32_t type) PURE;
   virtual void onRemoveWatchesRequest(const std::string& path, int32_t type) PURE;
   virtual void onCloseRequest() PURE;
-  virtual void onDefaultResponseBytes(const OpCodes opcode, const uint64_t bytes) PURE;
   virtual void onResponseBytes(const absl::optional<OpCodes> opcode, const uint64_t bytes) PURE;
   virtual void onConnectResponse(int32_t proto_version, int32_t timeout, bool readonly,
                                  const std::chrono::milliseconds latency) PURE;
@@ -156,7 +154,7 @@ class DecoderImpl : public Decoder, Logger::Loggable<Logger::Id::filter> {
   void decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype,
                              Buffer::OwnedImpl& zk_filter_buffer);
   void decode(Buffer::Instance& data, DecodeType dtype, uint64_t full_packets_len);
-  // The return value of decodeOnData and decodeOnWrite will be ZooKeeper opcode or absl::nullopt.
+  // decodeOnData and decodeOnWrite return ZooKeeper opcode or absl::nullopt.
   // absl::nullopt indicates WATCH_XID, which is generated by the server and has no corresponding
   // opcode.
   absl::optional<OpCodes> decodeOnData(Buffer::Instance& data, uint64_t& offset);
diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.cc b/source/extensions/filters/network/zookeeper_proxy/filter.cc
index 34e29bac1882..013bea03b1e0 100644
--- a/source/extensions/filters/network/zookeeper_proxy/filter.cc
+++ b/source/extensions/filters/network/zookeeper_proxy/filter.cc
@@ -244,48 +244,30 @@ void ZooKeeperFilter::onDecodeError() {
   setDynamicMetadata("opname", "error");
 }
 
-void ZooKeeperFilter::onDefaultRequestBytes(const OpCodes opcode, const uint64_t bytes) {
-  auto it = config_->op_code_map_.find(opcode);
-  ASSERT(it != config_->op_code_map_.end());
-  const ZooKeeperFilterConfig::OpCodeInfo& opcode_info = it->second;
-  opcode_info.rq_bytes_counter_->add(bytes);
-}
-
 void ZooKeeperFilter::onRequestBytes(const absl::optional<OpCodes> opcode, const uint64_t bytes) {
   config_->stats_.request_bytes_.add(bytes);
 
   if (config_->enable_per_opcode_request_bytes_metrics_ && opcode.has_value()) {
-    switch (*opcode) {
-    case OpCodes::Connect:
+    if (*opcode == OpCodes::Connect) {
       config_->stats_.connect_rq_bytes_.add(bytes);
-      break;
-    default:
-      onDefaultRequestBytes(*opcode, bytes);
-      break;
+    } else {
+      ASSERT(config_->op_code_map_.contains(*opcode));
+      config_->op_code_map_[*opcode].rq_bytes_counter_->add(bytes);
     }
   }
 
   setDynamicMetadata("bytes", std::to_string(bytes));
 }
 
-void ZooKeeperFilter::onDefaultResponseBytes(const OpCodes opcode, const uint64_t bytes) {
-  auto it = config_->op_code_map_.find(opcode);
-  ASSERT(it != config_->op_code_map_.end());
-  const ZooKeeperFilterConfig::OpCodeInfo& opcode_info = it->second;
-  opcode_info.resp_bytes_counter_->add(bytes);
-}
-
 void ZooKeeperFilter::onResponseBytes(const absl::optional<OpCodes> opcode, const uint64_t bytes) {
   config_->stats_.response_bytes_.add(bytes);
 
   if (config_->enable_per_opcode_response_bytes_metrics_ && opcode.has_value()) {
-    switch (*opcode) {
-    case OpCodes::Connect:
+    if (*opcode == OpCodes::Connect) {
       config_->stats_.connect_resp_bytes_.add(bytes);
-      break;
-    default:
-      onDefaultResponseBytes(*opcode, bytes);
-      break;
+    } else {
+      ASSERT(config_->op_code_map_.contains(*opcode));
+      config_->op_code_map_[*opcode].resp_bytes_counter_->add(bytes);
     }
   }
 
diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.h b/source/extensions/filters/network/zookeeper_proxy/filter.h
index b313f3f1feee..294d8a3119da 100644
--- a/source/extensions/filters/network/zookeeper_proxy/filter.h
+++ b/source/extensions/filters/network/zookeeper_proxy/filter.h
@@ -373,9 +373,6 @@ class ZooKeeperFilter : public Network::Filter,
   void clearDynamicMetadata();
 
 private:
-  void onDefaultRequestBytes(const OpCodes opcode, const uint64_t bytes) override;
-  void onDefaultResponseBytes(const OpCodes opcode, const uint64_t bytes) override;
-
   Network::ReadFilterCallbacks* read_callbacks_{};
   ZooKeeperFilterConfigSharedPtr config_;
   std::unique_ptr<Decoder> decoder_;