decompressor library: add stats to zlib library

docs/root/configuration/http/http_filters/decompressor_filter.rst

@@ -110,3 +110,6 @@ with the following:
   not_decompressed, Counter, Number of request/responses not compressed.
   total_uncompressed_bytes, Counter, The total uncompressed bytes of all the request/responses that were marked for decompression.
   total_compressed_bytes, Counter, The total compressed bytes of all the request/responses that were marked for decompression.
+
+Additional stats for the decompressor library are rooted at
+<stat_prefix>.decompressor.<decompressor_library.name>.<decompressor_library_stat_prefix>.decompressor_library.

include/envoy/compression/decompressor/factory.h

@@ -10,7 +10,7 @@ class DecompressorFactory {
 public:
   virtual ~DecompressorFactory() = default;
 
-  virtual DecompressorPtr createDecompressor() PURE;
+  virtual DecompressorPtr createDecompressor(const std::string& stats_prefix) PURE;
   virtual const std::string& statsPrefix() const PURE;
   // TODO(junr03): this method assumes that decompressors are used on http messages.
   // A more generic method might be `hint()` which gives the user of the decompressor a hint about

source/extensions/compression/common/decompressor/factory_base.h

@@ -17,7 +17,8 @@ class DecompressorLibraryFactoryBase
                                      Server::Configuration::FactoryContext& context) override {
     return createDecompressorFactoryFromProtoTyped(
         MessageUtil::downcastAndValidate<const ConfigProto&>(proto_config,
-                                                             context.messageValidationVisitor()));
+                                                             context.messageValidationVisitor()),
+        context);
   }
 
   ProtobufTypes::MessagePtr createEmptyConfigProto() override {
@@ -31,7 +32,8 @@ class DecompressorLibraryFactoryBase
 
 private:
   virtual Envoy::Compression::Decompressor::DecompressorFactoryPtr
-  createDecompressorFactoryFromProtoTyped(const ConfigProto&) PURE;
+  createDecompressorFactoryFromProtoTyped(const ConfigProto& proto_config,
+                                          Server::Configuration::FactoryContext& context) PURE;
 
   const std::string name_;
 };

source/extensions/compression/gzip/decompressor/BUILD

@@ -16,6 +16,8 @@ envoy_cc_library(
     external_deps = ["zlib"],
     deps = [
         "//include/envoy/compression/decompressor:decompressor_interface",
+        "//include/envoy/stats:stats_interface",
+        "//include/envoy/stats:stats_macros",
         "//source/common/buffer:buffer_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:minimal_logger_lib",

source/extensions/compression/gzip/decompressor/config.cc

@@ -15,21 +15,24 @@ const uint32_t GzipHeaderValue = 16;
 } // namespace
 
 GzipDecompressorFactory::GzipDecompressorFactory(
-    const envoy::extensions::compression::gzip::decompressor::v3::Gzip& gzip)
-    : window_bits_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(gzip, window_bits, DefaultWindowBits) |
+    const envoy::extensions::compression::gzip::decompressor::v3::Gzip& gzip, Stats::Scope& scope)
+    : scope_(scope),
+      window_bits_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(gzip, window_bits, DefaultWindowBits) |
                    GzipHeaderValue),
       chunk_size_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(gzip, chunk_size, DefaultChunkSize)) {}
 
-Envoy::Compression::Decompressor::DecompressorPtr GzipDecompressorFactory::createDecompressor() {
-  auto decompressor = std::make_unique<ZlibDecompressorImpl>(chunk_size_);
+Envoy::Compression::Decompressor::DecompressorPtr
+GzipDecompressorFactory::createDecompressor(const std::string& stats_prefix) {
+  auto decompressor = std::make_unique<ZlibDecompressorImpl>(scope_, stats_prefix, chunk_size_);
   decompressor->init(window_bits_);
   return decompressor;
 }
 
 Envoy::Compression::Decompressor::DecompressorFactoryPtr
 GzipDecompressorLibraryFactory::createDecompressorFactoryFromProtoTyped(
-    const envoy::extensions::compression::gzip::decompressor::v3::Gzip& proto_config) {
-  return std::make_unique<GzipDecompressorFactory>(proto_config);
+    const envoy::extensions::compression::gzip::decompressor::v3::Gzip& proto_config,
+    Server::Configuration::FactoryContext& context) {
+  return std::make_unique<GzipDecompressorFactory>(proto_config, context.scope());
 }
 
 /**

source/extensions/compression/gzip/decompressor/config.h

@@ -25,16 +25,19 @@ const std::string& gzipExtensionName() {
 
 class GzipDecompressorFactory : public Envoy::Compression::Decompressor::DecompressorFactory {
 public:
-  GzipDecompressorFactory(const envoy::extensions::compression::gzip::decompressor::v3::Gzip& gzip);
+  GzipDecompressorFactory(const envoy::extensions::compression::gzip::decompressor::v3::Gzip& gzip,
+                          Stats::Scope& scope);
 
   // Envoy::Compression::Decompressor::DecompressorFactory
-  Envoy::Compression::Decompressor::DecompressorPtr createDecompressor() override;
+  Envoy::Compression::Decompressor::DecompressorPtr
+  createDecompressor(const std::string& stats_prefix) override;
   const std::string& statsPrefix() const override { return gzipStatsPrefix(); }
   const std::string& contentEncoding() const override {
     return Http::Headers::get().ContentEncodingValues.Gzip;
   }
 
 private:
+  Stats::Scope& scope_;
   const int32_t window_bits_;
   const uint32_t chunk_size_;
 };
@@ -47,7 +50,8 @@ class GzipDecompressorLibraryFactory
 
 private:
   Envoy::Compression::Decompressor::DecompressorFactoryPtr createDecompressorFactoryFromProtoTyped(
-      const envoy::extensions::compression::gzip::decompressor::v3::Gzip& config) override;
+      const envoy::extensions::compression::gzip::decompressor::v3::Gzip& proto_config,
+      Server::Configuration::FactoryContext& context) override;
 };
 
 DECLARE_FACTORY(GzipDecompressorLibraryFactory);

source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.cc

@@ -1,5 +1,7 @@
 #include "extensions/compression/gzip/decompressor/zlib_decompressor_impl.h"
 
+#include <zlib.h>
+
 #include <memory>
 
 #include "envoy/common/exception.h"
@@ -14,13 +16,17 @@ namespace Compression {
 namespace Gzip {
 namespace Decompressor {
 
-ZlibDecompressorImpl::ZlibDecompressorImpl() : ZlibDecompressorImpl(4096) {}
+ZlibDecompressorImpl::ZlibDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix)
+    : ZlibDecompressorImpl(scope, stats_prefix, 4096) {}
 
-ZlibDecompressorImpl::ZlibDecompressorImpl(uint64_t chunk_size)
-    : Zlib::Base(chunk_size, [](z_stream* z) {
-        inflateEnd(z);
-        delete z;
-      }) {
+ZlibDecompressorImpl::ZlibDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix,
+                                           uint64_t chunk_size)
+    : Zlib::Base(chunk_size,
+                 [](z_stream* z) {
+                   inflateEnd(z);
+                   delete z;
+                 }),
+      stats_(generateStats(stats_prefix, scope)) {
   zstream_ptr_->zalloc = Z_NULL;
   zstream_ptr_->zfree = Z_NULL;
   zstream_ptr_->opaque = Z_NULL;
@@ -67,10 +73,30 @@ bool ZlibDecompressorImpl::inflateNext() {
 
   if (result < 0) {
     decompression_error_ = result;
-    ENVOY_LOG(
-        trace,
-        "zlib decompression error: {}. Error codes are defined in https://www.zlib.net/manual.html",
-        result);
+    ENVOY_LOG(trace,
+              "zlib decompression error: {}, msg: {}. Error codes are defined in "
+              "https://www.zlib.net/manual.html",
+              result, zstream_ptr_->msg);
+    switch (result) {
+    case Z_ERRNO:
+      stats_.zlib_errno_.inc();
+      break;
+    case Z_STREAM_ERROR:
+      stats_.zlib_stream_error_.inc();
+      break;
+    case Z_DATA_ERROR:
+      stats_.zlib_data_error_.inc();
+      break;
+    case Z_MEM_ERROR:
+      stats_.zlib_mem_error_.inc();
+      break;
+    case Z_BUF_ERROR:
+      stats_.zlib_buf_error_.inc();
+      break;
+    case Z_VERSION_ERROR:
+      stats_.zlib_version_error_.inc();
+      break;
+    }
     return false;
   }
 

source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.h

@@ -1,6 +1,8 @@
 #pragma once
 
 #include "envoy/compression/decompressor/decompressor.h"
+#include "envoy/stats/scope.h"
+#include "envoy/stats/stats_macros.h"
 
 #include "common/common/logger.h"
 
@@ -14,14 +16,32 @@ namespace Compression {
 namespace Gzip {
 namespace Decompressor {
 
+/**
+ * All zlib decompressor stats. @see stats_macros.h
+ */
+#define ALL_ZLIB_DECOMPRESSOR_STATS(COUNTER)                                                       \
+  COUNTER(zlib_errno)                                                                              \
+  COUNTER(zlib_stream_error)                                                                       \
+  COUNTER(zlib_data_error)                                                                         \
+  COUNTER(zlib_mem_error)                                                                          \
+  COUNTER(zlib_buf_error)                                                                          \
+  COUNTER(zlib_version_error)
+
+/**
+ * Struct definition for zlib decompressor stats. @see stats_macros.h
+ */
+struct ZlibDecompressorStats {
+  ALL_ZLIB_DECOMPRESSOR_STATS(GENERATE_COUNTER_STRUCT)
+};
+
 /**
  * Implementation of decompressor's interface.
  */
 class ZlibDecompressorImpl : public Zlib::Base,
                              public Envoy::Compression::Decompressor::Decompressor,
                              public Logger::Loggable<Logger::Id::decompression> {
 public:
-  ZlibDecompressorImpl();
+  ZlibDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix);
 
   /**
    * Constructor that allows setting the size of decompressor's output buffer. It
@@ -31,7 +51,7 @@ class ZlibDecompressorImpl : public Zlib::Base,
    * 256K bytes. @see http://zlib.net/zlib_how.html
    * @param chunk_size amount of memory reserved for the decompressor output.
    */
-  ZlibDecompressorImpl(uint64_t chunk_size);
+  ZlibDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix, uint64_t chunk_size);
 
   /**
    * Init must be called in order to initialize the decompressor. Once decompressor is initialized,
@@ -49,7 +69,13 @@ class ZlibDecompressorImpl : public Zlib::Base,
   int decompression_error_{0};
 
 private:
+  static ZlibDecompressorStats generateStats(const std::string& prefix, Stats::Scope& scope) {
+    return ZlibDecompressorStats{ALL_ZLIB_DECOMPRESSOR_STATS(POOL_COUNTER_PREFIX(scope, prefix))};
+  }
+
   bool inflateNext();
+
+  const ZlibDecompressorStats stats_;
 };
 
 } // namespace Decompressor

source/extensions/filters/http/decompressor/decompressor_filter.cc

@@ -17,6 +17,7 @@ DecompressorFilterConfig::DecompressorFilterConfig(
     : stats_prefix_(fmt::format("{}decompressor.{}.{}", stats_prefix,
                                 proto_config.decompressor_library().name(),
                                 decompressor_factory->statsPrefix())),
+      decompressor_stats_prefix_(stats_prefix_ + "decompressor_library"),
       decompressor_factory_(std::move(decompressor_factory)),
       request_direction_config_(proto_config.request_direction_config(), stats_prefix_, scope,
                                 runtime),

source/extensions/filters/http/decompressor/decompressor_filter.h

@@ -94,14 +94,15 @@ class DecompressorFilterConfig {
       Compression::Decompressor::DecompressorFactoryPtr decompressor_factory);
 
   Compression::Decompressor::DecompressorPtr makeDecompressor() {
-    return decompressor_factory_->createDecompressor();
+    return decompressor_factory_->createDecompressor(decompressor_stats_prefix_);
   }
   const std::string& contentEncoding() { return decompressor_factory_->contentEncoding(); }
   const RequestDirectionConfig& requestDirectionConfig() { return request_direction_config_; }
   const ResponseDirectionConfig& responseDirectionConfig() { return response_direction_config_; }
 
 private:
   const std::string stats_prefix_;
+  const std::string decompressor_stats_prefix_;
   const Compression::Decompressor::DecompressorFactoryPtr decompressor_factory_;
   const RequestDirectionConfig request_direction_config_;
   const ResponseDirectionConfig response_direction_config_;

test/extensions/compression/gzip/compressor_fuzz_test.cc

@@ -1,5 +1,6 @@
 #include "common/buffer/buffer_impl.h"
 #include "common/common/assert.h"
+#include "common/stats/isolated_store_impl.h"
 
 #include "extensions/compression/gzip/compressor/zlib_compressor_impl.h"
 #include "extensions/compression/gzip/decompressor/zlib_decompressor_impl.h"
@@ -19,9 +20,11 @@ namespace Fuzz {
 // trip compress-decompress pair; the decompressor itself is not fuzzed beyond
 // whatever the compressor emits, as it exists only as a test utility today.
 DEFINE_FUZZER(const uint8_t* buf, size_t len) {
+
   FuzzedDataProvider provider(buf, len);
   ZlibCompressorImpl compressor;
-  Decompressor::ZlibDecompressorImpl decompressor;
+  Stats::IsolatedStoreImpl stats_store;
+  Decompressor::ZlibDecompressorImpl decompressor{stats_store, "test"};
 
   // Select target compression level. We can't use ConsumeEnum() since the range
   // is non-contiguous.

test/extensions/compression/gzip/decompressor/BUILD

@@ -18,6 +18,7 @@ envoy_extension_cc_test(
     deps = [
         "//source/common/common:assert_lib",
         "//source/common/common:hex_lib",
+        "//source/common/stats:isolated_store_lib",
         "//source/extensions/compression/gzip/compressor:compressor_lib",
         "//source/extensions/compression/gzip/decompressor:zlib_decompressor_impl_lib",
         "//test/test_common:utility_lib",

test/extensions/compression/gzip/decompressor/zlib_decompressor_impl_test.cc

@@ -1,5 +1,6 @@
 #include "common/buffer/buffer_impl.h"
 #include "common/common/hex.h"
+#include "common/stats/isolated_store_impl.h"
 
 #include "extensions/compression/gzip/compressor/zlib_compressor_impl.h"
 #include "extensions/compression/gzip/decompressor/zlib_decompressor_impl.h"
@@ -46,7 +47,8 @@ class ZlibDecompressorImplTest : public testing::Test {
     drainBuffer(buffer);
     ASSERT_EQ(0, buffer.length());
 
-    ZlibDecompressorImpl decompressor;
+    Stats::IsolatedStoreImpl stats_store{};
+    ZlibDecompressorImpl decompressor{stats_store, "test."};
     decompressor.init(window_bits);
 
     decompressor.decompress(accumulation_buffer, buffer);
@@ -66,17 +68,20 @@ class ZlibDecompressorImplTest : public testing::Test {
 class ZlibDecompressorImplFailureTest : public ZlibDecompressorImplTest {
 protected:
   static void decompressorBadInitTestHelper(int64_t window_bits) {
-    ZlibDecompressorImpl decompressor;
+    Stats::IsolatedStoreImpl stats_store{};
+    ZlibDecompressorImpl decompressor{stats_store, "test."};
     decompressor.init(window_bits);
   }
 
   static void uninitializedDecompressorTestHelper() {
     Buffer::OwnedImpl input_buffer;
     Buffer::OwnedImpl output_buffer;
-    ZlibDecompressorImpl decompressor;
+    Stats::IsolatedStoreImpl stats_store{};
+    ZlibDecompressorImpl decompressor{stats_store, "test."};
     TestUtility::feedBufferWithRandomCharacters(input_buffer, 100);
     decompressor.decompress(input_buffer, output_buffer);
     ASSERT_TRUE(decompressor.decompression_error_ < 0);
+    ASSERT_EQ(stats_store.counterFromString("test.zlib_stream_error").value(), 1);
   }
 };
 
@@ -105,7 +110,8 @@ TEST_F(ZlibDecompressorImplTest, CallingChecksum) {
   compressor.compress(compressor_buffer, Envoy::Compression::Compressor::State::Flush);
   ASSERT_TRUE(compressor.checksum() > 0);
 
-  ZlibDecompressorImpl decompressor;
+  Stats::IsolatedStoreImpl stats_store{};
+  ZlibDecompressorImpl decompressor{stats_store, "test."};
   decompressor.init(gzip_window_bits);
   EXPECT_EQ(0, decompressor.checksum());
 
@@ -150,7 +156,8 @@ TEST_F(ZlibDecompressorImplTest, CompressAndDecompress) {
   drainBuffer(buffer);
   ASSERT_EQ(0, buffer.length());
 
-  ZlibDecompressorImpl decompressor;
+  Stats::IsolatedStoreImpl stats_store{};
+  ZlibDecompressorImpl decompressor{stats_store, "test."};
   decompressor.init(gzip_window_bits);
 
   decompressor.decompress(accumulation_buffer, buffer);
@@ -180,12 +187,14 @@ TEST_F(ZlibDecompressorImplTest, FailedDecompression) {
     accumulation_buffer.add(buffer);
     drainBuffer(buffer);
   }
-  ZlibDecompressorImpl decompressor;
+  Stats::IsolatedStoreImpl stats_store{};
+  ZlibDecompressorImpl decompressor{stats_store, "test."};
   decompressor.init(gzip_window_bits);
 
   decompressor.decompress(accumulation_buffer, buffer);
 
   ASSERT_TRUE(decompressor.decompression_error_ < 0);
+  ASSERT_EQ(stats_store.counterFromString("test.zlib_data_error").value(), 17);
 }
 
 // Exercises decompression with a very small output buffer.
@@ -218,7 +227,8 @@ TEST_F(ZlibDecompressorImplTest, DecompressWithSmallOutputBuffer) {
   drainBuffer(buffer);
   ASSERT_EQ(0, buffer.length());
 
-  ZlibDecompressorImpl decompressor(16);
+  Stats::IsolatedStoreImpl stats_store{};
+  ZlibDecompressorImpl decompressor{stats_store, "test.", 16};
   decompressor.init(gzip_window_bits);
 
   decompressor.decompress(accumulation_buffer, buffer);
@@ -284,7 +294,8 @@ TEST_F(ZlibDecompressorImplTest, CompressDecompressOfMultipleSlices) {
   compressor.compress(buffer, Envoy::Compression::Compressor::State::Flush);
   accumulation_buffer.add(buffer);
 
-  ZlibDecompressorImpl decompressor;
+  Stats::IsolatedStoreImpl stats_store{};
+  ZlibDecompressorImpl decompressor{stats_store, "test."};
   decompressor.init(gzip_window_bits);
 
   drainBuffer(buffer);