diff --git a/api/envoy/extensions/clusters/redis/v3/redis_cluster.proto b/api/envoy/extensions/clusters/redis/v3/redis_cluster.proto index ba8f434d6c74a..91afac08f34fc 100644 --- a/api/envoy/extensions/clusters/redis/v3/redis_cluster.proto +++ b/api/envoy/extensions/clusters/redis/v3/redis_cluster.proto @@ -54,7 +54,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // redirect_refresh_threshold: 10 // [#extension: envoy.clusters.redis] -// [#next-free-field: 7] +// [#next-free-field: 8] message RedisClusterConfig { option (udpa.annotations.versioning).previous_message_type = "envoy.config.cluster.redis.RedisClusterConfig"; @@ -83,4 +83,14 @@ message RedisClusterConfig { // If not set, this defaults to 0, which disables the topology refresh due to degraded or // unhealthy host. uint32 host_degraded_refresh_threshold = 6; + + // Enable zone discovery via INFO command. When enabled, the cluster will + // send INFO command to each node to discover its availability_zone field, + // which is then used for zone-aware routing. + // + // Note: This feature currently works with Valkey only. Valkey exposes + // availability_zone in its INFO response. Standard Redis does not support this field. + // + // If not set, this defaults to false. + google.protobuf.BoolValue enable_zone_discovery = 7; } diff --git a/api/envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.proto b/api/envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.proto index 40cc2858dfbc8..0f9e43397e6f9 100644 --- a/api/envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.proto +++ b/api/envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.proto @@ -33,7 +33,7 @@ message RedisProxy { "envoy.config.filter.network.redis_proxy.v2.RedisProxy"; // Redis connection pool settings. - // [#next-free-field: 11] + // [#next-free-field: 13] message ConnPoolSettings { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.redis_proxy.v2.RedisProxy.ConnPoolSettings"; @@ -60,6 +60,23 @@ message RedisProxy { // Read from any node of the cluster. A random node is selected among the primary and // replicas, healthy nodes have precedent over unhealthy nodes. ANY = 4; + + // Read from replicas in the same availability zone as the Envoy proxy. If no replicas + // are available in the same zone, fall back to any replica. If no replicas are available + // at all, fall back to the primary. + // + // Note: Zone discovery currently works with Valkey only. Valkey exposes availability_zone + // in its INFO response. Standard Redis does not support this field. + // + // The client zone is determined from Envoy's node.locality.zone. + AZ_AFFINITY = 5; + + // Similar to AZ_AFFINITY, but also considers the primary node for same-zone routing. + // Priority order: replicas in same zone -> primary in same zone -> any replica -> primary. + // This is useful when reducing cross-zone traffic is more important than read distribution. + // + // Note: Zone discovery currently works with Valkey only. + AZ_AFFINITY_REPLICAS_AND_PRIMARY = 6; } // Per-operation timeout in milliseconds. The timer starts when the first @@ -134,6 +151,37 @@ message RedisProxy { // storm to busy redis server. This config is a protection to rate limit reconnection rate. // If not set, there will be no rate limiting on the reconnection. ConnectionRateLimit connection_rate_limit = 10; + + // Enable per-shard statistics for tracking hot shard usage. When enabled, the following + // statistics will be emitted per upstream host (shard): + // + // * ``upstream_rq_total``: Total requests to this shard + // * ``upstream_rq_success``: Successful requests to this shard + // * ``upstream_rq_failure``: Failed requests to this shard + // * ``upstream_rq_active``: Active requests to this shard (gauge) + // + // The statistics will be emitted under the scope: + // ``cluster..shard..*`` + // + // .. note:: + // Enabling this option may significantly increase metric cardinality in large clusters + // with many shards. Use with caution in production environments. + bool enable_per_shard_stats = 11; + + // Enable per-shard latency histogram for tracking request latency per upstream host (shard). + // When enabled, the following histogram will be emitted per shard: + // + // * ``upstream_rq_time``: Request latency histogram in microseconds + // + // The histogram will be emitted under the scope: + // ``cluster..shard..upstream_rq_time`` + // + // This option requires ``enable_per_shard_stats`` to be enabled. + // + // .. note:: + // Enabling this option may significantly increase metric cardinality in large clusters + // with many shards. Use with caution in production environments. + bool enable_per_shard_latency_stats = 12; } message PrefixRoutes { diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 9ecf0d6e48ce5..0bfd24670fa74 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -13,5 +13,19 @@ removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` new_features: +- area: redis + change: | + Added zone-aware routing support for Redis Cluster proxy. New read policies + :ref:`AZ_AFFINITY ` + and :ref:`AZ_AFFINITY_REPLICAS_AND_PRIMARY ` + route read requests to replicas in the same availability zone. Zone discovery is enabled via + :ref:`enable_zone_discovery `. + Note: This feature currently works with Valkey only. +- area: tls + change: | + Added TLS certificate compression support (RFC 8879) with brotli, zstd, and zlib algorithms. + Compression reduces TLS handshake size, especially beneficial for QUIC where the ServerHello + needs to fit in the initial response. Enable via runtime guard + ``envoy.reloadable_features.tls_support_certificate_compression`` (defaults to ``false``). deprecated: diff --git a/docs/root/_include/ssl_stats.rst b/docs/root/_include/ssl_stats.rst index fa2f1e0a16bc3..b5033b50c8e80 100644 --- a/docs/root/_include/ssl_stats.rst +++ b/docs/root/_include/ssl_stats.rst @@ -19,3 +19,6 @@ sigalgs., Counter, Total successful TLS connections that used signature algorithm versions., Counter, Total successful TLS connections that used protocol version was_key_usage_invalid, Counter, Total successful TLS connections that used an `invalid keyUsage extension `_. (This is not available in BoringSSL FIPS yet due to `issue #28246 `_) + certificate_compression..compressed, Counter, Total certificates compressed using algorithm (brotli/zstd/zlib). Requires runtime flag ``envoy.reloadable_features.tls_support_certificate_compression``. + certificate_compression..total_uncompressed_bytes, Counter, Total bytes of certificates before compression using algorithm + certificate_compression..total_compressed_bytes, Counter, Total bytes of certificates after compression using algorithm diff --git a/docs/root/intro/arch_overview/other_protocols/redis.rst b/docs/root/intro/arch_overview/other_protocols/redis.rst index 42f851309a653..b2b573d19135c 100644 --- a/docs/root/intro/arch_overview/other_protocols/redis.rst +++ b/docs/root/intro/arch_overview/other_protocols/redis.rst @@ -228,6 +228,7 @@ For details on each command's usage see the official SISMEMBER, Set SMEMBERS, Set SPOP, Set + SPUBLISH Pubsub SRANDMEMBER, Set SREM, Set SCAN, Generic @@ -238,8 +239,11 @@ For details on each command's usage see the official SINTERSTORE, Set SMISMEMBER, Set SMOVE, Set + SSUBSCRIBE Pubsub + SUBSCRIBE, Pubsub SUNION, Set SUNIONSTORE, Set + SUNSUBSCRIBE Pubsub WATCH, String UNWATCH, String ZADD, Sorted Set diff --git a/source/common/formatter/http_specific_formatter.cc b/source/common/formatter/http_specific_formatter.cc index d924af5626e7a..849831f52ebbb 100644 --- a/source/common/formatter/http_specific_formatter.cc +++ b/source/common/formatter/http_specific_formatter.cc @@ -182,6 +182,37 @@ TraceIDFormatter::formatWithContext(const HttpFormatterContext& context, return trace_id; } +namespace { +// Determines if the request is being traced based on stream info. +// This is equivalent to TracerUtility::shouldTraceRequest but inlined +// to avoid dependency on tracer_lib. +bool isTraced(const StreamInfo::StreamInfo& stream_info) { + if (stream_info.healthCheck()) { + return false; + } + switch (stream_info.traceReason()) { + case Tracing::Reason::ClientForced: + case Tracing::Reason::ServiceForced: + case Tracing::Reason::Sampling: + return true; + default: + return false; + } +} +} // namespace + +absl::optional +TraceSampledFormatter::formatWithContext(const HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) const { + return isTraced(stream_info) ? "true" : "false"; +} + +Protobuf::Value +TraceSampledFormatter::formatValueWithContext(const HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) const { + return ValueUtil::stringValue(isTraced(stream_info) ? "true" : "false"); +} + GrpcStatusFormatter::Format GrpcStatusFormatter::parseFormat(absl::string_view format) { if (format.empty() || format == "CAMEL_STRING") { return GrpcStatusFormatter::CamelString; @@ -454,6 +485,11 @@ BuiltInHttpCommandParser::getKnownFormatters() { [](absl::string_view, absl::optional) { return std::make_unique(); }}}, + {"TRACE_SAMPLED", + {CommandSyntaxChecker::COMMAND_ONLY, + [](absl::string_view, absl::optional) { + return std::make_unique(); + }}}, {"QUERY_PARAM", {CommandSyntaxChecker::PARAMS_REQUIRED | CommandSyntaxChecker::LENGTH_ALLOWED, [](absl::string_view format, absl::optional max_length) { diff --git a/source/common/formatter/http_specific_formatter.h b/source/common/formatter/http_specific_formatter.h index f0ba656f3ed64..3e694da54c9dc 100644 --- a/source/common/formatter/http_specific_formatter.h +++ b/source/common/formatter/http_specific_formatter.h @@ -149,6 +149,20 @@ class TraceIDFormatter : public FormatterProvider { const StreamInfo::StreamInfo& stream_info) const override; }; +/** + * FormatterProvider for trace sampled status. + * Uses Envoy's internal tracing decision (stream_info.traceReason()). + * Works at trace origin (e.g., Istio Ingress Gateway) where no incoming traceparent header exists. + */ +class TraceSampledFormatter : public FormatterProvider { +public: + absl::optional + formatWithContext(const HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) const override; + Protobuf::Value formatValueWithContext(const HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) const override; +}; + class GrpcStatusFormatter : public FormatterProvider, HeaderFormatter { public: enum Format { diff --git a/source/common/quic/BUILD b/source/common/quic/BUILD index 3b176bbcf4888..0383acbae8a1a 100644 --- a/source/common/quic/BUILD +++ b/source/common/quic/BUILD @@ -525,6 +525,7 @@ envoy_cc_library( "//envoy/server:transport_socket_config_interface", "//envoy/ssl:context_config_interface", "//source/common/common:assert_lib", + "//source/common/network:raw_buffer_socket_lib", "//source/common/network:transport_socket_options_lib", "//source/common/tls:server_context_config_lib", "//source/common/tls:server_context_lib", @@ -704,13 +705,8 @@ envoy_cc_library( envoy_cc_library( name = "cert_compression_lib", - srcs = envoy_select_enable_http3(["cert_compression.cc"]), hdrs = envoy_select_enable_http3(["cert_compression.h"]), - external_deps = ["ssl"], deps = envoy_select_enable_http3([ - "//bazel/foreign_cc:zlib", - "//source/common/common:assert_lib", - "//source/common/common:logger_lib", - "//source/common/runtime:runtime_lib", + "//source/common/tls:cert_compression_lib", ]), ) diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc index 697e9d29f6739..69ba3f8cd8368 100644 --- a/source/common/quic/active_quic_listener.cc +++ b/source/common/quic/active_quic_listener.cc @@ -65,7 +65,8 @@ ActiveQuicListener::ActiveQuicListener( absl::string_view(reinterpret_cast(random_seed_), sizeof(random_seed_)), quic::QuicRandom::GetInstance(), proof_source_factory.createQuicProofSource( - listen_socket_, listener_config.filterChainManager(), stats_, dispatcher.timeSource()), + listen_socket_, listener_config.filterChainManager(), stats_, dispatcher.timeSource(), + listener_config.listenerScope()), quic::KeyExchangeSource::Default()); auto connection_helper = std::make_unique(dispatcher_); crypto_config_->AddDefaultConfig(random, connection_helper->GetClock(), diff --git a/source/common/quic/cert_compression.cc b/source/common/quic/cert_compression.cc deleted file mode 100644 index d3caecd417c66..0000000000000 --- a/source/common/quic/cert_compression.cc +++ /dev/null @@ -1,119 +0,0 @@ -#include "source/common/quic/cert_compression.h" - -#include "source/common/common/assert.h" -#include "source/common/runtime/runtime_features.h" - -#include "openssl/tls1.h" - -#define ZLIB_CONST -#include "zlib.h" - -namespace Envoy { -namespace Quic { - -namespace { - -class ScopedZStream { -public: - using CleanupFunc = int (*)(z_stream*); - - ScopedZStream(z_stream& z, CleanupFunc cleanup) : z_(z), cleanup_(cleanup) {} - ~ScopedZStream() { cleanup_(&z_); } - -private: - z_stream& z_; - CleanupFunc cleanup_; -}; - -} // namespace - -void CertCompression::registerSslContext(SSL_CTX* ssl_ctx) { - auto ret = SSL_CTX_add_cert_compression_alg(ssl_ctx, TLSEXT_cert_compression_zlib, compressZlib, - decompressZlib); - ASSERT(ret == 1); -} - -int CertCompression::compressZlib(SSL*, CBB* out, const uint8_t* in, size_t in_len) { - - z_stream z = {}; - int rv = deflateInit(&z, Z_DEFAULT_COMPRESSION); - if (rv != Z_OK) { - IS_ENVOY_BUG(fmt::format("Cert compression failure in deflateInit: {}", rv)); - return FAILURE; - } - - ScopedZStream deleter(z, deflateEnd); - - const auto upper_bound = deflateBound(&z, in_len); - - uint8_t* out_buf = nullptr; - if (!CBB_reserve(out, &out_buf, upper_bound)) { - IS_ENVOY_BUG(fmt::format("Cert compression failure in allocating output CBB buffer of size {}", - upper_bound)); - return FAILURE; - } - - z.next_in = in; - z.avail_in = in_len; - z.next_out = out_buf; - z.avail_out = upper_bound; - - rv = deflate(&z, Z_FINISH); - if (rv != Z_STREAM_END) { - IS_ENVOY_BUG(fmt::format( - "Cert compression failure in deflate: {}, z.total_out {}, in_len {}, z.avail_in {}", rv, - z.avail_in, in_len, z.avail_in)); - return FAILURE; - } - - if (!CBB_did_write(out, z.total_out)) { - IS_ENVOY_BUG("CBB_did_write failed"); - return FAILURE; - } - - ENVOY_LOG(trace, "Cert compression successful"); - - return SUCCESS; -} - -int CertCompression::decompressZlib(SSL*, CRYPTO_BUFFER** out, size_t uncompressed_len, - const uint8_t* in, size_t in_len) { - z_stream z = {}; - int rv = inflateInit(&z); - if (rv != Z_OK) { - IS_ENVOY_BUG(fmt::format("Cert decompression failure in inflateInit: {}", rv)); - return FAILURE; - } - - ScopedZStream deleter(z, inflateEnd); - - z.next_in = in; - z.avail_in = in_len; - bssl::UniquePtr decompressed_data( - CRYPTO_BUFFER_alloc(&z.next_out, uncompressed_len)); - z.avail_out = uncompressed_len; - - rv = inflate(&z, Z_FINISH); - if (rv != Z_STREAM_END) { - ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), - "Cert decompression failure in inflate, possibly caused by invalid " - "compressed cert from peer: {}, z.total_out {}, uncompressed_len {}", - rv, z.total_out, uncompressed_len); - return FAILURE; - } - - if (z.total_out != uncompressed_len) { - ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), - "Decompression length did not match peer provided uncompressed length, " - "caused by either invalid peer handshake data or decompression error."); - return FAILURE; - } - - ENVOY_LOG(trace, "Cert decompression successful"); - - *out = decompressed_data.release(); - return SUCCESS; -} - -} // namespace Quic -} // namespace Envoy diff --git a/source/common/quic/cert_compression.h b/source/common/quic/cert_compression.h index 65be63cb8fde0..f49947ef256c6 100644 --- a/source/common/quic/cert_compression.h +++ b/source/common/quic/cert_compression.h @@ -1,30 +1,29 @@ #pragma once -#include "source/common/common/logger.h" - -#include "openssl/ssl.h" +#include "source/common/runtime/runtime_features.h" +#include "source/common/tls/cert_compression.h" namespace Envoy { namespace Quic { -/** - * Support for certificate compression and decompression in QUIC TLS handshakes. This often - * needed for the ServerHello to fit in the initial response and not need an additional round trip - * between client and server. - */ +// QUIC wrapper for TLS certificate compression. class CertCompression : protected Logger::Loggable { public: - // Registers compression and decompression functions on `ssl_ctx` if enabled. - static void registerSslContext(SSL_CTX* ssl_ctx); + using Algorithm = Extensions::TransportSockets::Tls::CertCompression::Algorithm; - // Callbacks for `SSL_CTX_add_cert_compression_alg`. - static int compressZlib(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len); - static int decompressZlib(SSL*, CRYPTO_BUFFER** out, size_t uncompressed_len, const uint8_t* in, - size_t in_len); + static void registerSslContext(SSL_CTX* ssl_ctx, Stats::Scope* scope = nullptr) { + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.tls_support_certificate_compression")) { + Extensions::TransportSockets::Tls::CertCompression::registerAlgorithms( + ssl_ctx, {Algorithm::Brotli, Algorithm::Zstd, Algorithm::Zlib}, scope); + } else { + Extensions::TransportSockets::Tls::CertCompression::registerAlgorithms( + ssl_ctx, {Algorithm::Zlib}, scope); + } + } - // Defined return values for callbacks from `SSL_CTX_add_cert_compression_alg`. - static constexpr int SUCCESS = 1; - static constexpr int FAILURE = 0; + static constexpr int SUCCESS = Extensions::TransportSockets::Tls::CertCompression::SUCCESS; + static constexpr int FAILURE = Extensions::TransportSockets::Tls::CertCompression::FAILURE; }; } // namespace Quic diff --git a/source/common/quic/envoy_quic_proof_source.cc b/source/common/quic/envoy_quic_proof_source.cc index 04be05c68f311..6a77ba1fa9a2e 100644 --- a/source/common/quic/envoy_quic_proof_source.cc +++ b/source/common/quic/envoy_quic_proof_source.cc @@ -2,6 +2,9 @@ #include +#include +#include + #include "envoy/ssl/tls_certificate_config.h" #include "source/common/quic/cert_compression.h" @@ -9,6 +12,8 @@ #include "source/common/quic/quic_io_handle_wrapper.h" #include "source/common/runtime/runtime_features.h" #include "source/common/stream_info/stream_info_impl.h" +#include "source/common/tls/context_config_impl.h" +#include "source/common/network/utility.h" #include "openssl/bytestring.h" #include "quiche/quic/core/crypto/certificate_view.h" @@ -29,7 +34,7 @@ EnvoyQuicProofSource::GetCertChain(const quic::QuicSocketAddress& server_address return nullptr; } - return getTlsCertAndFilterChain(*res, hostname, cert_matched_sni).cert_; + return getTlsCertAndFilterChain(*res, hostname, cert_matched_sni, server_address, client_address).cert_; } void EnvoyQuicProofSource::signPayload( @@ -44,7 +49,7 @@ void EnvoyQuicProofSource::signPayload( } CertWithFilterChain res = - getTlsCertAndFilterChain(*data, hostname, nullptr /* cert_matched_sni */); + getTlsCertAndFilterChain(*data, hostname, nullptr /* cert_matched_sni */, server_address, client_address); if (res.private_key_ == nullptr) { ENVOY_LOG(warn, "No matching filter chain found for handshake."); callback->Run(false, "", nullptr); @@ -74,13 +79,26 @@ void EnvoyQuicProofSource::signPayload( EnvoyQuicProofSource::CertWithFilterChain EnvoyQuicProofSource::getTlsCertAndFilterChain(const TransportSocketFactoryWithFilterChain& data, const std::string& hostname, - bool* cert_matched_sni) { + bool* cert_matched_sni, + const quic::QuicSocketAddress& server_address, + const quic::QuicSocketAddress& client_address) { auto [cert, key] = data.transport_socket_factory_.getTlsCertificateAndKey(hostname, cert_matched_sni); if (cert == nullptr || key == nullptr) { ENVOY_LOG(warn, "No certificate is configured in transport socket config."); return {}; } + + // Cache the keylog configuration and connection info for this filter chain + try { + const auto& context_config = data.transport_socket_factory_.getContextConfig(); + storeKeylogInfo(data.filter_chain_, + std::shared_ptr(&context_config, [](const Ssl::ContextConfig*){}), + server_address, client_address); + } catch (const std::exception& e) { + ENVOY_LOG(debug, "Failed to cache keylog info for filter chain: {}", e.what()); + } + return {std::move(cert), std::move(key), data.filter_chain_}; } @@ -116,7 +134,160 @@ void EnvoyQuicProofSource::updateFilterChainManager( } void EnvoyQuicProofSource::OnNewSslCtx(SSL_CTX* ssl_ctx) { - CertCompression::registerSslContext(ssl_ctx); + CertCompression::registerSslContext(ssl_ctx, &stats_scope_); + + // Try to set up keylog callback for QUIC SSL contexts + setupQuicKeylogCallback(ssl_ctx); +} + +void EnvoyQuicProofSource::setupQuicKeylogCallback(SSL_CTX* ssl_ctx) { + // Store reference to this proof source in SSL_CTX for use in keylog callback + SSL_CTX_set_app_data(ssl_ctx, this); + + // Set up the keylog callback - the actual keylog configuration will be + // determined per-connection in the callback based on the filter chain + SSL_CTX_set_keylog_callback(ssl_ctx, quicKeylogCallback); +} + +// Helper function to convert Envoy address to QUICHE address +quic::QuicSocketAddress envoyAddressToQuicAddress(const Network::Address::Instance& envoy_addr) { + if (envoy_addr.type() == Network::Address::Type::Ip) { + const auto& ip_addr = *envoy_addr.ip(); + quiche::QuicheIpAddress quiche_addr; + if (quiche_addr.FromString(ip_addr.addressAsString())) { + return quic::QuicSocketAddress(quic::QuicIpAddress(quiche_addr), ip_addr.port()); + } + } + // Return any address for non-IP addresses + return quic::QuicSocketAddress(); +} + +// Static keylog callback for QUIC SSL contexts +void EnvoyQuicProofSource::quicKeylogCallback(const SSL* ssl, const char* line) { + ASSERT(ssl != nullptr); + + // Get the proof source instance from SSL_CTX + auto* proof_source = + static_cast(SSL_CTX_get_app_data(SSL_get_SSL_CTX(ssl))); + ASSERT(proof_source != nullptr); + + ENVOY_LOG(debug, "QUIC keylog callback invoked for line: {}", line); + + // Try to find keylog configuration from cached filter chain information + // We iterate through all cached filter chains to find one with keylog configuration + bool keylog_written = false; + { + absl::MutexLock lock(&proof_source->keylog_cache_mutex_); + for (const auto& entry : proof_source->keylog_config_cache_) { + const auto& keylog_info = entry.second; + if (keylog_info.config) { + try { + // Convert QUIC addresses back to Envoy addresses for the bridge + std::string server_addr_str = absl::StrCat( + keylog_info.server_address.host().ToString(), ":", + keylog_info.server_address.port()); + std::string client_addr_str = absl::StrCat( + keylog_info.client_address.host().ToString(), ":", + keylog_info.client_address.port()); + + Network::Address::InstanceConstSharedPtr local_addr = + Network::Utility::parseInternetAddressAndPortNoThrow(server_addr_str); + Network::Address::InstanceConstSharedPtr remote_addr = + Network::Utility::parseInternetAddressAndPortNoThrow(client_addr_str); + + if (local_addr && remote_addr) { + QuicKeylogBridge::writeKeylog(*keylog_info.config, *local_addr, *remote_addr, line); + keylog_written = true; + ENVOY_LOG(debug, "QUIC keylog written using cached configuration"); + break; // Successfully handled by built-in system + } + } catch (const std::exception& e) { + ENVOY_LOG(debug, "Failed to write keylog using cached config: {}", e.what()); + } + } + } + } + + if (keylog_written) { + return; + } + + // Fallback: Use environment variable for backward compatibility + const char* keylog_path = std::getenv("SSLKEYLOGFILE"); + if (keylog_path != nullptr) { + std::ofstream keylog_file(keylog_path, std::ios::app); + if (keylog_file.is_open()) { + keylog_file << line << "\n"; + keylog_file.close(); + ENVOY_LOG(debug, "QUIC keylog written to {}: {}", keylog_path, line); + } + } +} + +void EnvoyQuicProofSource::QuicKeylogBridge::writeKeylog( + const Ssl::ContextConfig& config, + const Network::Address::Instance& local_addr, + const Network::Address::Instance& remote_addr, + const char* line) { + + const std::string& keylog_path = config.tlsKeyLogPath(); + if (keylog_path.empty()) { + return; + } + + // Check address filtering + const auto& local_ip_list = config.tlsKeyLogLocal(); + const auto& remote_ip_list = config.tlsKeyLogRemote(); + + bool local_match = (local_ip_list.getIpListSize() == 0 || local_ip_list.contains(local_addr)); + bool remote_match = (remote_ip_list.getIpListSize() == 0 || remote_ip_list.contains(remote_addr)); + + if (!local_match || !remote_match) { + ENVOY_LOG(debug, "QUIC keylog filtered out by address match (local={}, remote={})", + local_match, remote_match); + return; + } + + // Use access log manager to write keylog + try { + auto& access_log_manager = config.accessLogManager(); + auto file_or_error = access_log_manager.createAccessLog( + Filesystem::FilePathAndType{Filesystem::DestinationType::File, keylog_path}); + + if (file_or_error.ok()) { + auto keylog_file = file_or_error.value(); + keylog_file->write(absl::StrCat(line, "\n")); + ENVOY_LOG(debug, "QUIC keylog written via bridge to {}: {}", keylog_path, line); + } else { + ENVOY_LOG(warn, "Failed to create keylog file {}: {}", keylog_path, + file_or_error.status().message()); + } + } catch (const std::exception& e) { + ENVOY_LOG(warn, "Failed to write QUIC keylog: {}", e.what()); + } +} + +// Get SSL socket index for storing transport socket callbacks +int EnvoyQuicProofSource::sslSocketIndex() { + static int ssl_socket_index = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); + return ssl_socket_index; +} + +void EnvoyQuicProofSource::storeKeylogInfo(const Network::FilterChain& filter_chain, + std::shared_ptr config, + const quic::QuicSocketAddress& server_address, + const quic::QuicSocketAddress& client_address) const { + absl::MutexLock lock(&keylog_cache_mutex_); + keylog_config_cache_[&filter_chain] = KeylogInfo{std::move(config), server_address, client_address}; +} + +absl::optional EnvoyQuicProofSource::getKeylogInfo(const Network::FilterChain& filter_chain) const { + absl::MutexLock lock(&keylog_cache_mutex_); + auto it = keylog_config_cache_.find(&filter_chain); + if (it != keylog_config_cache_.end()) { + return it->second; + } + return absl::nullopt; } } // namespace Quic diff --git a/source/common/quic/envoy_quic_proof_source.h b/source/common/quic/envoy_quic_proof_source.h index 6a9bb62ee255f..cc4f5ac6ba917 100644 --- a/source/common/quic/envoy_quic_proof_source.h +++ b/source/common/quic/envoy_quic_proof_source.h @@ -1,20 +1,37 @@ #pragma once +#include + +#include "envoy/ssl/context_config.h" +#include "envoy/stats/scope.h" + +#include "source/common/common/thread.h" #include "source/common/quic/envoy_quic_proof_source_base.h" #include "source/common/quic/quic_server_transport_socket_factory.h" #include "source/server/listener_stats.h" +#include "absl/synchronization/mutex.h" +#include "absl/types/optional.h" +#include "quiche/quic/platform/api/quic_socket_address.h" + namespace Envoy { namespace Quic { // A ProofSource implementation which supplies a proof instance with certs from filter chain. class EnvoyQuicProofSource : public EnvoyQuicProofSourceBase { public: + // Cache for keylog configurations by filter chain + struct KeylogInfo { + std::shared_ptr config; + quic::QuicSocketAddress server_address; + quic::QuicSocketAddress client_address; + }; EnvoyQuicProofSource(Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) + Server::ListenerStats& listener_stats, TimeSource& time_source, + Stats::Scope& stats_scope) : listen_socket_(listen_socket), filter_chain_manager_(&filter_chain_manager), - listener_stats_(listener_stats), time_source_(time_source) {} + listener_stats_(listener_stats), time_source_(time_source), stats_scope_(stats_scope) {} ~EnvoyQuicProofSource() override = default; @@ -27,6 +44,15 @@ class EnvoyQuicProofSource : public EnvoyQuicProofSourceBase { void updateFilterChainManager(Network::FilterChainManager& filter_chain_manager); + // Bridge interface for QUIC-TLS keylog integration + class QuicKeylogBridge { + public: + static void writeKeylog(const Ssl::ContextConfig& config, + const Network::Address::Instance& local_addr, + const Network::Address::Instance& remote_addr, + const char* line); + }; + protected: // quic::ProofSource void signPayload(const quic::QuicSocketAddress& server_address, @@ -47,17 +73,40 @@ class EnvoyQuicProofSource : public EnvoyQuicProofSourceBase { }; CertWithFilterChain getTlsCertAndFilterChain(const TransportSocketFactoryWithFilterChain& data, - const std::string& hostname, bool* cert_matched_sni); + const std::string& hostname, bool* cert_matched_sni, + const quic::QuicSocketAddress& server_address, + const quic::QuicSocketAddress& client_address); absl::optional getTransportSocketAndFilterChain(const quic::QuicSocketAddress& server_address, const quic::QuicSocketAddress& client_address, const std::string& hostname); + void setupQuicKeylogCallback(SSL_CTX* ssl_ctx); + + // Static callback function for QUIC keylog + static void quicKeylogCallback(const SSL* ssl, const char* line); + + // Get SSL socket index for storing transport socket callbacks + static int sslSocketIndex(); + + // Store keylog configuration and connection info for a filter chain + void storeKeylogInfo(const Network::FilterChain& filter_chain, + std::shared_ptr config, + const quic::QuicSocketAddress& server_address, + const quic::QuicSocketAddress& client_address) const; + + // Get cached keylog information for a filter chain + absl::optional getKeylogInfo(const Network::FilterChain& filter_chain) const; + Network::Socket& listen_socket_; Network::FilterChainManager* filter_chain_manager_{nullptr}; Server::ListenerStats& listener_stats_; TimeSource& time_source_; + Stats::Scope& stats_scope_; + + mutable absl::Mutex keylog_cache_mutex_; + mutable std::unordered_map keylog_config_cache_ ABSL_GUARDED_BY(keylog_cache_mutex_); }; } // namespace Quic diff --git a/source/common/quic/envoy_quic_proof_source_factory_interface.h b/source/common/quic/envoy_quic_proof_source_factory_interface.h index 9de1df10f8f85..b34f4018c2b0e 100644 --- a/source/common/quic/envoy_quic_proof_source_factory_interface.h +++ b/source/common/quic/envoy_quic_proof_source_factory_interface.h @@ -3,6 +3,7 @@ #include "envoy/config/typed_config.h" #include "envoy/network/filter.h" #include "envoy/network/socket.h" +#include "envoy/stats/scope.h" #include "source/server/listener_stats.h" @@ -21,7 +22,8 @@ class EnvoyQuicProofSourceFactoryInterface : public Config::TypedFactory { virtual std::unique_ptr createQuicProofSource(Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) PURE; + Server::ListenerStats& listener_stats, TimeSource& time_source, + Stats::Scope& stats_scope) PURE; }; } // namespace Quic diff --git a/source/common/quic/quic_server_transport_socket_factory.h b/source/common/quic/quic_server_transport_socket_factory.h index 85aaf45a7e5d5..48f5b365e09f9 100644 --- a/source/common/quic/quic_server_transport_socket_factory.h +++ b/source/common/quic/quic_server_transport_socket_factory.h @@ -7,6 +7,7 @@ #include "envoy/ssl/handshaker.h" #include "source/common/common/assert.h" +#include "source/common/network/raw_buffer_socket.h" #include "source/common/network/transport_socket_options_impl.h" #include "source/common/quic/quic_transport_socket_factory.h" #include "source/common/tls/server_ssl_socket.h" @@ -25,8 +26,12 @@ class QuicServerTransportSocketFactory : public Network::DownstreamTransportSock ~QuicServerTransportSocketFactory() override; // Network::DownstreamTransportSocketFactory + // QUIC uses a different transport socket mechanism, but some code paths may call this + // Return a raw buffer socket as a safe fallback Network::TransportSocketPtr createDownstreamTransportSocket() const override { - PANIC("not implemented"); + ENVOY_LOG(warn, "createDownstreamTransportSocket called on QUIC transport socket factory. " + "This should not happen in normal QUIC operation."); + return std::make_unique(); } bool implementsSecureTransport() const override { return true; } @@ -38,6 +43,9 @@ class QuicServerTransportSocketFactory : public Network::DownstreamTransportSock bool earlyDataEnabled() const { return enable_early_data_; } + // Access the TLS context configuration (for keylog integration) + const Ssl::ServerContextConfig& getContextConfig() const { return *config_; } + protected: QuicServerTransportSocketFactory(bool enable_early_data, Stats::Scope& store, Ssl::ServerContextConfigPtr config, diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 9d0156175754d..116b508bd0911 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -114,6 +114,8 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_always_use_v6); FALSE_RUNTIME_GUARD(envoy_restart_features_upstream_http_filters_with_tcp_proxy); // TODO(danzh) false deprecate it once QUICHE has its own enable/disable flag. FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_reject_all); +// TODO: flip to true after sufficient testing of TLS certificate compression. +FALSE_RUNTIME_GUARD(envoy_reloadable_features_tls_support_certificate_compression); // TODO(#10646) change to true when UHV is sufficiently tested // For more information about Universal Header Validation, please see // https://github.com/envoyproxy/envoy/issues/10646 diff --git a/source/common/tls/BUILD b/source/common/tls/BUILD index b4981836f9e9f..041fe390c0f9e 100644 --- a/source/common/tls/BUILD +++ b/source/common/tls/BUILD @@ -182,6 +182,8 @@ envoy_cc_library( # TLS is core functionality. visibility = ["//visibility:public"], deps = [ + ":cert_compression_lib", + ":ssl_ctx_stats_provider_lib", ":stats_lib", ":utility_lib", "//envoy/ssl:context_config_interface", @@ -259,3 +261,28 @@ envoy_cc_library( "//source/common/network:address_lib", ], ) + +envoy_cc_library( + name = "ssl_ctx_stats_provider_lib", + hdrs = ["ssl_ctx_stats_provider.h"], + deps = [ + ":stats_lib", + ], +) + +envoy_cc_library( + name = "cert_compression_lib", + srcs = ["cert_compression.cc"], + hdrs = ["cert_compression.h"], + external_deps = ["ssl"], + deps = [ + ":ssl_ctx_stats_provider_lib", + "//envoy/ssl:context_config_interface", + "//source/common/common:assert_lib", + "//source/common/common:logger_lib", + "//bazel/foreign_cc:zlib", + "//bazel/foreign_cc:zstd", + "@org_brotli//:brotlidec", + "@org_brotli//:brotlienc", + ], +) diff --git a/source/common/tls/cert_compression.cc b/source/common/tls/cert_compression.cc new file mode 100644 index 0000000000000..c57129682b672 --- /dev/null +++ b/source/common/tls/cert_compression.cc @@ -0,0 +1,325 @@ +#include "source/common/tls/cert_compression.h" + +#include "source/common/common/assert.h" +#include "source/common/common/macros.h" +#include "source/common/tls/stats.h" + +#include "brotli/decode.h" +#include "brotli/encode.h" +#include "openssl/tls1.h" +#include "zstd.h" + +#define ZLIB_CONST +#include "zlib.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +namespace { + +class ScopedZStream { +public: + using CleanupFunc = int (*)(z_stream*); + + ScopedZStream(z_stream& z, CleanupFunc cleanup) : z_(z), cleanup_(cleanup) {} + ~ScopedZStream() { cleanup_(&z_); } + +private: + z_stream& z_; + CleanupFunc cleanup_; +}; + +// ex_data index for Stats::Scope* - avoids conflict with QUICHE own SSL_CTX usage. +int getScopeExDataIndex() { + static int index = SSL_CTX_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); + return index; +} + +void recordCompressedCertSize(SSL* ssl, size_t uncompressed_size, size_t compressed_size, + const std::string& algo) { + if (ssl == nullptr) { + return; + } + SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl); + if (ssl_ctx == nullptr) { + return; + } + auto* scope = static_cast(SSL_CTX_get_ex_data(ssl_ctx, getScopeExDataIndex())); + if (scope != nullptr) { + const std::string prefix = "ssl.certificate_compression." + algo + "."; + CertCompressionStats stats = generateCertCompressionStats(*scope, prefix); + stats.compressed_.inc(); + stats.total_uncompressed_bytes_.add(uncompressed_size); + stats.total_compressed_bytes_.add(compressed_size); + } +} + +} // namespace + +void CertCompression::registerBrotli(SSL_CTX* ssl_ctx) { + auto ret = SSL_CTX_add_cert_compression_alg(ssl_ctx, TLSEXT_cert_compression_brotli, + compressBrotli, decompressBrotli); + ASSERT(ret == 1); +} + +void CertCompression::registerZstd(SSL_CTX* ssl_ctx) { + auto ret = SSL_CTX_add_cert_compression_alg(ssl_ctx, TLSEXT_cert_compression_zstd, compressZstd, + decompressZstd); + ASSERT(ret == 1); +} + +void CertCompression::registerZlib(SSL_CTX* ssl_ctx) { + auto ret = SSL_CTX_add_cert_compression_alg(ssl_ctx, TLSEXT_cert_compression_zlib, compressZlib, + decompressZlib); + ASSERT(ret == 1); +} + +void CertCompression::registerAlgorithms(SSL_CTX* ssl_ctx, const std::vector& algorithms, + Stats::Scope* scope) { + if (scope != nullptr) { + SSL_CTX_set_ex_data(ssl_ctx, getScopeExDataIndex(), scope); + } + for (const auto& algo : algorithms) { + switch (algo) { + case Algorithm::Brotli: + registerBrotli(ssl_ctx); + break; + case Algorithm::Zstd: + registerZstd(ssl_ctx); + break; + case Algorithm::Zlib: + registerZlib(ssl_ctx); + break; + } + } +} + +int CertCompression::compressBrotli(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len) { + size_t encoded_size = BrotliEncoderMaxCompressedSize(in_len); + if (encoded_size == 0) { + IS_ENVOY_BUG("BrotliEncoderMaxCompressedSize returned 0"); + return FAILURE; + } + + uint8_t* out_buf = nullptr; + if (!CBB_reserve(out, &out_buf, encoded_size)) { + IS_ENVOY_BUG(fmt::format("Cert compression failure in allocating output CBB buffer of size {}", + encoded_size)); + return FAILURE; + } + + if (BrotliEncoderCompress(BROTLI_DEFAULT_QUALITY, BROTLI_DEFAULT_WINDOW, BROTLI_MODE_GENERIC, + in_len, in, &encoded_size, out_buf) != BROTLI_TRUE) { + IS_ENVOY_BUG("Cert compression failure in BrotliEncoderCompress"); + return FAILURE; + } + + if (!CBB_did_write(out, encoded_size)) { + IS_ENVOY_BUG("CBB_did_write failed"); + return FAILURE; + } + + recordCompressedCertSize(ssl, in_len, encoded_size, "brotli"); + ENVOY_LOG(trace, "Cert brotli compression successful"); + return SUCCESS; +} + +int CertCompression::decompressBrotli(SSL*, CRYPTO_BUFFER** out, size_t uncompressed_len, + const uint8_t* in, size_t in_len) { + uint8_t* out_buf = nullptr; + bssl::UniquePtr decompressed_data(CRYPTO_BUFFER_alloc(&out_buf, uncompressed_len)); + if (!decompressed_data) { + IS_ENVOY_BUG("Failed to allocate CRYPTO_BUFFER for brotli decompression"); + return FAILURE; + } + + size_t decoded_size = uncompressed_len; + BrotliDecoderResult result = BrotliDecoderDecompress(in_len, in, &decoded_size, out_buf); + + if (result != BROTLI_DECODER_RESULT_SUCCESS) { + ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), + "Cert brotli decompression failure, possibly caused by invalid " + "compressed cert from peer: result={}, decoded_size={}, uncompressed_len={}", + static_cast(result), decoded_size, uncompressed_len); + return FAILURE; + } + + if (decoded_size != uncompressed_len) { + ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), + "Brotli decompression length did not match peer provided uncompressed " + "length, caused by either invalid peer handshake data or decompression " + "error: decoded_size={}, uncompressed_len={}", + decoded_size, uncompressed_len); + return FAILURE; + } + + ENVOY_LOG(trace, "Cert brotli decompression successful"); + *out = decompressed_data.release(); + return SUCCESS; +} + +int CertCompression::compressZstd(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len) { + size_t const max_size = ZSTD_compressBound(in_len); + if (max_size == 0) { + IS_ENVOY_BUG("ZSTD_compressBound returned 0"); + return FAILURE; + } + + uint8_t* out_buf = nullptr; + if (!CBB_reserve(out, &out_buf, max_size)) { + IS_ENVOY_BUG(fmt::format("Cert compression failure in allocating output CBB buffer of size {}", + max_size)); + return FAILURE; + } + + size_t const compressed_size = ZSTD_compress(out_buf, max_size, in, in_len, ZSTD_CLEVEL_DEFAULT); + if (ZSTD_isError(compressed_size)) { + IS_ENVOY_BUG( + fmt::format("Cert zstd compression failure: {}", ZSTD_getErrorName(compressed_size))); + return FAILURE; + } + + if (!CBB_did_write(out, compressed_size)) { + IS_ENVOY_BUG("CBB_did_write failed"); + return FAILURE; + } + + recordCompressedCertSize(ssl, in_len, compressed_size, "zstd"); + ENVOY_LOG(trace, "Cert zstd compression successful"); + return SUCCESS; +} + +int CertCompression::decompressZstd(SSL*, CRYPTO_BUFFER** out, size_t uncompressed_len, + const uint8_t* in, size_t in_len) { + uint8_t* out_buf = nullptr; + bssl::UniquePtr decompressed_data(CRYPTO_BUFFER_alloc(&out_buf, uncompressed_len)); + if (!decompressed_data) { + IS_ENVOY_BUG("Failed to allocate CRYPTO_BUFFER for zstd decompression"); + return FAILURE; + } + + size_t const decompressed_size = ZSTD_decompress(out_buf, uncompressed_len, in, in_len); + if (ZSTD_isError(decompressed_size)) { + ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), + "Cert zstd decompression failure, possibly caused by invalid " + "compressed cert from peer: {}, uncompressed_len={}", + ZSTD_getErrorName(decompressed_size), uncompressed_len); + return FAILURE; + } + + if (decompressed_size != uncompressed_len) { + ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), + "Zstd decompression length did not match peer provided uncompressed " + "length, caused by either invalid peer handshake data or decompression " + "error: decompressed_size={}, uncompressed_len={}", + decompressed_size, uncompressed_len); + return FAILURE; + } + + ENVOY_LOG(trace, "Cert zstd decompression successful"); + *out = decompressed_data.release(); + return SUCCESS; +} + +int CertCompression::compressZlib(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len) { + z_stream z = {}; + // The deflateInit macro from zlib.h contains an old-style cast, so we need to suppress the + // warning for this call. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wold-style-cast" + int rv = deflateInit(&z, Z_DEFAULT_COMPRESSION); +#pragma GCC diagnostic pop + if (rv != Z_OK) { + IS_ENVOY_BUG(fmt::format("Cert compression failure in deflateInit: {}", rv)); + return FAILURE; + } + + ScopedZStream deleter(z, deflateEnd); + + const auto upper_bound = deflateBound(&z, in_len); + + uint8_t* out_buf = nullptr; + if (!CBB_reserve(out, &out_buf, upper_bound)) { + IS_ENVOY_BUG(fmt::format("Cert compression failure in allocating output CBB buffer of size {}", + upper_bound)); + return FAILURE; + } + + z.next_in = in; + z.avail_in = in_len; + z.next_out = out_buf; + z.avail_out = upper_bound; + + rv = deflate(&z, Z_FINISH); + if (rv != Z_STREAM_END) { + IS_ENVOY_BUG(fmt::format( + "Cert compression failure in deflate: {}, z.total_out {}, in_len {}, z.avail_in {}", rv, + z.avail_in, in_len, z.avail_in)); + return FAILURE; + } + + if (!CBB_did_write(out, z.total_out)) { + IS_ENVOY_BUG("CBB_did_write failed"); + return FAILURE; + } + + recordCompressedCertSize(ssl, in_len, z.total_out, "zlib"); + ENVOY_LOG(trace, "Cert zlib compression successful"); + return SUCCESS; +} + +int CertCompression::decompressZlib(SSL*, CRYPTO_BUFFER** out, size_t uncompressed_len, + const uint8_t* in, size_t in_len) { + z_stream z = {}; + // The inflateInit macro from zlib.h contains an old-style cast, so we need to suppress the + // warning for this call. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wold-style-cast" + int rv = inflateInit(&z); +#pragma GCC diagnostic pop + if (rv != Z_OK) { + IS_ENVOY_BUG(fmt::format("Cert decompression failure in inflateInit: {}", rv)); + return FAILURE; + } + + ScopedZStream deleter(z, inflateEnd); + + z.next_in = in; + z.avail_in = in_len; + bssl::UniquePtr decompressed_data( + CRYPTO_BUFFER_alloc(&z.next_out, uncompressed_len)); + if (!decompressed_data) { + IS_ENVOY_BUG("Failed to allocate CRYPTO_BUFFER for zlib decompression"); + return FAILURE; + } + z.avail_out = uncompressed_len; + + rv = inflate(&z, Z_FINISH); + if (rv != Z_STREAM_END) { + ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), + "Cert zlib decompression failure, possibly caused by invalid " + "compressed cert from peer: {}, z.total_out {}, uncompressed_len {}", + rv, z.total_out, uncompressed_len); + return FAILURE; + } + + if (z.total_out != uncompressed_len) { + ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10), + "Zlib decompression length did not match peer provided uncompressed " + "length, caused by either invalid peer handshake data or decompression " + "error: z.total_out={}, uncompressed_len={}", + z.total_out, uncompressed_len); + return FAILURE; + } + + ENVOY_LOG(trace, "Cert zlib decompression successful"); + *out = decompressed_data.release(); + return SUCCESS; +} + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/source/common/tls/cert_compression.h b/source/common/tls/cert_compression.h new file mode 100644 index 0000000000000..14a7033b5fe29 --- /dev/null +++ b/source/common/tls/cert_compression.h @@ -0,0 +1,59 @@ +#pragma once + +#include + +#include "envoy/stats/scope.h" + +#include "source/common/common/logger.h" + +#include "openssl/ssl.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +// RFC 8879 Certificate Compression Algorithm IDs. +// TLSEXT_cert_compression_zlib (1) and TLSEXT_cert_compression_brotli (2) are defined in BoringSSL. +// Define zstd (3) locally as BoringSSL does not define it. +#ifndef TLSEXT_cert_compression_zstd +#define TLSEXT_cert_compression_zstd 3 +#endif + +// RFC 8879 TLS Certificate Compression. +class CertCompression : protected Logger::Loggable { +public: + enum class Algorithm : uint16_t { + Zlib = 1, + Brotli = 2, + Zstd = 3, + }; + + // Registers algorithms in the order provided (first = highest priority). + static void registerAlgorithms(SSL_CTX* ssl_ctx, const std::vector& algorithms, + Stats::Scope* scope = nullptr); + + static void registerBrotli(SSL_CTX* ssl_ctx); + static void registerZstd(SSL_CTX* ssl_ctx); + static void registerZlib(SSL_CTX* ssl_ctx); + + static int compressBrotli(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len); + static int decompressBrotli(SSL* ssl, CRYPTO_BUFFER** out, size_t uncompressed_len, + const uint8_t* in, size_t in_len); + + static int compressZstd(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len); + static int decompressZstd(SSL* ssl, CRYPTO_BUFFER** out, size_t uncompressed_len, + const uint8_t* in, size_t in_len); + + static int compressZlib(SSL* ssl, CBB* out, const uint8_t* in, size_t in_len); + static int decompressZlib(SSL* ssl, CRYPTO_BUFFER** out, size_t uncompressed_len, + const uint8_t* in, size_t in_len); + + static constexpr int SUCCESS = 1; + static constexpr int FAILURE = 0; +}; + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/source/common/tls/context_impl.cc b/source/common/tls/context_impl.cc index e32186ddb4c44..b6abcd0a5fef7 100644 --- a/source/common/tls/context_impl.cc +++ b/source/common/tls/context_impl.cc @@ -26,6 +26,7 @@ #include "source/common/protobuf/utility.h" #include "source/common/runtime/runtime_features.h" #include "source/common/stats/utility.h" +#include "source/common/tls/cert_compression.h" #include "source/common/tls/cert_validator/factory.h" #include "source/common/tls/stats.h" #include "source/common/tls/utility.h" @@ -162,6 +163,18 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c return; } } + + // Register certificate compression algorithms to reduce TLS handshake size (RFC 8879). + // Only register if the runtime feature flag is enabled (default: disabled). + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.tls_support_certificate_compression")) { + // Priority: brotli > zstd > zlib (brotli generally provides best compression for certs) + CertCompression::registerAlgorithms(ctx.ssl_ctx_.get(), + {CertCompression::Algorithm::Brotli, + CertCompression::Algorithm::Zstd, + CertCompression::Algorithm::Zlib}, + &scope); + } } auto verify_mode_or_error = cert_validator_->initializeSslContexts( diff --git a/source/common/tls/context_impl.h b/source/common/tls/context_impl.h index 64ace4fd02bf7..c15d1ced89a6c 100644 --- a/source/common/tls/context_impl.h +++ b/source/common/tls/context_impl.h @@ -21,6 +21,7 @@ #include "source/common/stats/symbol_table.h" #include "source/common/tls/cert_validator/cert_validator.h" #include "source/common/tls/context_manager_impl.h" +#include "source/common/tls/ssl_ctx_stats_provider.h" #include "source/common/tls/stats.h" #include "absl/synchronization/mutex.h" @@ -80,6 +81,7 @@ namespace TransportSockets { namespace Tls { class ContextImpl : public virtual Envoy::Ssl::Context, + public SslCtxStatsProvider, protected Logger::Loggable { public: virtual absl::StatusOr> @@ -92,7 +94,8 @@ class ContextImpl : public virtual Envoy::Ssl::Context, */ void logHandshake(SSL* ssl) const; - SslStats& stats() { return stats_; } + SslStats& stats() override { return stats_; } + Stats::Scope& statsScope() override { return scope_; } /** * The global SSL-library index used for storing a pointer to the SslExtendedSocketInfo diff --git a/source/common/tls/ssl_ctx_stats_provider.h b/source/common/tls/ssl_ctx_stats_provider.h new file mode 100644 index 0000000000000..132061e4cd1c6 --- /dev/null +++ b/source/common/tls/ssl_ctx_stats_provider.h @@ -0,0 +1,24 @@ +#pragma once + +#include "envoy/stats/scope.h" + +#include "source/common/tls/stats.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +// Interface for providing SSL stats from SSL_CTX app_data. +// This avoids circular dependency between cert_compression_lib and context_lib. +class SslCtxStatsProvider { +public: + virtual ~SslCtxStatsProvider() = default; + virtual SslStats& stats() = 0; + virtual Stats::Scope& statsScope() = 0; +}; + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/source/common/tls/stats.cc b/source/common/tls/stats.cc index 3840ea17b99bb..3cb404c19c6ee 100644 --- a/source/common/tls/stats.cc +++ b/source/common/tls/stats.cc @@ -22,6 +22,10 @@ Stats::Gauge& createCertificateExpirationGauge(Stats::Scope& scope, const std::s Stats::Gauge::ImportMode::NeverImport); } +CertCompressionStats generateCertCompressionStats(Stats::Scope& scope, const std::string& prefix) { + return CertCompressionStats{CERT_COMPRESSION_STATS(POOL_COUNTER_PREFIX(scope, prefix))}; +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/source/common/tls/stats.h b/source/common/tls/stats.h index cb7112c86c844..c4a0dc23b0567 100644 --- a/source/common/tls/stats.h +++ b/source/common/tls/stats.h @@ -34,6 +34,20 @@ SslStats generateSslStats(Stats::Scope& store); Stats::Gauge& createCertificateExpirationGauge(Stats::Scope& scope, const std::string& cert_name); +/** + * Certificate compression stats per algorithm. @see stats_macros.h + */ +#define CERT_COMPRESSION_STATS(COUNTER) \ + COUNTER(compressed) \ + COUNTER(total_uncompressed_bytes) \ + COUNTER(total_compressed_bytes) + +struct CertCompressionStats { + CERT_COMPRESSION_STATS(GENERATE_COUNTER_STRUCT) +}; + +CertCompressionStats generateCertCompressionStats(Stats::Scope& scope, const std::string& prefix); + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index 5c475c8322b6a..34a22f1b94465 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -24,6 +24,28 @@ absl::StatusOr> RedisCluster::RedisHost return ret; } +absl::StatusOr> +RedisCluster::RedisHost::create(Upstream::ClusterInfoConstSharedPtr cluster, + const std::string& hostname, + Network::Address::InstanceConstSharedPtr address, + RedisCluster& parent, bool primary, const std::string& zone) { + absl::Status creation_status = absl::OkStatus(); + auto ret = std::unique_ptr(new RedisCluster::RedisHost( + cluster, hostname, address, parent, primary, zone, creation_status)); + RETURN_IF_NOT_OK(creation_status); + return ret; +} + +std::shared_ptr +RedisCluster::RedisHost::makeLocalityWithZone( + const envoy::config::core::v3::Locality& base_locality, const std::string& zone) { + auto locality = std::make_shared(base_locality); + if (!zone.empty()) { + locality->set_zone(zone); + } + return locality; +} + absl::StatusOr> RedisCluster::create( const envoy::config::cluster::v3::Cluster& cluster, const envoy::extensions::clusters::redis::v3::RedisClusterConfig& redis_cluster, @@ -69,12 +91,15 @@ RedisCluster::RedisCluster( info(), context.serverFactoryContext().api())), auth_password_(NetworkFilters::RedisProxy::ProtocolOptionsConfigImpl::authPassword( info(), context.serverFactoryContext().api())), - cluster_name_(cluster.name()), refresh_manager_(Common::Redis::getClusterRefreshManager( - context.serverFactoryContext().singletonManager(), - context.serverFactoryContext().mainThreadDispatcher(), - context.serverFactoryContext().clusterManager(), - context.serverFactoryContext().api().timeSource())), - registration_handle_(nullptr) { + cluster_name_(cluster.name()), + refresh_manager_(Common::Redis::getClusterRefreshManager( + context.serverFactoryContext().singletonManager(), + context.serverFactoryContext().mainThreadDispatcher(), + context.serverFactoryContext().clusterManager(), + context.serverFactoryContext().api().timeSource())), + registration_handle_(nullptr), + enable_zone_discovery_( + PROTOBUF_GET_WRAPPED_OR_DEFAULT(redis_cluster, enable_zone_discovery, false)) { const auto& locality_lb_endpoints = load_assignment_.endpoints(); for (const auto& locality_lb_endpoint : locality_lb_endpoints) { for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { @@ -85,21 +110,39 @@ RedisCluster::RedisCluster( } // Register the cluster callback using weak_ptr to avoid use-after-free + // Also capture a pointer to is_destroying_ to check destruction state std::weak_ptr weak_session = redis_discovery_session_; + std::atomic* is_destroying_ptr = &is_destroying_; registration_handle_ = refresh_manager_->registerCluster( cluster_name_, redirect_refresh_interval_, redirect_refresh_threshold_, - failure_refresh_threshold_, host_degraded_refresh_threshold_, [weak_session]() { + failure_refresh_threshold_, host_degraded_refresh_threshold_, + [weak_session, is_destroying_ptr]() { + // Check if cluster is being destroyed first + if (is_destroying_ptr->load(std::memory_order_acquire)) { + return; + } // Try to lock the weak pointer to ensure the session is still alive auto session = weak_session.lock(); if (session && session->resolve_timer_) { session->resolve_timer_->enableTimer(std::chrono::milliseconds(0)); } }); + + // Initialize the session after construction is complete so it can use shared_from_this() + redis_discovery_session_->initialize(); } RedisCluster::~RedisCluster() { // Set flag to prevent any callbacks from executing during destruction - is_destroying_.store(true); + // Use memory_order_release to ensure this write is visible to callbacks + is_destroying_.store(true, std::memory_order_release); + + // CRITICAL: Set the session's parent_destroyed_ flag BEFORE resetting the session. + // This allows callbacks with shared_from_this() to safely check if parent is destroyed + // without accessing the parent object itself (which may be destroyed). + if (redis_discovery_session_) { + redis_discovery_session_->parent_destroyed_.store(true, std::memory_order_release); + } // Reset redis_discovery_session_ before other members are destroyed // to ensure any pending callbacks from refresh_manager_ don't access it. @@ -109,6 +152,11 @@ RedisCluster::~RedisCluster() { // Also clear DNS discovery targets to prevent their callbacks from // accessing the destroyed cluster. dns_discovery_resolve_targets_.clear(); + + // Reset the registration handle LAST to ensure no new callbacks are scheduled + // while we're cleaning up. Any callbacks already scheduled will check is_destroying_ + // and return early. + registration_handle_.reset(); } void RedisCluster::startPreInit() { @@ -138,21 +186,32 @@ void RedisCluster::updateAllHosts(const Upstream::HostVector& hosts_added, hosts_added, hosts_removed, absl::nullopt, absl::nullopt, absl::nullopt); } -void RedisCluster::onClusterSlotUpdate(ClusterSlotsSharedPtr&& slots) { +void RedisCluster::onClusterSlotUpdate(ClusterSlotsSharedPtr&& slots, + const HostZoneMap& host_zone_map) { Upstream::HostVector new_hosts; absl::flat_hash_set all_new_hosts; + // Helper to look up zone from map + auto get_zone = [&host_zone_map](const std::string& address) -> std::string { + auto it = host_zone_map.find(address); + return (it != host_zone_map.end()) ? it->second : ""; + }; + for (const ClusterSlot& slot : *slots) { - if (all_new_hosts.count(slot.primary()->asString()) == 0) { + const std::string primary_address = slot.primary()->asString(); + if (all_new_hosts.count(primary_address) == 0) { + // Pass zone from map to set host's locality.zone new_hosts.emplace_back(THROW_OR_RETURN_VALUE( - RedisHost::create(info(), "", slot.primary(), *this, true), std::unique_ptr)); - all_new_hosts.emplace(slot.primary()->asString()); + RedisHost::create(info(), "", slot.primary(), *this, true, get_zone(primary_address)), + std::unique_ptr)); + all_new_hosts.emplace(primary_address); } for (auto const& replica : slot.replicas()) { if (all_new_hosts.count(replica.first) == 0) { - new_hosts.emplace_back( - THROW_OR_RETURN_VALUE(RedisHost::create(info(), "", replica.second, *this, false), - std::unique_ptr)); + // Pass zone from map to set host's locality.zone + new_hosts.emplace_back(THROW_OR_RETURN_VALUE( + RedisHost::create(info(), "", replica.second, *this, false, get_zone(replica.first)), + std::unique_ptr)); all_new_hosts.emplace(replica.first); } } @@ -228,6 +287,10 @@ RedisCluster::DnsDiscoveryResolveTarget::~DnsDiscoveryResolveTarget() { void RedisCluster::DnsDiscoveryResolveTarget::startResolveDns() { ENVOY_LOG(trace, "starting async DNS resolution for {}", dns_address_); + if (parent_.is_destroying_.load(std::memory_order_acquire) || !parent_.dns_resolver_) { + return; + } + active_query_ = parent_.dns_resolver_->resolve( dns_address_, parent_.dns_lookup_family_, [this](Network::DnsResolver::ResolutionStatus status, absl::string_view, @@ -235,26 +298,34 @@ void RedisCluster::DnsDiscoveryResolveTarget::startResolveDns() { active_query_ = nullptr; ENVOY_LOG(trace, "async DNS resolution complete for {}", dns_address_); if (status == Network::DnsResolver::ResolutionStatus::Failure || response.empty()) { - if (status == Network::DnsResolver::ResolutionStatus::Failure) { - parent_.info_->configUpdateStats().update_failure_.inc(); - } else { - parent_.info_->configUpdateStats().update_empty_.inc(); + auto info = parent_.info_; + if (info) { + if (status == Network::DnsResolver::ResolutionStatus::Failure) { + info->configUpdateStats().update_failure_.inc(); + } else { + info->configUpdateStats().update_empty_.inc(); + } } if (!resolve_timer_) { - resolve_timer_ = parent_.dispatcher_.createTimer([this]() -> void { - // Check if the parent cluster is being destroyed - if (parent_.is_destroying_.load()) { - return; - } - startResolveDns(); - }); + resolve_timer_ = + parent_.dispatcher_.createTimer([this]() -> void { + // Check if the parent cluster is being destroyed + if (parent_.is_destroying_.load()) { + return; + } + startResolveDns(); + }); } // if the initial dns resolved to empty, we'll skip the redis discovery phase and // treat it as an empty cluster. parent_.onPreInitComplete(); resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); } else { + // Check if the parent cluster is being destroyed + if (parent_.is_destroying_.load(std::memory_order_acquire)) { + return; + } // Once the DNS resolve the initial set of addresses, call startResolveRedis on // the RedisDiscoverySession. The RedisDiscoverySession will using the "cluster // slots" command for service discovery and slot allocation. All subsequent @@ -271,18 +342,23 @@ RedisCluster::RedisDiscoverySession::RedisDiscoverySession( Envoy::Extensions::Clusters::Redis::RedisCluster& parent, NetworkFilters::Common::Redis::Client::ClientFactory& client_factory) : parent_(parent), dispatcher_(parent.dispatcher_), - resolve_timer_(parent.dispatcher_.createTimer([this]() -> void { - // Check if the parent cluster is being destroyed - if (parent_.is_destroying_.load()) { - return; - } - startResolveRedis(); - })), + resolve_timer_(nullptr), client_factory_(client_factory), buffer_timeout_(0), redis_command_stats_( NetworkFilters::Common::Redis::RedisCommandStats::createRedisCommandStats( parent_.info()->statsScope().symbolTable())) {} +void RedisCluster::RedisDiscoverySession::initialize() { + // Create timer with shared_from_this() to keep session alive during callbacks + auto self = shared_from_this(); + resolve_timer_ = dispatcher_.createTimer([self]() -> void { + if (!self->isParentAlive()) { + return; + } + self->startResolveRedis(); + }); +} + // Convert the cluster slot IP/Port response to an address, return null if the response // does not match the expected type. Network::Address::InstanceConstSharedPtr @@ -310,6 +386,10 @@ RedisCluster::RedisDiscoverySession::~RedisDiscoverySession() { void RedisCluster::RedisDiscoveryClient::onEvent(Network::ConnectionEvent event) { if (event == Network::ConnectionEvent::RemoteClose || event == Network::ConnectionEvent::LocalClose) { + // Check if the parent cluster is being destroyed + if (parent_.parent_.is_destroying_.load(std::memory_order_acquire)) { + return; + } auto client_to_delete = parent_.client_map_.find(host_); ASSERT(client_to_delete != parent_.client_map_.end()); parent_.dispatcher_.deferredDelete(std::move(client_to_delete->second->client_)); @@ -330,11 +410,17 @@ void RedisCluster::RedisDiscoverySession::registerDiscoveryAddress( } void RedisCluster::RedisDiscoverySession::startResolveRedis() { - parent_.info_->configUpdateStats().update_attempt_.inc(); + // Use helper to safely get parent info (returns nullptr if parent is being destroyed) + auto info = parentInfo(); + if (!info) { + return; + } + + info->configUpdateStats().update_attempt_.inc(); // If a resolution is currently in progress, skip it. if (current_request_) { ENVOY_LOG(debug, "redis cluster slot request is already in progress for '{}'", - parent_.info_->name()); + info ? info->name() : "unknown"); return; } @@ -356,23 +442,33 @@ void RedisCluster::RedisDiscoverySession::startResolveRedis() { if (!client) { client = std::make_unique(*this); client->host_ = current_host_address_; + // Get parent info again in case parent was destroyed between checks + auto parent_info = parentInfo(); + if (!parent_info) { + return; + } // absl::nullopt here disables AWS IAM authentication in redis client which is not supported by // redis cluster implementation client->client_ = client_factory_.create( - host, dispatcher_, shared_from_this(), redis_command_stats_, parent_.info()->statsScope(), + host, dispatcher_, shared_from_this(), redis_command_stats_, parent_info->statsScope(), parent_.auth_username_, parent_.auth_password_, false, absl::nullopt, absl::nullopt); client->client_->addConnectionCallbacks(*client); } - ENVOY_LOG(debug, "executing redis cluster slot request for '{}'", parent_.info_->name()); + ENVOY_LOG(debug, "executing redis cluster slot request for '{}'", + info ? info->name() : "unknown"); current_request_ = client->client_->makeRequest(ClusterSlotsRequest::instance_, *this); } void RedisCluster::RedisDiscoverySession::updateDnsStats( Network::DnsResolver::ResolutionStatus status, bool empty_response) { + auto info = parentInfo(); + if (!info) { + return; + } if (status == Network::DnsResolver::ResolutionStatus::Failure) { - parent_.info_->configUpdateStats().update_failure_.inc(); + info->configUpdateStats().update_failure_.inc(); } else if (empty_response) { - parent_.info_->configUpdateStats().update_empty_.inc(); + info->configUpdateStats().update_empty_.inc(); } } @@ -393,6 +489,10 @@ void RedisCluster::RedisDiscoverySession::updateDnsStats( void RedisCluster::RedisDiscoverySession::resolveClusterHostnames( ClusterSlotsSharedPtr&& slots, std::shared_ptr hostname_resolution_required_cnt) { + if (!isParentAlive() || !parent_.dns_resolver_) { + return; + } + for (uint64_t slot_idx = 0; slot_idx < slots->size(); slot_idx++) { auto& slot = (*slots)[slot_idx]; if (slot.primary() == nullptr) { @@ -404,6 +504,10 @@ void RedisCluster::RedisDiscoverySession::resolveClusterHostnames( [this, slot_idx, slots, hostname_resolution_required_cnt]( Network::DnsResolver::ResolutionStatus status, absl::string_view, std::list&& response) -> void { + if (!isParentAlive()) { + return; + } + auto& slot = (*slots)[slot_idx]; ENVOY_LOG( debug, @@ -451,6 +555,10 @@ void RedisCluster::RedisDiscoverySession::resolveClusterHostnames( void RedisCluster::RedisDiscoverySession::resolveReplicas( ClusterSlotsSharedPtr slots, std::size_t index, std::shared_ptr hostname_resolution_required_cnt) { + if (!isParentAlive() || !parent_.dns_resolver_) { + return; + } + auto& slot = (*slots)[index]; if (slot.replicas_to_resolve_.empty()) { if (*hostname_resolution_required_cnt == 0) { @@ -467,6 +575,11 @@ void RedisCluster::RedisDiscoverySession::resolveReplicas( [this, index, slots, replica_idx, hostname_resolution_required_cnt]( Network::DnsResolver::ResolutionStatus status, absl::string_view, std::list&& response) -> void { + // Check if the parent cluster is being destroyed before accessing any parent members + if (parent_.is_destroying_.load(std::memory_order_acquire)) { + return; + } + auto& slot = (*slots)[index]; auto& replica = slot.replicas_to_resolve_[replica_idx]; ENVOY_LOG(debug, "async DNS resolution complete for replica address {}", replica.first); @@ -495,13 +608,28 @@ void RedisCluster::RedisDiscoverySession::resolveReplicas( void RedisCluster::RedisDiscoverySession::finishClusterHostnameResolution( ClusterSlotsSharedPtr slots) { - parent_.onClusterSlotUpdate(std::move(slots)); - resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + if (!isParentAlive()) { + return; + } + if (parent_.enable_zone_discovery_) { + startZoneDiscovery(std::move(slots)); + } else { + parent_.onClusterSlotUpdate(std::move(slots)); + if (resolve_timer_) { + resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + } + } } void RedisCluster::RedisDiscoverySession::onResponse( NetworkFilters::Common::Redis::RespValuePtr&& value) { - ENVOY_LOG(debug, "redis cluster slot request for '{}' succeeded", parent_.info_->name()); + auto info = parentInfo(); + if (!info) { + current_request_ = nullptr; + return; + } + ENVOY_LOG(debug, "redis cluster slot request for '{}' succeeded", + info ? info->name() : "unknown"); current_request_ = nullptr; const uint32_t SlotRangeStart = 0; @@ -596,8 +724,14 @@ void RedisCluster::RedisDiscoverySession::onResponse( resolveClusterHostnames(std::move(cluster_slots), hostname_resolution_required_cnt); } else { // All slots addresses were represented by IP/Port pairs. - parent_.onClusterSlotUpdate(std::move(cluster_slots)); - resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + if (parent_.enable_zone_discovery_) { + startZoneDiscovery(std::move(cluster_slots)); + } else { + parent_.onClusterSlotUpdate(std::move(cluster_slots)); + if (resolve_timer_) { + resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + } + } } } @@ -627,23 +761,210 @@ bool RedisCluster::RedisDiscoverySession::validateCluster( void RedisCluster::RedisDiscoverySession::onUnexpectedResponse( const NetworkFilters::Common::Redis::RespValuePtr& value) { + // Check if the parent cluster is being destroyed before accessing any parent members + auto info = parentInfo(); + if (!info) { + return; + } + ENVOY_LOG(warn, "Unexpected response to cluster slot command: {}", value->toString()); - this->parent_.info_->configUpdateStats().update_failure_.inc(); - resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + info->configUpdateStats().update_failure_.inc(); + if (resolve_timer_) { + resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + } } void RedisCluster::RedisDiscoverySession::onFailure() { - ENVOY_LOG(debug, "redis cluster slot request for '{}' failed", parent_.info_->name()); current_request_ = nullptr; + + auto info = parentInfo(); + if (!info) { + return; + } + + ENVOY_LOG(debug, "redis cluster slot request for '{}' failed", info->name()); if (!current_host_address_.empty()) { auto client_to_delete = client_map_.find(current_host_address_); client_to_delete->second->client_->close(); } - parent_.info()->configUpdateStats().update_failure_.inc(); + info->configUpdateStats().update_failure_.inc(); + if (resolve_timer_) { + resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + } +} + +// ZoneDiscoveryCallback implementation +void RedisCluster::ZoneDiscoveryCallback::onResponse( + NetworkFilters::Common::Redis::RespValuePtr&& value) { + parent_.onZoneResponse(address_, is_primary_, std::move(value)); +} + +void RedisCluster::ZoneDiscoveryCallback::onFailure() { + parent_.onZoneDiscoveryFailure(address_, is_primary_); +} + +// Zone discovery methods +void RedisCluster::RedisDiscoverySession::startZoneDiscovery(ClusterSlotsSharedPtr slots) { + ENVOY_LOG(debug, "starting zone discovery for '{}' with {} slots", parent_.info_->name(), + slots->size()); + + // Collect unique node addresses + absl::flat_hash_set unique_addresses; + absl::flat_hash_map address_is_primary; // track if address is primary + + for (const ClusterSlot& slot : *slots) { + if (slot.primary()) { + const std::string& addr = slot.primary()->asString(); + if (unique_addresses.find(addr) == unique_addresses.end()) { + unique_addresses.insert(addr); + address_is_primary[addr] = true; + } + } + for (const auto& replica : slot.replicas()) { + if (unique_addresses.find(replica.first) == unique_addresses.end()) { + unique_addresses.insert(replica.first); + address_is_primary[replica.first] = false; + } + } + } + + if (unique_addresses.empty()) { + ENVOY_LOG(debug, "no nodes to discover zones for"); + parent_.onClusterSlotUpdate(std::move(slots)); + resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); + return; + } + + pending_zone_discovery_slots_ = std::move(slots); + pending_zone_requests_.store(unique_addresses.size()); + + // Send INFO command to each unique node + for (const auto& addr : unique_addresses) { + bool is_primary = address_is_primary[addr]; + + // Find existing client or create new one + RedisDiscoveryClientPtr& client = client_map_[addr]; + if (!client) { + // Need to create a new host and client for this address + auto address = Network::Utility::parseInternetAddressAndPortNoThrow(addr, false); + if (!address) { + ENVOY_LOG(warn, "failed to parse address {} for zone discovery", addr); + onZoneDiscoveryFailure(addr, is_primary); + continue; + } + + auto host_or_error = RedisHost::create(parent_.info(), "", address, parent_, is_primary); + if (!host_or_error.ok()) { + ENVOY_LOG(warn, "failed to create host for zone discovery: {}", addr); + onZoneDiscoveryFailure(addr, is_primary); + continue; + } + + // Convert unique_ptr to shared_ptr for the client factory + Upstream::HostSharedPtr host = std::move(*host_or_error); + + client = std::make_unique(*this); + client->host_ = addr; + client->client_ = client_factory_.create( + host, dispatcher_, shared_from_this(), redis_command_stats_, parent_.info()->statsScope(), + parent_.auth_username_, parent_.auth_password_, false, absl::nullopt, absl::nullopt); + client->client_->addConnectionCallbacks(*client); + } + + // Create callback for this zone request + auto callback = std::make_unique(*this, addr, is_primary); + zone_callbacks_[addr] = std::move(callback); + + ENVOY_LOG(debug, "sending INFO command to {} for zone discovery", addr); + auto* request = client->client_->makeRequest(InfoRequest::instance_, *zone_callbacks_[addr]); + if (request) { + zone_requests_[addr] = request; + } else { + ENVOY_LOG(warn, "failed to send INFO command to {}", addr); + onZoneDiscoveryFailure(addr, is_primary); + } + } +} + +void RedisCluster::RedisDiscoverySession::onZoneResponse( + const std::string& address, bool /*is_primary*/, + NetworkFilters::Common::Redis::RespValuePtr&& value) { + ENVOY_LOG(debug, "received zone discovery response from {}", address); + + // Remove from tracking + zone_requests_.erase(address); + zone_callbacks_.erase(address); + + if (value->type() == NetworkFilters::Common::Redis::RespType::BulkString) { + std::string zone = parseAvailabilityZone(value->asString()); + if (!zone.empty()) { + discovered_zones_[address] = zone; + ENVOY_LOG(debug, "discovered zone '{}' for node {}", zone, address); + } + } else { + ENVOY_LOG(warn, "unexpected INFO response type from {}: {}", address, + static_cast(value->type())); + } + + uint32_t remaining = pending_zone_requests_.fetch_sub(1) - 1; + if (remaining == 0) { + finishZoneDiscovery(); + } +} + +void RedisCluster::RedisDiscoverySession::onZoneDiscoveryFailure(const std::string& address, + bool /*is_primary*/) { + ENVOY_LOG(warn, "zone discovery failed for node {}", address); + + // Remove from tracking + zone_requests_.erase(address); + zone_callbacks_.erase(address); + + // Continue even on failure - just won't have zone info for this node + uint32_t remaining = pending_zone_requests_.fetch_sub(1) - 1; + if (remaining == 0) { + finishZoneDiscovery(); + } +} + +void RedisCluster::RedisDiscoverySession::finishZoneDiscovery() { + ENVOY_LOG(debug, "zone discovery complete for '{}'", parent_.info_->name()); + + if (pending_zone_discovery_slots_) { + parent_.onClusterSlotUpdate(std::move(pending_zone_discovery_slots_), discovered_zones_); + pending_zone_discovery_slots_ = nullptr; + discovered_zones_.clear(); // Clear after use + } resolve_timer_->enableTimer(parent_.cluster_refresh_rate_); } +// Parse availability_zone from Valkey INFO response. +// Valkey INFO response format: +// # Server +// valkey_version:8.0.0 +// ... +// availability_zone:us-east-1a +// ... +// Note: Standard Redis does not expose availability_zone. +std::string +RedisCluster::RedisDiscoverySession::parseAvailabilityZone(const std::string& info_response) { + const std::string key = "availability_zone:"; + size_t pos = info_response.find(key); + if (pos == std::string::npos) { + return ""; + } + + pos += key.size(); + size_t end = info_response.find_first_of("\r\n", pos); + if (end == std::string::npos) { + end = info_response.size(); + } + + return info_response.substr(pos, end - pos); +} + RedisCluster::ClusterSlotsRequest RedisCluster::ClusterSlotsRequest::instance_; +RedisCluster::InfoRequest RedisCluster::InfoRequest::instance_; absl::StatusOr> RedisClusterFactory::createClusterWithConfig( diff --git a/source/extensions/clusters/redis/redis_cluster.h b/source/extensions/clusters/redis/redis_cluster.h index 1b12f347ddf88..d4786dc88b7ff 100644 --- a/source/extensions/clusters/redis/redis_cluster.h +++ b/source/extensions/clusters/redis/redis_cluster.h @@ -113,6 +113,20 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { static ClusterSlotsRequest instance_; }; + // INFO command request for zone discovery + struct InfoRequest : public Extensions::NetworkFilters::Common::Redis::RespValue { + public: + InfoRequest() { + type(Extensions::NetworkFilters::Common::Redis::RespType::Array); + std::vector values(1); + values[0].type(NetworkFilters::Common::Redis::RespType::BulkString); + values[0].asString() = "INFO"; + asArray().swap(values); + } + + static InfoRequest instance_; + }; + InitializePhase initializePhase() const override { return InitializePhase::Primary; } /// TimeSource& timeSource() const { return time_source_; } @@ -134,7 +148,7 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { void updateAllHosts(const Upstream::HostVector& hosts_added, const Upstream::HostVector& hosts_removed, uint32_t priority); - void onClusterSlotUpdate(ClusterSlotsSharedPtr&&); + void onClusterSlotUpdate(ClusterSlotsSharedPtr&&, const HostZoneMap& host_zone_map = {}); void reloadHealthyHostsHelper(const Upstream::HostSharedPtr& host) override; @@ -151,11 +165,19 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { // A redis node in the Redis cluster. class RedisHost : public Upstream::HostImpl { public: + // Factory method without zone (for backward compatibility) static absl::StatusOr> create(Upstream::ClusterInfoConstSharedPtr cluster, const std::string& hostname, Network::Address::InstanceConstSharedPtr address, RedisCluster& parent, bool primary); + // Factory method with zone parameter - sets locality.zone on the host + static absl::StatusOr> + create(Upstream::ClusterInfoConstSharedPtr cluster, const std::string& hostname, + Network::Address::InstanceConstSharedPtr address, RedisCluster& parent, bool primary, + const std::string& zone); + protected: + // Constructor without zone RedisHost(Upstream::ClusterInfoConstSharedPtr cluster, const std::string& hostname, Network::Address::InstanceConstSharedPtr address, RedisCluster& parent, bool primary, absl::Status& creation_status) @@ -171,9 +193,29 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { parent.localityLbEndpoint().priority(), parent.lbEndpoint().health_status()), primary_(primary) {} + // Constructor with zone - creates locality with zone set + RedisHost(Upstream::ClusterInfoConstSharedPtr cluster, const std::string& hostname, + Network::Address::InstanceConstSharedPtr address, RedisCluster& parent, bool primary, + const std::string& zone, absl::Status& creation_status) + : Upstream::HostImpl( + creation_status, cluster, hostname, address, + std::make_shared(parent.lbEndpoint().metadata()), + std::make_shared( + parent.localityLbEndpoint().metadata()), + parent.lbEndpoint().load_balancing_weight().value(), + *makeLocalityWithZone(parent.localityLbEndpoint().locality(), zone), + parent.lbEndpoint().endpoint().health_check_config(), + parent.localityLbEndpoint().priority(), parent.lbEndpoint().health_status()), + primary_(primary) {} + bool isPrimary() const { return primary_; } private: + // Helper to create Locality proto with zone set + static std::shared_ptr + makeLocalityWithZone(const envoy::config::core::v3::Locality& base_locality, + const std::string& zone); + const bool primary_; }; @@ -212,6 +254,26 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { using RedisDiscoveryClientPtr = std::unique_ptr; + // Callback handler for zone discovery INFO requests + struct ZoneDiscoveryCallback + : public Extensions::NetworkFilters::Common::Redis::Client::ClientCallbacks { + ZoneDiscoveryCallback(RedisDiscoverySession& parent, const std::string& address, + bool is_primary) + : parent_(parent), address_(address), is_primary_(is_primary) {} + + // Extensions::NetworkFilters::Common::Redis::Client::ClientCallbacks + void onResponse(NetworkFilters::Common::Redis::RespValuePtr&& value) override; + void onFailure() override; + void onRedirection(NetworkFilters::Common::Redis::RespValuePtr&&, const std::string&, + bool) override {} + + RedisDiscoverySession& parent_; + const std::string address_; + const bool is_primary_; + }; + + using ZoneDiscoveryCallbackPtr = std::unique_ptr; + struct RedisDiscoverySession : public Extensions::NetworkFilters::Common::Redis::Client::Config, public Extensions::NetworkFilters::Common::Redis::Client::ClientCallbacks, @@ -221,11 +283,22 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { ~RedisDiscoverySession() override; + // Initialize timer - must be called after construction since it uses shared_from_this() + void initialize(); + void registerDiscoveryAddress(std::list&& response, const uint32_t port); // Start discovery against a random host from existing hosts void startResolveRedis(); + // Zone discovery methods + void startZoneDiscovery(ClusterSlotsSharedPtr slots); + void onZoneResponse(const std::string& address, bool is_primary, + NetworkFilters::Common::Redis::RespValuePtr&& value); + void onZoneDiscoveryFailure(const std::string& address, bool is_primary); + void finishZoneDiscovery(); + static std::string parseAvailabilityZone(const std::string& info_response); + // Extensions::NetworkFilters::Common::Redis::Client::Config bool disableOutlierEvents() const override { return true; } std::chrono::milliseconds opTimeout() const override { @@ -238,6 +311,8 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { std::chrono::milliseconds bufferFlushTimeoutInMs() const override { return buffer_timeout_; } uint32_t maxUpstreamUnknownConnections() const override { return 0; } bool enableCommandStats() const override { return true; } + bool enablePerShardStats() const override { return false; } // Not needed for discovery + bool enablePerShardLatencyStats() const override { return false; } // Not needed for discovery bool connectionRateLimitEnabled() const override { return false; } uint32_t connectionRateLimitPerSec() const override { return 0; } // For any readPolicy other than Primary, the RedisClientFactory will send a READONLY command @@ -266,6 +341,28 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { void finishClusterHostnameResolution(ClusterSlotsSharedPtr slots); void updateDnsStats(Network::DnsResolver::ResolutionStatus status, bool empty_response); + private: + friend class RedisCluster; + friend struct RedisCluster::DnsDiscoveryResolveTarget; + friend struct RedisDiscoveryClient; + // Thread-safe check if parent cluster is being destroyed. + // Returns true if it's safe to proceed with parent operations. + // NOTE: We check our own flag instead of parent_.is_destroying_ because + // parent_ is a reference that becomes dangling after parent is destroyed. + bool isParentAlive() const { + return !parent_destroyed_.load(std::memory_order_acquire); + } + + // Thread-safe accessor for parent cluster info. + // Returns nullptr if parent is being destroyed or info is not available. + // This encapsulates the safety checks needed when accessing parent state from callbacks. + Upstream::ClusterInfoConstSharedPtr parentInfo() const { + if (!isParentAlive()) { + return nullptr; + } + return parent_.info_; + } + RedisCluster& parent_; Event::Dispatcher& dispatcher_; std::string current_host_address_; @@ -278,6 +375,20 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { NetworkFilters::Common::Redis::Client::ClientFactory& client_factory_; const std::chrono::milliseconds buffer_timeout_; NetworkFilters::Common::Redis::RedisCommandStatsSharedPtr redis_command_stats_; + + // Flag set by parent's destructor to signal that parent is being destroyed. + // Callbacks check this flag (owned by session) instead of accessing parent's flag + // to avoid use-after-free when parent is destroyed but callbacks are still queued. + std::atomic parent_destroyed_{false}; + + // Zone discovery state + ClusterSlotsSharedPtr pending_zone_discovery_slots_; + std::atomic pending_zone_requests_{0}; + absl::node_hash_map zone_callbacks_; + absl::node_hash_map + zone_requests_; + HostZoneMap discovered_zones_; // address -> zone mapping from INFO responses }; Upstream::ClusterManager& cluster_manager_; @@ -304,6 +415,7 @@ class RedisCluster : public Upstream::BaseDynamicClusterImpl { const std::string cluster_name_; const Common::Redis::ClusterRefreshManagerSharedPtr refresh_manager_; Common::Redis::ClusterRefreshManager::HandlePtr registration_handle_; + const bool enable_zone_discovery_; // Flag to prevent callbacks during destruction std::atomic is_destroying_{false}; diff --git a/source/extensions/clusters/redis/redis_cluster_lb.cc b/source/extensions/clusters/redis/redis_cluster_lb.cc index cd63e5d05f8d7..c7ceb146098c7 100644 --- a/source/extensions/clusters/redis/redis_cluster_lb.cc +++ b/source/extensions/clusters/redis/redis_cluster_lb.cc @@ -124,6 +124,7 @@ Upstream::HostConstSharedPtr chooseRandomHost(const Upstream::HostSetImpl& host_ return nullptr; } } + } // namespace Upstream::HostSelectionResponse @@ -174,6 +175,50 @@ RedisClusterLoadBalancerFactory::RedisClusterLoadBalancer::chooseHost( } case NetworkFilters::Common::Redis::Client::ReadPolicy::Any: return chooseRandomHost(shard->allHosts(), random_); + case NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity: { + // Spread read requests between replicas in the same AZ in round robin. + // Falls back to other replicas or primary if needed. + const std::string& client_zone = redis_context->clientZone(); + if (!client_zone.empty()) { + const auto* local_replicas = shard->replicasInZone(client_zone); + if (local_replicas) { + auto host = chooseRandomHost(*local_replicas, random_); + if (host) { + return host; + } + } + } + // Fall back to any replica, then primary + if (!shard->replicas().hosts().empty()) { + return chooseRandomHost(shard->replicas(), random_); + } + return shard->primary(); + } + case NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary: { + // Spread read requests among nodes within the client's AZ in round robin, + // prioritizing: local replicas → local primary → any replica → primary. + const std::string& client_zone = redis_context->clientZone(); + if (!client_zone.empty()) { + // Try local replicas first + const auto* local_replicas = shard->replicasInZone(client_zone); + if (local_replicas) { + auto host = chooseRandomHost(*local_replicas, random_); + if (host) { + return host; + } + } + // Try local primary + if (shard->primaryZone() == client_zone && + shard->primary()->coarseHealth() == Upstream::Host::Health::Healthy) { + return shard->primary(); + } + } + // Fall back to any replica, then primary + if (!shard->replicas().hosts().empty()) { + return chooseRandomHost(shard->replicas(), random_); + } + return shard->primary(); + } } } return shard->primary(); @@ -201,10 +246,10 @@ bool RedisLoadBalancerContextImpl::isReadRequest( RedisLoadBalancerContextImpl::RedisLoadBalancerContextImpl( const std::string& key, bool enabled_hashtagging, bool is_redis_cluster, const NetworkFilters::Common::Redis::RespValue& request, - NetworkFilters::Common::Redis::Client::ReadPolicy read_policy) + NetworkFilters::Common::Redis::Client::ReadPolicy read_policy, const std::string& client_zone) : hash_key_(is_redis_cluster ? Crc16::crc16(hashtag(key, true)) : MurmurHash::murmurHash2(hashtag(key, enabled_hashtagging))), - is_read_(isReadRequest(request)), read_policy_(read_policy) {} + is_read_(isReadRequest(request)), read_policy_(read_policy), client_zone_(client_zone) {} // Inspired by the redis-cluster hashtagging algorithm // https://redis.io/topics/cluster-spec#keys-hash-tags @@ -227,8 +272,9 @@ absl::string_view RedisLoadBalancerContextImpl::hashtag(absl::string_view v, boo } RedisSpecifyShardContextImpl::RedisSpecifyShardContextImpl( uint64_t shard_index, const NetworkFilters::Common::Redis::RespValue& request, - NetworkFilters::Common::Redis::Client::ReadPolicy read_policy) - : RedisLoadBalancerContextImpl(std::to_string(shard_index), true, true, request, read_policy), + NetworkFilters::Common::Redis::Client::ReadPolicy read_policy, const std::string& client_zone) + : RedisLoadBalancerContextImpl(std::to_string(shard_index), true, true, request, read_policy, + client_zone), shard_index_(shard_index) {} } // namespace Redis diff --git a/source/extensions/clusters/redis/redis_cluster_lb.h b/source/extensions/clusters/redis/redis_cluster_lb.h index c07e758cf26b6..1a5f465107807 100644 --- a/source/extensions/clusters/redis/redis_cluster_lb.h +++ b/source/extensions/clusters/redis/redis_cluster_lb.h @@ -16,6 +16,7 @@ #include "source/extensions/filters/network/common/redis/supported_commands.h" #include "absl/container/btree_map.h" +#include "absl/container/flat_hash_map.h" #include "absl/synchronization/mutex.h" namespace Envoy { @@ -66,12 +67,21 @@ class ClusterSlot { using ClusterSlotsPtr = std::unique_ptr>; using ClusterSlotsSharedPtr = std::shared_ptr>; +// Map from host address to zone (used during zone discovery) +using HostZoneMap = absl::flat_hash_map; + class RedisLoadBalancerContext { public: virtual ~RedisLoadBalancerContext() = default; virtual bool isReadCommand() const PURE; virtual NetworkFilters::Common::Redis::Client::ReadPolicy readPolicy() const PURE; + + /** + * @return the client zone for zone-aware routing. + * Returns empty string if zone-aware routing is not configured. + */ + virtual const std::string& clientZone() const PURE; }; class RedisLoadBalancerContextImpl : public RedisLoadBalancerContext, @@ -87,12 +97,14 @@ class RedisLoadBalancerContextImpl : public RedisLoadBalancerContext, * will be hashed using crc16. * @param request specify the Redis request. * @param read_policy specify the read policy. + * @param client_zone specify the client zone for zone-aware routing. */ RedisLoadBalancerContextImpl(const std::string& key, bool enabled_hashtagging, bool is_redis_cluster, const NetworkFilters::Common::Redis::RespValue& request, NetworkFilters::Common::Redis::Client::ReadPolicy read_policy = - NetworkFilters::Common::Redis::Client::ReadPolicy::Primary); + NetworkFilters::Common::Redis::Client::ReadPolicy::Primary, + const std::string& client_zone = ""); // Upstream::LoadBalancerContextBase absl::optional computeHashKey() override { return hash_key_; } @@ -103,6 +115,8 @@ class RedisLoadBalancerContextImpl : public RedisLoadBalancerContext, return read_policy_; } + const std::string& clientZone() const override { return client_zone_; } + private: absl::string_view hashtag(absl::string_view v, bool enabled); @@ -111,6 +125,7 @@ class RedisLoadBalancerContextImpl : public RedisLoadBalancerContext, const absl::optional hash_key_; const bool is_read_; const NetworkFilters::Common::Redis::Client::ReadPolicy read_policy_; + const std::string client_zone_; }; class RedisSpecifyShardContextImpl : public RedisLoadBalancerContextImpl { @@ -120,11 +135,13 @@ class RedisSpecifyShardContextImpl : public RedisLoadBalancerContextImpl { * @param shard_index specify the shard index for the Redis request. * @param request specify the Redis request. * @param read_policy specify the read policy. + * @param client_zone specify the client zone for zone-aware routing. */ RedisSpecifyShardContextImpl(uint64_t shard_index, const NetworkFilters::Common::Redis::RespValue& request, NetworkFilters::Common::Redis::Client::ReadPolicy read_policy = - NetworkFilters::Common::Redis::Client::ReadPolicy::Primary); + NetworkFilters::Common::Redis::Client::ReadPolicy::Primary, + const std::string& client_zone = ""); // Upstream::LoadBalancerContextBase absl::optional computeHashKey() override { return shard_index_; } @@ -140,7 +157,7 @@ class ClusterSlotUpdateCallBack { /** * Callback when cluster slot is updated * @param slots provides the updated cluster slots. - * @param all_hosts provides the updated hosts. + * @param all_hosts provides the updated hosts (with zone info already set in host locality). * @return indicate if the cluster slot is updated or not. */ virtual bool onClusterSlotUpdate(ClusterSlotsSharedPtr&& slots, @@ -174,24 +191,60 @@ class RedisClusterLoadBalancerFactory : public ClusterSlotUpdateCallBack, private: class RedisShard { public: + // Constructor derives zone information from host localities RedisShard(Upstream::HostConstSharedPtr primary, Upstream::HostVectorConstSharedPtr replicas, Upstream::HostVectorConstSharedPtr all_hosts, Random::RandomGenerator& random) : primary_(std::move(primary)) { + // Derive primary zone from host's locality + primary_zone_ = primary_->locality().zone(); + replicas_.updateHosts(Upstream::HostSetImpl::partitionHosts( std::move(replicas), Upstream::HostsPerLocalityImpl::empty()), nullptr, {}, {}, random.random()); all_hosts_.updateHosts(Upstream::HostSetImpl::partitionHosts( std::move(all_hosts), Upstream::HostsPerLocalityImpl::empty()), nullptr, {}, {}, random.random()); + + // Group replicas by zone from host localities for efficient zone-aware routing + absl::flat_hash_map zone_hosts; + for (const auto& host : replicas_.hosts()) { + const std::string& zone = host->locality().zone(); + if (!zone.empty()) { + zone_hosts[zone].push_back(host); + } + } + // Convert each zone's hosts to HostSetImpl + for (auto& [zone, hosts] : zone_hosts) { + auto host_set = std::make_unique(0, absl::nullopt, absl::nullopt); + auto hosts_ptr = std::make_shared(std::move(hosts)); + host_set->updateHosts(Upstream::HostSetImpl::partitionHosts( + std::move(hosts_ptr), Upstream::HostsPerLocalityImpl::empty()), + nullptr, {}, {}, random.random()); + replicas_by_zone_[zone] = std::move(host_set); + } } const Upstream::HostConstSharedPtr primary() const { return primary_; } const Upstream::HostSetImpl& replicas() const { return replicas_; } const Upstream::HostSetImpl& allHosts() const { return all_hosts_; } + // Zone information for zone-aware routing + const std::string& primaryZone() const { return primary_zone_; } + + // Get replicas in a specific zone. Returns nullptr if no replicas in that zone. + const Upstream::HostSetImpl* replicasInZone(const std::string& zone) const { + if (zone.empty()) { + return nullptr; + } + auto it = replicas_by_zone_.find(zone); + return (it != replicas_by_zone_.end()) ? it->second.get() : nullptr; + } + private: const Upstream::HostConstSharedPtr primary_; Upstream::HostSetImpl replicas_{0, absl::nullopt, absl::nullopt}; Upstream::HostSetImpl all_hosts_{0, absl::nullopt, absl::nullopt}; + std::string primary_zone_; + absl::flat_hash_map> replicas_by_zone_; }; using RedisShardSharedPtr = std::shared_ptr; diff --git a/source/extensions/filters/network/common/redis/client.h b/source/extensions/filters/network/common/redis/client.h index 6879a12b12d4b..b90212ea63d44 100644 --- a/source/extensions/filters/network/common/redis/client.h +++ b/source/extensions/filters/network/common/redis/client.h @@ -117,7 +117,17 @@ using ClientPtr = std::unique_ptr; /** * Read policy to use for Redis cluster. */ -enum class ReadPolicy { Primary, PreferPrimary, Replica, PreferReplica, Any }; +enum class ReadPolicy { + Primary, + PreferPrimary, + Replica, + PreferReplica, + Any, + // Zone-aware routing: prefer replicas in same AZ, fallback to any replica, then primary + AzAffinity, + // Zone-aware routing: prefer replicas in same AZ, then primary in same AZ, then any + AzAffinityReplicasAndPrimary +}; /** * Configuration for a redis connection pool. @@ -183,6 +193,16 @@ class Config { */ virtual bool enableCommandStats() const PURE; + /** + * @return when enabled, per-shard statistics will be recorded for tracking hot shard usage. + */ + virtual bool enablePerShardStats() const PURE; + + /** + * @return when enabled, per-shard latency histograms will be recorded. + */ + virtual bool enablePerShardLatencyStats() const PURE; + /** * @return the read policy the proxy should use. */ diff --git a/source/extensions/filters/network/common/redis/client_impl.cc b/source/extensions/filters/network/common/redis/client_impl.cc index 92a9f183c11e3..74b4042fcd636 100644 --- a/source/extensions/filters/network/common/redis/client_impl.cc +++ b/source/extensions/filters/network/common/redis/client_impl.cc @@ -41,7 +41,9 @@ ConfigImpl::ConfigImpl( // as the buffer is flushed on each request immediately. max_upstream_unknown_connections_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_upstream_unknown_connections, 100)), - enable_command_stats_(config.enable_command_stats()) { + enable_command_stats_(config.enable_command_stats()), + enable_per_shard_stats_(config.enable_per_shard_stats()), + enable_per_shard_latency_stats_(config.enable_per_shard_latency_stats()) { switch (config.read_policy()) { PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; case envoy::extensions::filters::network::redis_proxy::v3::RedisProxy::ConnPoolSettings::MASTER: @@ -61,6 +63,14 @@ ConfigImpl::ConfigImpl( case envoy::extensions::filters::network::redis_proxy::v3::RedisProxy::ConnPoolSettings::ANY: read_policy_ = ReadPolicy::Any; break; + case envoy::extensions::filters::network::redis_proxy::v3::RedisProxy::ConnPoolSettings:: + AZ_AFFINITY: + read_policy_ = ReadPolicy::AzAffinity; + break; + case envoy::extensions::filters::network::redis_proxy::v3::RedisProxy::ConnPoolSettings:: + AZ_AFFINITY_REPLICAS_AND_PRIMARY: + read_policy_ = ReadPolicy::AzAffinityReplicasAndPrimary; + break; } if (config.has_connection_rate_limit()) { diff --git a/source/extensions/filters/network/common/redis/client_impl.h b/source/extensions/filters/network/common/redis/client_impl.h index a535084033948..01970b5ebee7c 100644 --- a/source/extensions/filters/network/common/redis/client_impl.h +++ b/source/extensions/filters/network/common/redis/client_impl.h @@ -54,6 +54,8 @@ class ConfigImpl : public Config { return max_upstream_unknown_connections_; } bool enableCommandStats() const override { return enable_command_stats_; } + bool enablePerShardStats() const override { return enable_per_shard_stats_; } + bool enablePerShardLatencyStats() const override { return enable_per_shard_latency_stats_; } ReadPolicy readPolicy() const override { return read_policy_; } bool connectionRateLimitEnabled() const override { return connection_rate_limit_enabled_; } uint32_t connectionRateLimitPerSec() const override { return connection_rate_limit_per_sec_; } @@ -66,6 +68,8 @@ class ConfigImpl : public Config { const std::chrono::milliseconds buffer_flush_timeout_; const uint32_t max_upstream_unknown_connections_; const bool enable_command_stats_; + const bool enable_per_shard_stats_; + const bool enable_per_shard_latency_stats_; ReadPolicy read_policy_; bool connection_rate_limit_enabled_; uint32_t connection_rate_limit_per_sec_; diff --git a/source/extensions/filters/network/common/redis/supported_commands.h b/source/extensions/filters/network/common/redis/supported_commands.h index 2bb6e77398fbb..a2681f26e3fd6 100644 --- a/source/extensions/filters/network/common/redis/supported_commands.h +++ b/source/extensions/filters/network/common/redis/supported_commands.h @@ -30,16 +30,17 @@ struct SupportedCommands { "lpop", "lpush", "lpushx", "lrange", "lrem", "lset", "ltrim", "persist", "pexpire", "pexpireat", "pfadd", "pfcount", "psetex", "pttl", "publish", "restore", "rpop", "rpush", "rpushx", "sadd", "scard", "set", "setbit", "setex", "setnx", "setrange", "sismember", - "smembers", "spop", "srandmember", "srem", "sscan", "strlen", "ttl", "type", "xack", "xadd", - "xautoclaim", "xclaim", "xdel", "xlen", "xpending", "xrange", "xrevrange", "xtrim", "zadd", - "zcard", "zcount", "zincrby", "zlexcount", "zpopmin", "zpopmax", "zrange", "zrangebylex", - "zrangebyscore", "zrank", "zrem", "zremrangebylex", "zremrangebyrank", "zremrangebyscore", - "zrevrange", "zrevrangebylex", "zrevrangebyscore", "zrevrank", "zscan", "zscore", "copy", - "rpoplpush", "smove", "sunion", "sdiff", "sinter", "sinterstore", "zunionstore", - "zinterstore", "pfmerge", "georadius", "georadiusbymember", "rename", "sort", "sort_ro", - "zmscore", "sdiffstore", "msetnx", "substr", "zrangestore", "zunion", "zdiff", - "sunionstore", "smismember", "hrandfield", "geosearchstore", "zdiffstore", "zinter", - "zrandmember", "bitop", "lpos", "renamenx"); + "smembers", "spop", "spublish", "srandmember", "srem", "sscan", "ssubscribe", "strlen", + "subscribe", "sunsubscribe", "ttl", "type", "watch", "xack", "xadd", "xautoclaim", "xclaim", + "xdel", "xlen", "xpending", "xrange", "xrevrange", "xtrim", "zadd", "zcard", "zcount", + "zincrby", "zlexcount", "zpopmin", "zpopmax", "zrange", "zrangebylex", "zrangebyscore", + "zrank", "zrem", "zremrangebylex", "zremrangebyrank", "zremrangebyscore", "zrevrange", + "zrevrangebylex", "zrevrangebyscore", "zrevrank", "zscan", "zscore", "copy", "rpoplpush", + "smove", "sunion", "sdiff", "sinter", "sinterstore", "zunionstore", "zinterstore", "pfmerge", + "georadius", "georadiusbymember", "rename", "sort", "sort_ro", "zmscore", "sdiffstore", + "msetnx", "substr", "zrangestore", "zunion", "zdiff", "sunionstore", "smismember", + "hrandfield", "geosearchstore", "zdiffstore", "zinter", "zrandmember", "bitop", "lpos", + "renamenx"); } /** @@ -54,7 +55,8 @@ struct SupportedCommands { * @return commands which hash on the fourth argument */ static const absl::flat_hash_set& evalCommands() { - CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "eval", "evalsha"); + CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "eval", "evalsha", "eval_ro", + "evalsha_ro"); } /** @@ -142,11 +144,11 @@ struct SupportedCommands { "hincrbyfloat", "hmset", "hset", "hsetnx", "incr", "incrby", "incrbyfloat", "linsert", "lmove", "lpop", "lpush", "lpushx", "lrem", "lset", "ltrim", "mset", "multi", "persist", "pexpire", "pexpireat", "pfadd", "psetex", "restore", "rpop", "rpush", "rpushx", "sadd", - "set", "setbit", "setex", "setnx", "setrange", "spop", "srem", "zadd", "zincrby", "touch", - "zpopmin", "zpopmax", "zrem", "zremrangebylex", "zremrangebyrank", "zremrangebyscore", - "unlink", "copy", "rpoplpush", "smove", "sinterstore", "zunionstore", "zinterstore", - "pfmerge", "georadius", "georadiusbymember", "rename", "sort", "sdiffstore", "msetnx", - "zrangestore", "sunionstore", "geosearchstore", "zdiffstore", "bitop", "renamenx"); + "set", "setbit", "setex", "setnx", "setrange", "spop", "spublish", "srem", "zadd", "zincrby", + "touch", "zpopmin", "zpopmax", "zrem", "zremrangebylex", "zremrangebyrank", + "zremrangebyscore", "unlink", "copy", "rpoplpush", "smove", "sinterstore", "zunionstore", + "zinterstore", "pfmerge", "georadius", "georadiusbymember", "rename", "sort", "sdiffstore", + "msetnx", "zrangestore", "sunionstore", "geosearchstore", "zdiffstore", "bitop", "renamenx"); } static bool isReadCommand(const std::string& command) { diff --git a/source/extensions/filters/network/redis_proxy/config.cc b/source/extensions/filters/network/redis_proxy/config.cc index c5481540f4954..ce39330a7c22c 100644 --- a/source/extensions/filters/network/redis_proxy/config.cc +++ b/source/extensions/filters/network/redis_proxy/config.cc @@ -102,11 +102,14 @@ Network::FilterFactoryCb RedisProxyFilterConfigFactory::createFilterFactoryFromP Stats::ScopeSharedPtr stats_scope = context.scope().createScope(fmt::format("cluster.{}.redis_cluster", cluster)); + // Get zone from LocalInfo for fallback when client_zone not explicitly configured + const std::string& local_zone = server_context.localInfo().zoneName(); auto conn_pool_ptr = std::make_shared( cluster, server_context.clusterManager(), Common::Redis::Client::ClientFactoryImpl::instance_, server_context.threadLocal(), proto_config.settings(), server_context.api(), std::move(stats_scope), redis_command_stats, - refresh_manager, filter_config->dns_cache_, aws_iam_config, aws_iam_authenticator); + refresh_manager, filter_config->dns_cache_, aws_iam_config, aws_iam_authenticator, + local_zone); conn_pool_ptr->init(); upstreams.emplace(cluster, conn_pool_ptr); } diff --git a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc index 76eabcc5cf103..6e3f7cff6fcf3 100644 --- a/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc +++ b/source/extensions/filters/network/redis_proxy/conn_pool_impl.cc @@ -53,13 +53,15 @@ InstanceImpl::InstanceImpl( const Extensions::Common::DynamicForwardProxy::DnsCacheSharedPtr& dns_cache, absl::optional aws_iam_config, absl::optional - aws_iam_authenticator) + aws_iam_authenticator, + const std::string& local_zone) : cluster_name_(cluster_name), cm_(cm), client_factory_(client_factory), tls_(tls.allocateSlot()), config_(new Common::Redis::Client::ConfigImpl(config)), api_(api), stats_scope_(std::move(stats_scope)), redis_command_stats_(redis_command_stats), redis_cluster_stats_{REDIS_CLUSTER_STATS(POOL_COUNTER(*stats_scope_))}, refresh_manager_(std::move(refresh_manager)), dns_cache_(dns_cache), - aws_iam_authenticator_(aws_iam_authenticator), aws_iam_config_(aws_iam_config) {} + aws_iam_authenticator_(aws_iam_authenticator), aws_iam_config_(aws_iam_config), + local_zone_(local_zone) {} void InstanceImpl::init() { // Note: `this` and `cluster_name` have a a lifetime of the filter. @@ -120,7 +122,7 @@ InstanceImpl::ThreadLocalPool::ThreadLocalPool( stats_scope_(parent->stats_scope_), redis_command_stats_(parent->redis_command_stats_), redis_cluster_stats_(parent->redis_cluster_stats_), refresh_manager_(parent->refresh_manager_), aws_iam_authenticator_(aws_iam_authenticator), - aws_iam_config_(aws_iam_config) { + aws_iam_config_(aws_iam_config), client_zone_(parent->localZone()) { cluster_update_handle_ = parent->cm_.addThreadLocalClusterUpdateCallbacks(*this); Upstream::ThreadLocalCluster* cluster = parent->cm_.getThreadLocalCluster(cluster_name_); @@ -269,6 +271,52 @@ void InstanceImpl::ThreadLocalPool::drainClients() { } } +RedisShardStatsSharedPtr +InstanceImpl::ThreadLocalPool::getOrCreateShardStats(const std::string& host_address) { + auto it = shard_stats_map_.find(host_address); + if (it != shard_stats_map_.end()) { + return it->second.stats_; + } + + // Create a sanitized stat name from the host address (replace ':' and '.' with '_') + std::string stat_name = host_address; + std::replace(stat_name.begin(), stat_name.end(), ':', '_'); + std::replace(stat_name.begin(), stat_name.end(), '.', '_'); + + Stats::ScopeSharedPtr shard_scope = stats_scope_->createScope(fmt::format("shard.{}.", stat_name)); + auto shard_stats = std::make_shared(RedisShardStats{ + REDIS_SHARD_STATS(POOL_COUNTER(*shard_scope), POOL_GAUGE(*shard_scope))}); + // Store both scope and stats to keep the scope alive + shard_stats_map_[host_address] = ShardStatsEntry{shard_scope, shard_stats}; + return shard_stats; +} + +Stats::ScopeSharedPtr +InstanceImpl::ThreadLocalPool::getShardScope(const std::string& host_address) { + auto it = shard_stats_map_.find(host_address); + if (it != shard_stats_map_.end()) { + return it->second.scope_; + } + return nullptr; +} + +Stats::Histogram* +InstanceImpl::ThreadLocalPool::getOrCreateShardLatencyHistogram(const std::string& host_address) { + auto it = shard_stats_map_.find(host_address); + if (it == shard_stats_map_.end()) { + // Create shard stats entry first if it doesn't exist + getOrCreateShardStats(host_address); + it = shard_stats_map_.find(host_address); + } + + if (it->second.latency_histogram_ == nullptr) { + // Create the histogram if it doesn't exist + it->second.latency_histogram_ = &it->second.scope_->histogramFromString( + "upstream_rq_time", Stats::Histogram::Unit::Microseconds); + } + return it->second.latency_histogram_; +} + InstanceImpl::ThreadLocalActiveClientPtr& InstanceImpl::ThreadLocalPool::threadLocalActiveClient(Upstream::HostConstSharedPtr host) { TokenBucketPtr& rate_limiter = cx_rate_limiter_map_[host]; @@ -306,7 +354,7 @@ uint16_t InstanceImpl::ThreadLocalPool::shardSize() { unique_hosts.reserve(Envoy::Extensions::Clusters::Redis::MaxSlot); for (uint16_t size = 0; size < Envoy::Extensions::Clusters::Redis::MaxSlot; size++) { Clusters::Redis::RedisSpecifyShardContextImpl lb_context( - size, request, Common::Redis::Client::ReadPolicy::Primary); + size, request, Common::Redis::Client::ReadPolicy::Primary, client_zone_); Upstream::HostConstSharedPtr host = Upstream::LoadBalancer::onlyAllowSynchronousHostSelection( cluster_->loadBalancer().chooseHost(&lb_context)); if (!host) { @@ -329,8 +377,8 @@ InstanceImpl::ThreadLocalPool::makeRequest(const std::string& key, RespVariant&& Clusters::Redis::RedisLoadBalancerContextImpl lb_context( key, config_->enableHashtagging(), is_redis_cluster_, getRequest(request), - transaction.active_ ? Common::Redis::Client::ReadPolicy::Primary : config_->readPolicy()); - + transaction.active_ ? Common::Redis::Client::ReadPolicy::Primary : config_->readPolicy(), + client_zone_); Upstream::HostConstSharedPtr host = Upstream::LoadBalancer::onlyAllowSynchronousHostSelection( cluster_->loadBalancer().chooseHost(&lb_context)); if (!host) { @@ -353,7 +401,8 @@ InstanceImpl::ThreadLocalPool::makeRequestToShard(uint16_t shard_index, RespVari Clusters::Redis::RedisSpecifyShardContextImpl lb_context( shard_index, getRequest(request), - transaction.active_ ? Common::Redis::Client::ReadPolicy::Primary : config_->readPolicy()); + transaction.active_ ? Common::Redis::Client::ReadPolicy::Primary : config_->readPolicy(), + client_zone_); Upstream::HostConstSharedPtr host = Upstream::LoadBalancer::onlyAllowSynchronousHostSelection( cluster_->loadBalancer().chooseHost(&lb_context)); @@ -455,7 +504,22 @@ InstanceImpl::ThreadLocalPool::makeRequestToHost(Upstream::HostConstSharedPtr& h } } - pending_requests_.emplace_back(*this, std::move(request), callbacks, host); + // Get or create per-shard stats for tracking hot shard usage (if enabled) + RedisShardStatsSharedPtr shard_stats = nullptr; + Stats::ScopeSharedPtr shard_scope = nullptr; + Stats::Histogram* latency_histogram = nullptr; + if (config_->enablePerShardStats()) { + const std::string host_address = host->address()->asString(); + shard_stats = getOrCreateShardStats(host_address); + shard_scope = getShardScope(host_address); + } + if (config_->enablePerShardLatencyStats()) { + const std::string host_address = host->address()->asString(); + latency_histogram = getOrCreateShardLatencyHistogram(host_address); + } + + pending_requests_.emplace_back(*this, std::move(request), callbacks, host, shard_stats, + shard_scope, latency_histogram); PendingRequest& pending_request = pending_requests_.back(); if (!transaction.active_) { @@ -518,9 +582,30 @@ void InstanceImpl::ThreadLocalActiveClient::onEvent(Network::ConnectionEvent eve InstanceImpl::PendingRequest::PendingRequest(InstanceImpl::ThreadLocalPool& parent, RespVariant&& incoming_request, PoolCallbacks& pool_callbacks, - Upstream::HostConstSharedPtr& host) + Upstream::HostConstSharedPtr& host, + RedisShardStatsSharedPtr shard_stats, + Stats::ScopeSharedPtr shard_scope, + Stats::Histogram* latency_histogram) : parent_(parent), incoming_request_(std::move(incoming_request)), - pool_callbacks_(pool_callbacks), host_(host) {} + pool_callbacks_(pool_callbacks), host_(host), shard_stats_(std::move(shard_stats)), + shard_scope_(std::move(shard_scope)), latency_histogram_(latency_histogram), + start_time_(parent.dispatcher_.timeSource().monotonicTime()) { + // Track per-shard request metrics and command stats + if (shard_stats_) { + shard_stats_->upstream_rq_total_.inc(); + shard_stats_->upstream_rq_active_.inc(); + } + // Extract and track command name for per-shard command stats + if (shard_scope_ && parent_.config_->enableCommandStats()) { + command_ = parent_.redis_command_stats_->getCommandFromRequest(getRequest(incoming_request_)); + parent_.redis_command_stats_->updateStatsTotal(*shard_scope_, command_); + // Create per-shard per-command latency timer when both command stats and per-shard latency are enabled + if (parent_.config_->enablePerShardLatencyStats()) { + command_latency_timer_ = parent_.redis_command_stats_->createCommandTimer( + *shard_scope_, command_, parent_.dispatcher_.timeSource()); + } + } +} InstanceImpl::PendingRequest::~PendingRequest() { cache_load_handle_.reset(); @@ -528,6 +613,15 @@ InstanceImpl::PendingRequest::~PendingRequest() { if (request_handler_) { request_handler_->cancel(); request_handler_ = nullptr; + // Update per-shard stats - treat cancellation as failure + if (shard_stats_) { + shard_stats_->upstream_rq_active_.dec(); + shard_stats_->upstream_rq_failure_.inc(); + } + // Update per-shard command stats (failure due to cancellation) + if (shard_scope_ && parent_.config_->enableCommandStats()) { + parent_.redis_command_stats_->updateStats(*shard_scope_, command_, false); + } // If we have to cancel the request on the client, then we'll treat this as failure for pool // callback pool_callbacks_.onFailure(); @@ -536,12 +630,52 @@ InstanceImpl::PendingRequest::~PendingRequest() { void InstanceImpl::PendingRequest::onResponse(Common::Redis::RespValuePtr&& response) { request_handler_ = nullptr; + // Update per-shard stats + if (shard_stats_) { + shard_stats_->upstream_rq_active_.dec(); + shard_stats_->upstream_rq_success_.inc(); + } + // Update per-shard command stats (success) + if (shard_scope_ && parent_.config_->enableCommandStats()) { + parent_.redis_command_stats_->updateStats(*shard_scope_, command_, true); + } + // Record per-shard latency histogram + if (latency_histogram_ != nullptr) { + const auto end_time = parent_.dispatcher_.timeSource().monotonicTime(); + const auto latency_us = std::chrono::duration_cast( + end_time - start_time_).count(); + latency_histogram_->recordValue(latency_us); + } + // Complete per-shard per-command latency timer + if (command_latency_timer_) { + command_latency_timer_->complete(); + } pool_callbacks_.onResponse(std::move(response)); parent_.onRequestCompleted(); } void InstanceImpl::PendingRequest::onFailure() { request_handler_ = nullptr; + // Update per-shard stats + if (shard_stats_) { + shard_stats_->upstream_rq_active_.dec(); + shard_stats_->upstream_rq_failure_.inc(); + } + // Update per-shard command stats (failure) + if (shard_scope_ && parent_.config_->enableCommandStats()) { + parent_.redis_command_stats_->updateStats(*shard_scope_, command_, false); + } + // Record per-shard latency histogram (even for failures) + if (latency_histogram_ != nullptr) { + const auto end_time = parent_.dispatcher_.timeSource().monotonicTime(); + const auto latency_us = std::chrono::duration_cast( + end_time - start_time_).count(); + latency_histogram_->recordValue(latency_us); + } + // Complete per-shard per-command latency timer + if (command_latency_timer_) { + command_latency_timer_->complete(); + } pool_callbacks_.onFailure(); parent_.refresh_manager_->onFailure(parent_.cluster_name_); parent_.onRequestCompleted(); @@ -640,6 +774,26 @@ void InstanceImpl::PendingRequest::doRedirection(Common::Redis::RespValuePtr&& v void InstanceImpl::PendingRequest::cancel() { request_handler_->cancel(); request_handler_ = nullptr; + // Update per-shard stats - treat cancellation as failure + if (shard_stats_) { + shard_stats_->upstream_rq_active_.dec(); + shard_stats_->upstream_rq_failure_.inc(); + } + // Update per-shard command stats (failure due to cancellation) + if (shard_scope_ && parent_.config_->enableCommandStats()) { + parent_.redis_command_stats_->updateStats(*shard_scope_, command_, false); + } + // Record per-shard latency histogram (even for cancellations) + if (latency_histogram_ != nullptr) { + const auto end_time = parent_.dispatcher_.timeSource().monotonicTime(); + const auto latency_us = std::chrono::duration_cast( + end_time - start_time_).count(); + latency_histogram_->recordValue(latency_us); + } + // Complete per-shard per-command latency timer + if (command_latency_timer_) { + command_latency_timer_->complete(); + } parent_.onRequestCompleted(); } diff --git a/source/extensions/filters/network/redis_proxy/conn_pool_impl.h b/source/extensions/filters/network/redis_proxy/conn_pool_impl.h index b8f520777ae34..7ada14fbe97af 100644 --- a/source/extensions/filters/network/redis_proxy/conn_pool_impl.h +++ b/source/extensions/filters/network/redis_proxy/conn_pool_impl.h @@ -7,8 +7,10 @@ #include #include +#include "envoy/common/time.h" #include "envoy/extensions/filters/network/redis_proxy/v3/redis_proxy.pb.h" #include "envoy/stats/stats_macros.h" +#include "envoy/stats/timespan.h" #include "envoy/thread_local/thread_local.h" #include "envoy/upstream/cluster_manager.h" @@ -49,6 +51,32 @@ struct RedisClusterStats { REDIS_CLUSTER_STATS(GENERATE_COUNTER_STRUCT) }; +/** + * Per-shard statistics for tracking hot shard usage. + * These metrics help identify which shards are receiving more traffic. + */ +#define REDIS_SHARD_STATS(COUNTER, GAUGE) \ + COUNTER(upstream_rq_total) \ + COUNTER(upstream_rq_success) \ + COUNTER(upstream_rq_failure) \ + GAUGE(upstream_rq_active, Accumulate) + +struct RedisShardStats { + REDIS_SHARD_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) +}; + +using RedisShardStatsSharedPtr = std::shared_ptr; + +/** + * Struct to hold per-shard stats and the scope they belong to. + * The scope must be kept alive for the stats to remain valid. + */ +struct ShardStatsEntry { + Stats::ScopeSharedPtr scope_; + RedisShardStatsSharedPtr stats_; + Stats::Histogram* latency_histogram_{nullptr}; // Per-shard latency histogram (optional) +}; + class DoNothingPoolCallbacks : public PoolCallbacks { public: void onResponse(Common::Redis::RespValuePtr&&) override {}; @@ -68,7 +96,8 @@ class InstanceImpl : public Instance, public std::enable_shared_from_this aws_iam_config, absl::optional - aws_iam_authenticator); + aws_iam_authenticator, + const std::string& local_zone = ""); uint16_t shardSize() override; // RedisProxy::ConnPool::Instance Common::Redis::Client::PoolRequest* @@ -119,7 +148,9 @@ class InstanceImpl : public Instance, public std::enable_shared_from_this { PendingRequest(ThreadLocalPool& parent, RespVariant&& incoming_request, - PoolCallbacks& pool_callbacks, Upstream::HostConstSharedPtr& host); + PoolCallbacks& pool_callbacks, Upstream::HostConstSharedPtr& host, + RedisShardStatsSharedPtr shard_stats, Stats::ScopeSharedPtr shard_scope, + Stats::Histogram* latency_histogram); ~PendingRequest() override; // Common::Redis::Client::ClientCallbacks @@ -148,6 +179,12 @@ class InstanceImpl : public Instance, public std::enable_shared_from_this& hosts_added); void onHostsRemoved(const std::vector& hosts_removed); void drainClients(); + RedisShardStatsSharedPtr getOrCreateShardStats(const std::string& host_address); + Stats::ScopeSharedPtr getShardScope(const std::string& host_address); + Stats::Histogram* getOrCreateShardLatencyHistogram(const std::string& host_address); // Upstream::ClusterUpdateCallbacks void onClusterAddOrUpdate(absl::string_view cluster_name, @@ -222,8 +262,13 @@ class InstanceImpl : public Instance, public std::enable_shared_from_this aws_iam_authenticator_; absl::optional aws_iam_config_; + // Per-shard stats map keyed by host address (e.g., "10.0.0.1:6379") + absl::node_hash_map shard_stats_map_; + std::string client_zone_; // Zone from node.locality.zone }; + const std::string& localZone() const { return local_zone_; } + const std::string cluster_name_; Upstream::ClusterManager& cm_; Common::Redis::Client::ClientFactory& client_factory_; @@ -238,6 +283,7 @@ class InstanceImpl : public Instance, public std::enable_shared_from_this aws_iam_authenticator_; absl::optional aws_iam_config_; + const std::string local_zone_; // Zone from node.locality.zone }; } // namespace ConnPool diff --git a/source/extensions/health_checkers/redis/redis.h b/source/extensions/health_checkers/redis/redis.h index 8668904b26447..3be9d26cff47f 100644 --- a/source/extensions/health_checkers/redis/redis.h +++ b/source/extensions/health_checkers/redis/redis.h @@ -95,6 +95,8 @@ class RedisHealthChecker : public Upstream::HealthCheckerImplBase { uint32_t maxUpstreamUnknownConnections() const override { return 0; } bool enableCommandStats() const override { return false; } + bool enablePerShardStats() const override { return false; } // Not needed for health checks + bool enablePerShardLatencyStats() const override { return false; } // Not needed for health checks bool connectionRateLimitEnabled() const override { return false; } uint32_t connectionRateLimitPerSec() const override { return 0; } diff --git a/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.cc b/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.cc index b82d9b82a4d1b..ec508368d8afa 100644 --- a/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.cc +++ b/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.cc @@ -5,9 +5,9 @@ namespace Quic { std::unique_ptr EnvoyQuicProofSourceFactoryImpl::createQuicProofSource( Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) { + Server::ListenerStats& listener_stats, TimeSource& time_source, Stats::Scope& stats_scope) { return std::make_unique(listen_socket, filter_chain_manager, listener_stats, - time_source); + time_source, stats_scope); } REGISTER_FACTORY(EnvoyQuicProofSourceFactoryImpl, EnvoyQuicProofSourceFactoryInterface); diff --git a/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.h b/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.h index 4e3b2a8663092..85438522f9d96 100644 --- a/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.h +++ b/source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.h @@ -21,7 +21,8 @@ class EnvoyQuicProofSourceFactoryImpl : public EnvoyQuicProofSourceFactoryInterf std::unique_ptr createQuicProofSource(Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) override; + Server::ListenerStats& listener_stats, TimeSource& time_source, + Stats::Scope& stats_scope) override; }; DECLARE_FACTORY(EnvoyQuicProofSourceFactoryImpl); diff --git a/source/extensions/request_id/uuid/BUILD b/source/extensions/request_id/uuid/BUILD index 7e2fd0ffb3b8c..9243aac3b277e 100644 --- a/source/extensions/request_id/uuid/BUILD +++ b/source/extensions/request_id/uuid/BUILD @@ -20,6 +20,7 @@ envoy_cc_extension( deps = [ "//envoy/http:request_id_extension_interface", "//envoy/server:request_id_extension_config_interface", + "//source/common/common:hex_lib", "//source/common/common:random_generator_lib", "//source/common/stream_info:stream_id_provider_lib", "@envoy_api//envoy/extensions/request_id/uuid/v3:pkg_cc_proto", diff --git a/source/extensions/request_id/uuid/config.cc b/source/extensions/request_id/uuid/config.cc index be75b13b763e4..3c274befb44be 100644 --- a/source/extensions/request_id/uuid/config.cc +++ b/source/extensions/request_id/uuid/config.cc @@ -1,12 +1,17 @@ #include "source/extensions/request_id/uuid/config.h" +#include + #include "envoy/http/header_map.h" #include "envoy/tracing/tracer.h" +#include "source/common/common/hex.h" #include "source/common/common/random_generator.h" #include "source/common/common/utility.h" #include "source/common/stream_info/stream_id_provider_impl.h" +#include "absl/strings/str_cat.h" + namespace Envoy { namespace Extensions { namespace RequestId { @@ -17,7 +22,7 @@ void UUIDRequestIDExtension::set(Http::RequestHeaderMap& request_headers, bool e // No request ID then set new one anyway. if (request_id_header == nullptr || request_id_header->value().empty()) { - request_headers.setRequestId(random_.uuid()); + request_headers.setRequestId(generateUuidV7()); return; } @@ -32,7 +37,7 @@ void UUIDRequestIDExtension::set(Http::RequestHeaderMap& request_headers, bool e if (!keep_external_id) { // If we are not keeping external request ID, then set new one anyway. - request_headers.setRequestId(random_.uuid()); + request_headers.setRequestId(generateUuidV7()); return; } @@ -64,12 +69,15 @@ UUIDRequestIDExtension::getInteger(const Http::RequestHeaderMap& request_headers return absl::nullopt; } const std::string uuid(request_headers.getRequestIdValue()); - if (uuid.length() < 8) { + // UUID format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx (36 chars) + // For UUIDv7: first 8 chars contain timestamp, use last 8 hex chars (rand_b) for randomness. + // Position 28-35 contains the last 8 hex digits. + if (uuid.length() < 36) { return absl::nullopt; } uint64_t value; - if (!StringUtil::atoull(uuid.substr(0, 8).c_str(), value, 16)) { + if (!StringUtil::atoull(uuid.substr(28, 8).c_str(), value, 16)) { return absl::nullopt; } @@ -95,6 +103,8 @@ UUIDRequestIDExtension::getTraceReason(const Http::RequestHeaderMap& request_hea return Tracing::Reason::Sampling; case TRACE_CLIENT: return Tracing::Reason::ClientForced; + case NO_TRACE_V4: // UUIDv4 freshly generated (version bit '4') + case NO_TRACE_V7: // UUIDv7 freshly generated (version bit '7') default: return Tracing::Reason::NotTraceable; } @@ -122,7 +132,15 @@ void UUIDRequestIDExtension::setTraceReason(Http::RequestHeaderMap& request_head uuid[TRACE_BYTE_POSITION] = TRACE_SAMPLED; break; case Tracing::Reason::NotTraceable: - uuid[TRACE_BYTE_POSITION] = NO_TRACE; + // Preserve the original version bit ('4' for UUIDv4, '7' for UUIDv7). + // Only reset if currently set to a trace reason marker. + if (uuid[TRACE_BYTE_POSITION] == TRACE_SAMPLED || + uuid[TRACE_BYTE_POSITION] == TRACE_FORCED || + uuid[TRACE_BYTE_POSITION] == TRACE_CLIENT) { + // Default to UUIDv7 version bit when clearing trace reason. + uuid[TRACE_BYTE_POSITION] = NO_TRACE_V7; + } + // If already NO_TRACE_V4 or NO_TRACE_V7, leave unchanged. break; default: break; @@ -130,6 +148,57 @@ void UUIDRequestIDExtension::setTraceReason(Http::RequestHeaderMap& request_head request_headers.setRequestId(uuid); } +std::string UUIDRequestIDExtension::generateUuidV7() { + // Modified UUIDv7: 상위 4비트를 0xF 마커로 설정하여 trace_id와 구분 + // request_id: fxxxxxxx-xxxx-7xxx-yxxx-xxxxxxxxxxxx ('f'로 시작) + // trace_id: 0xxxxxxx-xxxx-7xxx-yxxx-xxxxxxxxxxxx ('0'으로 시작) + // + // Bit layout: + // [0-3] 4-bit marker (1111 = 0xF for request_id) + // [4-47] 44-bit Unix timestamp in milliseconds (~557 years until ~2527 AD) + // [48-51] 4-bit version (0111 = 7) + // [52-63] 12-bit rand_a + // [64-65] 2-bit variant (10 = RFC 4122) + // [66-127] 62-bit rand_b + // + // Result format: fxxxxxxx-xxxx-7xxx-yxxx-xxxxxxxxxxxx + // where y is one of [8, 9, a, b] (variant bits). + + const uint64_t timestamp_ms = static_cast( + std::chrono::duration_cast( + time_source_.systemTime().time_since_epoch()) + .count()); + + // 실제 timestamp는 44비트만 사용 (상위 4비트는 마커용) + // 44비트로 약 557년 표현 가능 (서기 ~2527년까지) + const uint64_t timestamp_44bit = timestamp_ms & 0x0FFFFFFFFFFFULL; + + // 상위 4비트에 0xF 마커 설정 → request_id가 'f'로 시작하게 됨 + constexpr uint64_t kRequestIdMarker = 0xFULL; + const uint64_t marked_timestamp = (kRequestIdMarker << 44) | timestamp_44bit; + + // rand_a: 12 bits of randomness for sub-millisecond ordering + const uint64_t rand_a = random_.random() & 0x0FFFULL; + // rand_b: 62 bits of randomness (2 bits reserved for variant) + const uint64_t rand_b = random_.random() & 0x3FFFFFFFFFFFFFFFULL; + + constexpr uint64_t kVersion7 = 0x7ULL; + constexpr uint64_t kVariantRfc4122 = 0x2ULL; + + // Build high 64 bits: marked_timestamp (48) | version (4) | rand_a (12) + const uint64_t uuid_high = (marked_timestamp << 16) | (kVersion7 << 12) | rand_a; + // Build low 64 bits: variant (2) | rand_b (62) + const uint64_t uuid_low = (kVariantRfc4122 << 62) | rand_b; + + // Convert to hex strings (16 chars each, zero-padded) + std::string high_hex = Hex::uint64ToHex(uuid_high); + std::string low_hex = Hex::uint64ToHex(uuid_low); + + // Format: fxxxxxxx-xxxx-7xxx-yxxx-xxxxxxxxxxxx + return absl::StrCat(high_hex.substr(0, 8), "-", high_hex.substr(8, 4), "-", + high_hex.substr(12, 4), "-", low_hex.substr(0, 4), "-", low_hex.substr(4, 12)); +} + REGISTER_FACTORY(UUIDRequestIDExtensionFactory, Server::Configuration::RequestIDExtensionFactory); } // namespace RequestId diff --git a/source/extensions/request_id/uuid/config.h b/source/extensions/request_id/uuid/config.h index 92c1ea86dc2a5..c2f2ed7812acf 100644 --- a/source/extensions/request_id/uuid/config.h +++ b/source/extensions/request_id/uuid/config.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/common/time.h" #include "envoy/extensions/request_id/uuid/v3/uuid.pb.h" #include "envoy/extensions/request_id/uuid/v3/uuid.pb.validate.h" #include "envoy/http/request_id_extension.h" @@ -14,15 +15,16 @@ namespace RequestId { class UUIDRequestIDExtension : public Envoy::Http::RequestIDExtension { public: UUIDRequestIDExtension(const envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig& config, - Random::RandomGenerator& random) - : random_(random), + Random::RandomGenerator& random, TimeSource& time_source) + : random_(random), time_source_(time_source), pack_trace_reason_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, pack_trace_reason, true)), use_request_id_for_trace_sampling_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_request_id_for_trace_sampling, true)) {} - static Envoy::Http::RequestIDExtensionSharedPtr defaultInstance(Random::RandomGenerator& random) { + static Envoy::Http::RequestIDExtensionSharedPtr + defaultInstance(Random::RandomGenerator& random, TimeSource& time_source) { return std::make_shared( - envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), random); + envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), random, time_source); } bool packTraceReason() { return pack_trace_reason_; } @@ -42,7 +44,22 @@ class UUIDRequestIDExtension : public Envoy::Http::RequestIDExtension { bool useRequestIdForTraceSampling() const override { return use_request_id_for_trace_sampling_; } private: + /** + * Generates a 128-bit request ID following UUIDv7 format (RFC 9562). + * + * UUIDv7 layout (128 bits): + * [0-47] 48-bit Unix timestamp (milliseconds) + * [48-51] 4-bit version (0111 = 7) + * [52-63] 12-bit rand_a + * [64-65] 2-bit variant (10) + * [66-127] 62-bit rand_b + * + * @return 36-character UUID string with hyphens. + */ + std::string generateUuidV7(); + Envoy::Random::RandomGenerator& random_; + Envoy::TimeSource& time_source_; const bool pack_trace_reason_; const bool use_request_id_for_trace_sampling_; @@ -61,8 +78,10 @@ class UUIDRequestIDExtension : public Envoy::Http::RequestIDExtension { // one modified because of client trace. static const char TRACE_CLIENT = 'b'; - // Initial value for freshly generated uuid4. - static const char NO_TRACE = '4'; + // Initial value for freshly generated UUIDv4. + static const char NO_TRACE_V4 = '4'; + // Initial value for freshly generated UUIDv7. + static const char NO_TRACE_V7 = '7'; }; // Factory for the UUID request ID extension. @@ -79,7 +98,8 @@ class UUIDRequestIDExtensionFactory : public Server::Configuration::RequestIDExt MessageUtil::downcastAndValidate< const envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig&>( config, context.messageValidationVisitor()), - context.serverFactoryContext().api().randomGenerator()); + context.serverFactoryContext().api().randomGenerator(), + context.serverFactoryContext().api().timeSource()); } }; diff --git a/source/extensions/tracers/opentelemetry/span_context_extractor.h b/source/extensions/tracers/opentelemetry/span_context_extractor.h index dffeb6218c921..517a9700bafaf 100644 --- a/source/extensions/tracers/opentelemetry/span_context_extractor.h +++ b/source/extensions/tracers/opentelemetry/span_context_extractor.h @@ -15,8 +15,8 @@ namespace OpenTelemetry { class OpenTelemetryConstantValues { public: - const Tracing::TraceContextHandler TRACE_PARENT{"traceparent"}; - const Tracing::TraceContextHandler TRACE_STATE{"tracestate"}; + const Tracing::TraceContextHandler TRACE_PARENT{"x-sendbird-traceparent"}; + const Tracing::TraceContextHandler TRACE_STATE{"x-sendbird-tracestate"}; }; using OpenTelemetryConstants = ConstSingleton; diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index 9017f360e2fbe..f796cb277208c 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -5,6 +5,7 @@ #include "envoy/config/trace/v3/opentelemetry.pb.h" +#include "source/common/common/assert.h" #include "source/common/common/empty_string.h" #include "source/common/common/hex.h" #include "source/common/tracing/common_values.h" @@ -27,11 +28,11 @@ using opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest; namespace { const Tracing::TraceContextHandler& traceParentHeader() { - CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "traceparent"); + CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "x-sendbird-traceparent"); } const Tracing::TraceContextHandler& traceStateHeader() { - CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "tracestate"); + CONSTRUCT_ON_FIRST_USE(Tracing::TraceContextHandler, "x-sendbird-tracestate"); } void callSampler(SamplerSharedPtr sampler, const StreamInfo::StreamInfo& stream_info, @@ -266,6 +267,34 @@ void Tracer::sendSpan(::opentelemetry::proto::trace::v1::Span& span) { } } +std::string Tracer::generateTraceIdV7() { + // UUIDv7 bit layout (RFC 9562): + // High 64 bits: [timestamp_ms (48)] [version (4)] [rand_a (12)] + // Low 64 bits: [variant (2)] [rand_b (62)] + + // Get current Unix timestamp in milliseconds. + const uint64_t timestamp_ms = static_cast( + std::chrono::duration_cast( + time_source_.systemTime().time_since_epoch()) + .count()); + // Timestamp must fit in 48 bits (valid until year ~10889). + ASSERT((timestamp_ms >> 48) == 0); + + // Generate random bits. + const uint64_t rand_a = random_.random() & 0x0FFFULL; // 12 bits + const uint64_t rand_b = random_.random() & 0x3FFFFFFFFFFFFFFFULL; // 62 bits + + // Assemble high 64 bits: [timestamp_ms(48)][version=7(4)][rand_a(12)] + constexpr uint64_t kVersion7 = 0x7ULL; + const uint64_t trace_id_high = (timestamp_ms << 16) | (kVersion7 << 12) | rand_a; + + // Assemble low 64 bits: [variant=2(2)][rand_b(62)] + constexpr uint64_t kVariantRfc4122 = 0x2ULL; + const uint64_t trace_id_low = (kVariantRfc4122 << 62) | rand_b; + + return absl::StrCat(Hex::uint64ToHex(trace_id_high), Hex::uint64ToHex(trace_id_low)); +} + Tracing::SpanPtr Tracer::startSpan(const std::string& operation_name, const StreamInfo::StreamInfo& stream_info, SystemTime start_time, Tracing::Decision tracing_decision, @@ -279,9 +308,7 @@ Tracing::SpanPtr Tracer::startSpan(const std::string& operation_name, // Create an Tracers::OpenTelemetry::Span class that will contain the OTel span. auto new_span = std::make_unique(operation_name, stream_info, start_time, time_source_, *this, span_kind, use_local_decision); - uint64_t trace_id_high = random_.random(); - uint64_t trace_id = random_.random(); - new_span->setTraceId(absl::StrCat(Hex::uint64ToHex(trace_id_high), Hex::uint64ToHex(trace_id))); + new_span->setTraceId(generateTraceIdV7()); uint64_t span_id = random_.random(); new_span->setId(Hex::uint64ToHex(span_id)); if (sampler_) { diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index 1f6e10ccf63ba..17cf51d7cfc45 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -65,6 +65,19 @@ class Tracer : Logger::Loggable { * Removes all spans from the span buffer and sends them to the collector. */ void flushSpans(); + /** + * Generates a 128-bit trace ID following UUIDv7 format (RFC 9562). + * + * UUIDv7 layout (128 bits): + * [0-47] 48-bit Unix timestamp (milliseconds) + * [48-51] 4-bit version (0111 = 7) + * [52-63] 12-bit rand_a + * [64-65] 2-bit variant (10) + * [66-127] 62-bit rand_b + * + * @return 32-character hex string (128-bit trace ID). + */ + std::string generateTraceIdV7(); OpenTelemetryTraceExporterPtr exporter_; Envoy::TimeSource& time_source_; diff --git a/source/server/admin/admin.cc b/source/server/admin/admin.cc index d4b308c760d18..87d0a4f46f6b5 100644 --- a/source/server/admin/admin.cc +++ b/source/server/admin/admin.cc @@ -110,7 +110,7 @@ AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server, : server_(server), listener_info_(std::make_shared()), factory_context_(server, listener_info_), request_id_extension_(Extensions::RequestId::UUIDRequestIDExtension::defaultInstance( - server_.api().randomGenerator())), + server_.api().randomGenerator(), server_.api().timeSource())), profile_path_(profile_path), stats_(Http::ConnectionManagerImpl::generateStats( "http.admin.", *server_.stats().rootScope())), null_overload_manager_(server.threadLocal(), false), diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index b7d5f6247bd25..c86a53b4df60e 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -2900,6 +2900,50 @@ TEST(SubstitutionFormatterTest, TraceIDFormatter) { } } +TEST(SubstitutionFormatterTest, TraceSampledFormatter) { + StreamInfo::MockStreamInfo stream_info; + Http::TestRequestHeaderMapImpl request_header{}; + Http::TestResponseHeaderMapImpl response_header{}; + Http::TestResponseTrailerMapImpl response_trailer{}; + std::string body; + HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, + body); + TraceSampledFormatter formatter{}; + + // Test traced=true cases: Sampling, ClientForced, ServiceForced + { + EXPECT_CALL(stream_info, healthCheck()).WillRepeatedly(Return(false)); + EXPECT_CALL(stream_info, traceReason()).WillRepeatedly(Return(Tracing::Reason::Sampling)); + EXPECT_EQ("true", formatter.formatWithContext(formatter_context, stream_info)); + EXPECT_THAT(formatter.formatValueWithContext(formatter_context, stream_info), + ProtoEq(ValueUtil::stringValue("true"))); + } + { + EXPECT_CALL(stream_info, healthCheck()).WillOnce(Return(false)); + EXPECT_CALL(stream_info, traceReason()).WillOnce(Return(Tracing::Reason::ClientForced)); + EXPECT_EQ("true", formatter.formatWithContext(formatter_context, stream_info)); + } + { + EXPECT_CALL(stream_info, healthCheck()).WillOnce(Return(false)); + EXPECT_CALL(stream_info, traceReason()).WillOnce(Return(Tracing::Reason::ServiceForced)); + EXPECT_EQ("true", formatter.formatWithContext(formatter_context, stream_info)); + } + + // Test traced=false cases: NotTraceable, HealthCheck + { + EXPECT_CALL(stream_info, healthCheck()).WillRepeatedly(Return(false)); + EXPECT_CALL(stream_info, traceReason()).WillRepeatedly(Return(Tracing::Reason::NotTraceable)); + EXPECT_EQ("false", formatter.formatWithContext(formatter_context, stream_info)); + EXPECT_THAT(formatter.formatValueWithContext(formatter_context, stream_info), + ProtoEq(ValueUtil::stringValue("false"))); + } + { + // HealthCheck requests should return false regardless of traceReason + EXPECT_CALL(stream_info, healthCheck()).WillOnce(Return(true)); + EXPECT_EQ("false", formatter.formatWithContext(formatter_context, stream_info)); + } +} + /** * Populate a metadata object with the following test data: * "com.test": {"test_key":"test_value","test_obj":{"inner_key":"inner_value"}} diff --git a/test/common/quic/cert_compression_test.cc b/test/common/quic/cert_compression_test.cc index 767b13df1bf32..73ad5edc06c9e 100644 --- a/test/common/quic/cert_compression_test.cc +++ b/test/common/quic/cert_compression_test.cc @@ -1,4 +1,5 @@ #include "source/common/quic/cert_compression.h" +#include "source/common/tls/cert_compression.h" #include "test/test_common/logging.h" @@ -7,16 +8,17 @@ namespace Envoy { namespace Quic { +using TlsCertCompression = Extensions::TransportSockets::Tls::CertCompression; + TEST(CertCompressionZlibTest, DecompressBadData) { EXPECT_LOG_CONTAINS( "error", - "Cert decompression failure in inflate, possibly caused by invalid compressed cert from peer", - { + "Cert zlib decompression failure, possibly caused by invalid compressed cert from peer", { CRYPTO_BUFFER* out = nullptr; const uint8_t bad_compressed_data = 1; - EXPECT_EQ(CertCompression::FAILURE, - CertCompression::decompressZlib(nullptr, &out, 100, &bad_compressed_data, - sizeof(bad_compressed_data))); + EXPECT_EQ(TlsCertCompression::FAILURE, + TlsCertCompression::decompressZlib(nullptr, &out, 100, &bad_compressed_data, + sizeof(bad_compressed_data))); }); } @@ -25,18 +27,19 @@ TEST(CertCompressionZlibTest, DecompressBadLength) { constexpr size_t uncompressed_len = 6; bssl::ScopedCBB compressed; ASSERT_EQ(1, CBB_init(compressed.get(), 0)); - ASSERT_EQ(CertCompression::SUCCESS, - CertCompression::compressZlib(nullptr, compressed.get(), the_data, uncompressed_len)); + ASSERT_EQ( + TlsCertCompression::SUCCESS, + TlsCertCompression::compressZlib(nullptr, compressed.get(), the_data, uncompressed_len)); const auto compressed_len = CBB_len(compressed.get()); EXPECT_NE(0, compressed_len); EXPECT_LOG_CONTAINS("error", - "Decompression length did not match peer provided uncompressed length, " - "caused by either invalid peer handshake data or decompression error.", + "Zlib decompression length did not match peer provided uncompressed length, " + "caused by either invalid peer handshake data or decompression error", { CRYPTO_BUFFER* out = nullptr; - EXPECT_EQ(CertCompression::FAILURE, - CertCompression::decompressZlib( + EXPECT_EQ(TlsCertCompression::FAILURE, + TlsCertCompression::decompressZlib( nullptr, &out, uncompressed_len + 1 /* intentionally incorrect */, CBB_data(compressed.get()), compressed_len)); diff --git a/test/common/quic/envoy_quic_proof_source_test.cc b/test/common/quic/envoy_quic_proof_source_test.cc index a51dfa36722b8..51c8ca875b274 100644 --- a/test/common/quic/envoy_quic_proof_source_test.cc +++ b/test/common/quic/envoy_quic_proof_source_test.cc @@ -1,3 +1,7 @@ +#include + +#include +#include #include #include #include @@ -13,6 +17,7 @@ #include "test/mocks/network/mocks.h" #include "test/mocks/server/server_factory_context.h" #include "test/mocks/ssl/mocks.h" +#include "test/test_common/network_utility.h" #include "test/test_common/test_runtime.h" #include "gmock/gmock.h" @@ -156,7 +161,8 @@ class EnvoyQuicProofSourceTest : public ::testing::Test { listener_stats_({ALL_LISTENER_STATS(POOL_COUNTER(listener_config_.listenerScope()), POOL_GAUGE(listener_config_.listenerScope()), POOL_HISTOGRAM(listener_config_.listenerScope()))}), - proof_source_(listen_socket_, filter_chain_manager_, listener_stats_, time_system_) { + proof_source_(listen_socket_, filter_chain_manager_, listener_stats_, time_system_, + listener_config_.listenerScope()) { EXPECT_CALL(*mock_context_config_, setSecretUpdateCallback(_)) .Times(testing::AtLeast(1u)) .WillRepeatedly(SaveArg<0>(&secret_update_callback_)); @@ -347,5 +353,176 @@ TEST_F(EnvoyQuicProofSourceTest, ComputeSignatureFailNoFilterChain) { std::make_unique(false, filter_chain_, signature)); } +// Test keylog functionality +TEST_F(EnvoyQuicProofSourceTest, TestKeylogFunctionality) { + // Test that OnNewSslCtx sets up keylog callback correctly + bssl::UniquePtr ssl_ctx(SSL_CTX_new(TLS_method())); + ASSERT_NE(ssl_ctx, nullptr); + + // Call OnNewSslCtx which should set up the keylog callback + proof_source_.OnNewSslCtx(ssl_ctx.get()); + + // Verify that the proof source was stored in SSL_CTX app data + void* app_data = SSL_CTX_get_app_data(ssl_ctx.get()); + EXPECT_EQ(app_data, static_cast(&proof_source_)); + + // Verify that keylog callback was set + void (*callback)(const SSL*, const char*) = SSL_CTX_get_keylog_callback(ssl_ctx.get()); + EXPECT_NE(callback, nullptr); +} + +// Test keylog callback registration +TEST_F(EnvoyQuicProofSourceTest, TestKeylogCallbackRegistration) { + // Create SSL_CTX and setup keylog + bssl::UniquePtr ssl_ctx(SSL_CTX_new(TLS_method())); + proof_source_.OnNewSslCtx(ssl_ctx.get()); + + // Verify that keylog callback is registered + void (*callback)(const SSL*, const char*) = SSL_CTX_get_keylog_callback(ssl_ctx.get()); + EXPECT_NE(callback, nullptr); + + // Verify that app data points to our proof source + void* app_data = SSL_CTX_get_app_data(ssl_ctx.get()); + EXPECT_EQ(app_data, static_cast(&proof_source_)); +} + +// Test keylog file writing with environment variable +TEST_F(EnvoyQuicProofSourceTest, TestKeylogFileWriting) { + // Create a temporary file for keylog output + std::string temp_file = "/tmp/test_keylog_" + std::to_string(getpid()) + ".txt"; + + // Set SSLKEYLOGFILE environment variable + setenv("SSLKEYLOGFILE", temp_file.c_str(), 1); + + // Create SSL_CTX and setup keylog + bssl::UniquePtr ssl_ctx(SSL_CTX_new(TLS_method())); + proof_source_.OnNewSslCtx(ssl_ctx.get()); + + // Create SSL connection + bssl::UniquePtr ssl(SSL_new(ssl_ctx.get())); + + // Get the keylog callback and call it to test functionality + void (*callback)(const SSL*, const char*) = SSL_CTX_get_keylog_callback(ssl_ctx.get()); + ASSERT_NE(callback, nullptr); + + // Call the callback with test data + const char* test_line = "CLIENT_RANDOM 0123456789abcdef test_key_material"; + callback(ssl.get(), test_line); + + // Verify the keylog was written to file + std::ifstream keylog_file(temp_file); + ASSERT_TRUE(keylog_file.is_open()); + std::string line; + ASSERT_TRUE(std::getline(keylog_file, line)); + EXPECT_EQ(line, test_line); + keylog_file.close(); + + // Clean up + unlink(temp_file.c_str()); + unsetenv("SSLKEYLOGFILE"); +} + +// Test keylog callback without environment variable +TEST_F(EnvoyQuicProofSourceTest, TestKeylogCallbackWithoutEnvironmentVariable) { + // Ensure SSLKEYLOGFILE is not set + unsetenv("SSLKEYLOGFILE"); + + // Create SSL_CTX and setup keylog + bssl::UniquePtr ssl_ctx(SSL_CTX_new(TLS_method())); + proof_source_.OnNewSslCtx(ssl_ctx.get()); + + // Verify that keylog callback is still registered (even without env var) + void (*callback)(const SSL*, const char*) = SSL_CTX_get_keylog_callback(ssl_ctx.get()); + EXPECT_NE(callback, nullptr); + + // Create SSL connection and test that callback doesn't crash without env var + bssl::UniquePtr ssl(SSL_new(ssl_ctx.get())); + + // Call the callback - it should not crash even without SSLKEYLOGFILE set + const char* test_line = "CLIENT_RANDOM 0123456789abcdef test_key_material"; + EXPECT_NO_THROW(callback(ssl.get(), test_line)); +} + +// Test QUIC keylog bridge functionality +TEST_F(EnvoyQuicProofSourceTest, TestQuicKeylogBridge) { + // Create a mock context config with keylog configuration + NiceMock mock_config; + NiceMock mock_access_log_manager; + auto mock_access_log_file = std::make_shared>(); + + std::string keylog_path = "/tmp/test_bridge_keylog_" + std::to_string(getpid()) + ".txt"; + + // Setup mock expectations + EXPECT_CALL(mock_config, tlsKeyLogPath()) + .WillRepeatedly(ReturnRef(keylog_path)); + + Network::Address::IpList empty_ip_list; + EXPECT_CALL(mock_config, tlsKeyLogLocal()) + .WillRepeatedly(ReturnRef(empty_ip_list)); + EXPECT_CALL(mock_config, tlsKeyLogRemote()) + .WillRepeatedly(ReturnRef(empty_ip_list)); + + EXPECT_CALL(mock_config, accessLogManager()) + .WillRepeatedly(ReturnRef(mock_access_log_manager)); + + EXPECT_CALL(mock_access_log_manager, createAccessLog(_)) + .WillOnce(Return(absl::StatusOr(mock_access_log_file))); + + EXPECT_CALL(*mock_access_log_file, write(_)) + .Times(1); + + // Create test addresses + auto local_addr = Network::Test::getCanonicalLoopbackAddress(Network::Address::IpVersion::v4); + auto remote_addr = Network::Test::getCanonicalLoopbackAddress(Network::Address::IpVersion::v4); + + // Test the bridge functionality + const char* test_line = "CLIENT_RANDOM 123456789 ABCDEF"; + EnvoyQuicProofSource::QuicKeylogBridge::writeKeylog(mock_config, *local_addr, *remote_addr, test_line); +} + +// Test the complete keylog callback flow including SSL context setup +TEST_F(EnvoyQuicProofSourceTest, TestKeylogCallbackWithSslContext) { + // Create an SSL context to test the callback registration + bssl::UniquePtr ssl_ctx(SSL_CTX_new(TLS_method())); + ASSERT_NE(ssl_ctx, nullptr); + + // Use OnNewSslCtx which calls setupQuicKeylogCallback internally + proof_source_.OnNewSslCtx(ssl_ctx.get()); + + // Create an SSL connection + bssl::UniquePtr ssl(SSL_new(ssl_ctx.get())); + ASSERT_NE(ssl, nullptr); + + // Verify that the keylog callback is set + auto callback = SSL_CTX_get_keylog_callback(ssl_ctx.get()); + EXPECT_NE(callback, nullptr); + + // Verify that the proof source is stored as app data + auto stored_proof_source = SSL_CTX_get_app_data(ssl_ctx.get()); + EXPECT_EQ(stored_proof_source, &proof_source_); + + // Test calling the callback - it should handle the case where transport socket callbacks are not available + const char* test_line = "CLIENT_RANDOM 0123456789abcdef test_key_material"; + + // Set up environment variable for fallback test + std::string keylog_path = "/tmp/test_callback_keylog_" + std::to_string(getpid()) + ".txt"; + setenv("SSLKEYLOGFILE", keylog_path.c_str(), 1); + + EXPECT_NO_THROW(callback(ssl.get(), test_line)); + + // Check that the keylog was written via environment variable fallback + std::ifstream keylog_file(keylog_path); + EXPECT_TRUE(keylog_file.good()); + if (keylog_file.good()) { + std::string line; + std::getline(keylog_file, line); + EXPECT_EQ(line, test_line); + } + + // Clean up + unsetenv("SSLKEYLOGFILE"); + unlink(keylog_path.c_str()); +} + } // namespace Quic } // namespace Envoy diff --git a/test/common/tls/BUILD b/test/common/tls/BUILD index bd5867e46ee10..ebe4d334fcedd 100644 --- a/test/common/tls/BUILD +++ b/test/common/tls/BUILD @@ -293,6 +293,16 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "cert_compression_test", + srcs = ["cert_compression_test.cc"], + external_deps = ["ssl"], + deps = [ + "//source/common/tls:cert_compression_lib", + "//test/test_common:logging_lib", + ], +) + envoy_cc_benchmark_binary( name = "tls_throughput_benchmark", srcs = ["tls_throughput_benchmark.cc"], diff --git a/test/common/tls/cert_compression_test.cc b/test/common/tls/cert_compression_test.cc new file mode 100644 index 0000000000000..d7231d2cfe432 --- /dev/null +++ b/test/common/tls/cert_compression_test.cc @@ -0,0 +1,260 @@ +#include "source/common/tls/cert_compression.h" + +#include "test/test_common/logging.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace TransportSockets { +namespace Tls { + +// Test data for round-trip compression tests +constexpr uint8_t kTestData[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; +constexpr size_t kTestDataLen = sizeof(kTestData); + +// +// Brotli Tests +// + +TEST(CertCompressionBrotliTest, RoundTrip) { + // Compress + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::compressBrotli(nullptr, compressed.get(), kTestData, kTestDataLen)); + const auto compressed_len = CBB_len(compressed.get()); + EXPECT_GT(compressed_len, 0u); + + // Decompress + CRYPTO_BUFFER* out = nullptr; + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::decompressBrotli(nullptr, &out, kTestDataLen, + CBB_data(compressed.get()), compressed_len)); + ASSERT_NE(nullptr, out); + bssl::UniquePtr out_ptr(out); + + // Verify + EXPECT_EQ(kTestDataLen, CRYPTO_BUFFER_len(out)); + EXPECT_EQ(0, memcmp(kTestData, CRYPTO_BUFFER_data(out), kTestDataLen)); +} + +TEST(CertCompressionBrotliTest, DecompressBadData) { + EXPECT_LOG_CONTAINS( + "error", + "Cert brotli decompression failure, possibly caused by invalid compressed cert from peer", { + CRYPTO_BUFFER* out = nullptr; + const uint8_t bad_compressed_data = 1; + EXPECT_EQ(CertCompression::FAILURE, + CertCompression::decompressBrotli(nullptr, &out, 100, &bad_compressed_data, + sizeof(bad_compressed_data))); + }); +} + +TEST(CertCompressionBrotliTest, DecompressBadLength) { + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + ASSERT_EQ(CertCompression::SUCCESS, + CertCompression::compressBrotli(nullptr, compressed.get(), kTestData, kTestDataLen)); + const auto compressed_len = CBB_len(compressed.get()); + EXPECT_GT(compressed_len, 0u); + + EXPECT_LOG_CONTAINS( + "error", "Brotli decompression length did not match peer provided uncompressed length", { + CRYPTO_BUFFER* out = nullptr; + EXPECT_EQ(CertCompression::FAILURE, + CertCompression::decompressBrotli(nullptr, &out, + kTestDataLen + 1 /* intentionally incorrect */, + CBB_data(compressed.get()), compressed_len)); + }); +} + +// +// Zstd Tests +// + +TEST(CertCompressionZstdTest, RoundTrip) { + // Compress + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::compressZstd(nullptr, compressed.get(), kTestData, kTestDataLen)); + const auto compressed_len = CBB_len(compressed.get()); + EXPECT_GT(compressed_len, 0u); + + // Decompress + CRYPTO_BUFFER* out = nullptr; + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::decompressZstd(nullptr, &out, kTestDataLen, CBB_data(compressed.get()), + compressed_len)); + ASSERT_NE(nullptr, out); + bssl::UniquePtr out_ptr(out); + + // Verify + EXPECT_EQ(kTestDataLen, CRYPTO_BUFFER_len(out)); + EXPECT_EQ(0, memcmp(kTestData, CRYPTO_BUFFER_data(out), kTestDataLen)); +} + +TEST(CertCompressionZstdTest, DecompressBadData) { + EXPECT_LOG_CONTAINS( + "error", + "Cert zstd decompression failure, possibly caused by invalid compressed cert from peer", { + CRYPTO_BUFFER* out = nullptr; + const uint8_t bad_compressed_data = 1; + EXPECT_EQ(CertCompression::FAILURE, + CertCompression::decompressZstd(nullptr, &out, 100, &bad_compressed_data, + sizeof(bad_compressed_data))); + }); +} + +TEST(CertCompressionZstdTest, DecompressBadLength) { + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + ASSERT_EQ(CertCompression::SUCCESS, + CertCompression::compressZstd(nullptr, compressed.get(), kTestData, kTestDataLen)); + const auto compressed_len = CBB_len(compressed.get()); + EXPECT_GT(compressed_len, 0u); + + EXPECT_LOG_CONTAINS( + "error", "Zstd decompression length did not match peer provided uncompressed length", { + CRYPTO_BUFFER* out = nullptr; + EXPECT_EQ(CertCompression::FAILURE, + CertCompression::decompressZstd(nullptr, &out, + kTestDataLen + 1 /* intentionally incorrect */, + CBB_data(compressed.get()), compressed_len)); + }); +} + +// +// Zlib Tests +// + +TEST(CertCompressionZlibTest, RoundTrip) { + // Compress + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::compressZlib(nullptr, compressed.get(), kTestData, kTestDataLen)); + const auto compressed_len = CBB_len(compressed.get()); + EXPECT_GT(compressed_len, 0u); + + // Decompress + CRYPTO_BUFFER* out = nullptr; + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::decompressZlib(nullptr, &out, kTestDataLen, CBB_data(compressed.get()), + compressed_len)); + ASSERT_NE(nullptr, out); + bssl::UniquePtr out_ptr(out); + + // Verify + EXPECT_EQ(kTestDataLen, CRYPTO_BUFFER_len(out)); + EXPECT_EQ(0, memcmp(kTestData, CRYPTO_BUFFER_data(out), kTestDataLen)); +} + +TEST(CertCompressionZlibTest, DecompressBadData) { + EXPECT_LOG_CONTAINS( + "error", + "Cert zlib decompression failure, possibly caused by invalid compressed cert from peer", { + CRYPTO_BUFFER* out = nullptr; + const uint8_t bad_compressed_data = 1; + EXPECT_EQ(CertCompression::FAILURE, + CertCompression::decompressZlib(nullptr, &out, 100, &bad_compressed_data, + sizeof(bad_compressed_data))); + }); +} + +TEST(CertCompressionZlibTest, DecompressBadLength) { + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + ASSERT_EQ(CertCompression::SUCCESS, + CertCompression::compressZlib(nullptr, compressed.get(), kTestData, kTestDataLen)); + const auto compressed_len = CBB_len(compressed.get()); + EXPECT_GT(compressed_len, 0u); + + EXPECT_LOG_CONTAINS("error", + "Zlib decompression length did not match peer provided uncompressed " + "length, caused by either invalid peer handshake data or decompression " + "error", + { + CRYPTO_BUFFER* out = nullptr; + EXPECT_EQ(CertCompression::FAILURE, + CertCompression::decompressZlib( + nullptr, &out, kTestDataLen + 1 /* intentionally incorrect */, + CBB_data(compressed.get()), compressed_len)); + }); +} + +// +// Stats Recording Tests +// These tests verify that compression succeeds even when SSL context is null +// (stats recording is safely skipped in this case) +// + +TEST(CertCompressionStatsTest, CompressBrotliWithNullSslRecordsNoStats) { + // Verify compression succeeds and doesn't crash when SSL is null + // (stats recording is safely skipped) + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::compressBrotli(nullptr, compressed.get(), kTestData, kTestDataLen)); + EXPECT_GT(CBB_len(compressed.get()), 0u); +} + +TEST(CertCompressionStatsTest, CompressZstdWithNullSslRecordsNoStats) { + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::compressZstd(nullptr, compressed.get(), kTestData, kTestDataLen)); + EXPECT_GT(CBB_len(compressed.get()), 0u); +} + +TEST(CertCompressionStatsTest, CompressZlibWithNullSslRecordsNoStats) { + bssl::ScopedCBB compressed; + ASSERT_EQ(1, CBB_init(compressed.get(), 0)); + EXPECT_EQ(CertCompression::SUCCESS, + CertCompression::compressZlib(nullptr, compressed.get(), kTestData, kTestDataLen)); + EXPECT_GT(CBB_len(compressed.get()), 0u); +} + +// +// Registration Tests +// These tests verify that the compression algorithms can be registered with SSL_CTX +// + +class CertCompressionRegistrationTest : public testing::Test { +protected: + void SetUp() override { + ssl_ctx_.reset(SSL_CTX_new(TLS_method())); + ASSERT_NE(nullptr, ssl_ctx_.get()); + } + + bssl::UniquePtr ssl_ctx_; +}; + +TEST_F(CertCompressionRegistrationTest, RegisterBrotli) { + // Verify brotli registration succeeds without crashing + EXPECT_NO_THROW(CertCompression::registerBrotli(ssl_ctx_.get())); +} + +TEST_F(CertCompressionRegistrationTest, RegisterZstd) { + // Verify zstd registration succeeds without crashing + EXPECT_NO_THROW(CertCompression::registerZstd(ssl_ctx_.get())); +} + +TEST_F(CertCompressionRegistrationTest, RegisterZlib) { + // Verify zlib registration succeeds without crashing + EXPECT_NO_THROW(CertCompression::registerZlib(ssl_ctx_.get())); +} + +TEST_F(CertCompressionRegistrationTest, RegisterAllAlgorithms) { + // Verify all algorithms can be registered on the same context + // Order matters: brotli > zstd > zlib (by priority) + EXPECT_NO_THROW(CertCompression::registerBrotli(ssl_ctx_.get())); + EXPECT_NO_THROW(CertCompression::registerZstd(ssl_ctx_.get())); + EXPECT_NO_THROW(CertCompression::registerZlib(ssl_ctx_.get())); +} + +} // namespace Tls +} // namespace TransportSockets +} // namespace Extensions +} // namespace Envoy diff --git a/test/common/tls/ssl_socket_test.cc b/test/common/tls/ssl_socket_test.cc index f9a69cc9fdd42..76b2d05fd7b04 100644 --- a/test/common/tls/ssl_socket_test.cc +++ b/test/common/tls/ssl_socket_test.cc @@ -7914,6 +7914,59 @@ TEST_P(SslSocketTest, RsaKeyUsageVerificationEnforcementOn) { testUtilV2(test_options); } +// Test that TLS handshakes succeed when certificate compression is enabled via runtime flag. +// This verifies the certificate compression feature (RFC 8879) integration with brotli, zstd, +// and zlib algorithms when the runtime flag is enabled. +TEST_P(SslSocketTest, CertificateCompressionEnabled) { + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues( + {{"envoy.reloadable_features.tls_support_certificate_compression", "true"}}); + + envoy::config::listener::v3::Listener listener; + envoy::config::listener::v3::FilterChain* filter_chain = listener.add_filter_chains(); + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext server_tls_context; + envoy::extensions::transport_sockets::tls::v3::TlsCertificate* server_cert = + server_tls_context.mutable_common_tls_context()->add_tls_certificates(); + server_cert->mutable_certificate_chain()->set_filename( + TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns_cert.pem")); + server_cert->mutable_private_key()->set_filename( + TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns_key.pem")); + + updateFilterChain(server_tls_context, *filter_chain); + + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; + + // TLS handshake should succeed with compression algorithms registered. + TestUtilOptionsV2 test_options(listener, client_tls_context, /*expect_success=*/true, version_); + testUtilV2(test_options); +} + +// Test that TLS handshakes succeed with certificate compression disabled (default behavior). +// This verifies backward compatibility when the runtime flag is disabled. +TEST_P(SslSocketTest, CertificateCompressionDisabled) { + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues( + {{"envoy.reloadable_features.tls_support_certificate_compression", "false"}}); + + envoy::config::listener::v3::Listener listener; + envoy::config::listener::v3::FilterChain* filter_chain = listener.add_filter_chains(); + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext server_tls_context; + envoy::extensions::transport_sockets::tls::v3::TlsCertificate* server_cert = + server_tls_context.mutable_common_tls_context()->add_tls_certificates(); + server_cert->mutable_certificate_chain()->set_filename( + TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns_cert.pem")); + server_cert->mutable_private_key()->set_filename( + TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns_key.pem")); + + updateFilterChain(server_tls_context, *filter_chain); + + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; + + // TLS handshake should succeed without compression algorithms (backward compatibility). + TestUtilOptionsV2 test_options(listener, client_tls_context, /*expect_success=*/true, version_); + testUtilV2(test_options); +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/test/extensions/clusters/redis/redis_cluster_lb_test.cc b/test/extensions/clusters/redis/redis_cluster_lb_test.cc index 3bd635f646d2a..bc90cb1dbf7a0 100644 --- a/test/extensions/clusters/redis/redis_cluster_lb_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_lb_test.cc @@ -20,8 +20,10 @@ class TestLoadBalancerContext : public RedisLoadBalancerContext, public Upstream::LoadBalancerContextBase { public: TestLoadBalancerContext(uint64_t hash_key, bool is_read, - NetworkFilters::Common::Redis::Client::ReadPolicy read_policy) - : hash_key_(hash_key), is_read_(is_read), read_policy_(read_policy) {} + NetworkFilters::Common::Redis::Client::ReadPolicy read_policy, + const std::string& client_zone = "") + : hash_key_(hash_key), is_read_(is_read), read_policy_(read_policy), + client_zone_(client_zone) {} TestLoadBalancerContext(absl::optional hash) : hash_key_(hash) {} @@ -32,16 +34,25 @@ class TestLoadBalancerContext : public RedisLoadBalancerContext, NetworkFilters::Common::Redis::Client::ReadPolicy readPolicy() const override { return read_policy_; }; + const std::string& clientZone() const override { return client_zone_; } absl::optional hash_key_; bool is_read_; NetworkFilters::Common::Redis::Client::ReadPolicy read_policy_; + std::string client_zone_; }; class RedisClusterLoadBalancerTest : public Event::TestUsingSimulatedTime, public testing::Test { public: RedisClusterLoadBalancerTest() = default; + // Helper to create a host with locality zone set + Upstream::HostSharedPtr makeHostWithZone(const std::string& url, const std::string& zone) { + envoy::config::core::v3::Locality locality; + locality.set_zone(zone); + return Upstream::makeTestHost(info_, url, locality); + } + void init() { factory_ = std::make_shared(random_); lb_ = std::make_unique(factory_); @@ -53,11 +64,12 @@ class RedisClusterLoadBalancerTest : public Event::TestUsingSimulatedTime, publi const std::vector>& expected_assignments, bool read_command = false, NetworkFilters::Common::Redis::Client::ReadPolicy read_policy = - NetworkFilters::Common::Redis::Client::ReadPolicy::Primary) { + NetworkFilters::Common::Redis::Client::ReadPolicy::Primary, + const std::string& client_zone = "") { Upstream::LoadBalancerPtr lb = lb_->factory()->create(lb_params_); for (auto& assignment : expected_assignments) { - TestLoadBalancerContext context(assignment.first, read_command, read_policy); + TestLoadBalancerContext context(assignment.first, read_command, read_policy, client_zone); auto host = lb->chooseHost(&context).host; EXPECT_FALSE(host == nullptr); EXPECT_EQ(hosts[assignment.second]->address()->asString(), host->address()->asString()); @@ -604,6 +616,273 @@ TEST_F(RedisLoadBalancerContextImplTest, EnforceHashTag) { EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::Primary, context2.readPolicy()); } +TEST_F(RedisLoadBalancerContextImplTest, ReadOnlyCommand) { + std::vector eval_ro_foo(4); + eval_ro_foo[0].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[0].asString() = "eval_ro"; + eval_ro_foo[1].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[1].asString() = "return {KEYS[1]}"; + eval_ro_foo[2].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[2].asString() = "foo"; + eval_ro_foo[3].type(NetworkFilters::Common::Redis::RespType::BulkString); + eval_ro_foo[3].asString() = "0"; + + NetworkFilters::Common::Redis::RespValue eval_ro_request; + eval_ro_request.type(NetworkFilters::Common::Redis::RespType::Array); + eval_ro_request.asArray().swap(eval_ro_foo); + + RedisLoadBalancerContextImpl context1( + "foo", true, true, eval_ro_request, + NetworkFilters::Common::Redis::Client::ReadPolicy::PreferReplica); + + EXPECT_EQ(absl::optional(44950), context1.computeHashKey()); + EXPECT_EQ(true, context1.isReadCommand()); + EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::PreferReplica, + context1.readPolicy()); +} + +TEST_F(RedisLoadBalancerContextImplTest, ClientZone) { + std::vector get_foo(2); + get_foo[0].type(NetworkFilters::Common::Redis::RespType::BulkString); + get_foo[0].asString() = "get"; + get_foo[1].type(NetworkFilters::Common::Redis::RespType::BulkString); + get_foo[1].asString() = "foo"; + + NetworkFilters::Common::Redis::RespValue get_request; + get_request.type(NetworkFilters::Common::Redis::RespType::Array); + get_request.asArray().swap(get_foo); + + // Test with client zone specified + RedisLoadBalancerContextImpl context1( + "foo", true, true, get_request, NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity, + "us-east-1a"); + + EXPECT_EQ("us-east-1a", context1.clientZone()); + EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity, context1.readPolicy()); + + // Test with empty client zone + RedisLoadBalancerContextImpl context2( + "foo", true, true, get_request, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary); + + EXPECT_EQ("", context2.clientZone()); + EXPECT_EQ(NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary, + context2.readPolicy()); +} + +// Tests for AZ_AFFINITY read policy with replicas in the same zone +TEST_F(RedisClusterLoadBalancerTest, AzAffinityWithLocalReplica) { + // Setup: primary in zone-a, replica in zone-a (same as client) + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary, zone-a + makeHostWithZone("tcp://127.0.0.1:91", "zone-b"), // primary, zone-b + makeHostWithZone("tcp://127.0.0.2:90", "zone-a"), // replica for slot 0, zone-a + makeHostWithZone("tcp://127.0.0.2:91", "zone-b"), // replica for slot 1, zone-b + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 8000, hosts[0]->address()), + ClusterSlot(8001, 16383, hosts[1]->address()), + }); + slots->at(0).addReplica(hosts[2]->address()); + slots->at(1).addReplica(hosts[3]->address()); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // Client is in zone-a, should prefer replica in zone-a for slot 0 + const std::vector> expected_assignments = { + {0, 2}, // slot 0: replica in zone-a + {8001, 3}, // slot 1: replica in zone-b (no local replica, fall back to any replica) + }; + validateAssignment(hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity, "zone-a"); +} + +// Tests for AZ_AFFINITY when no local replica exists - should fall back to any replica +TEST_F(RedisClusterLoadBalancerTest, AzAffinityNoLocalReplica) { + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary, zone-a + makeHostWithZone("tcp://127.0.0.2:90", "zone-b"), // replica, zone-b + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + slots->at(0).addReplica(hosts[1]->address()); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // Client is in zone-c (no local replica), should fall back to any replica + const std::vector> expected_assignments = { + {0, 1}, // falls back to replica in zone-b + }; + validateAssignment(hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity, "zone-c"); +} + +// Tests for AZ_AFFINITY when no replica exists - should fall back to primary +TEST_F(RedisClusterLoadBalancerTest, AzAffinityNoReplica) { + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary only + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // No replicas, should fall back to primary + const std::vector> expected_assignments = { + {0, 0}, // falls back to primary + }; + validateAssignment(hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity, "zone-a"); +} + +// Tests for AZ_AFFINITY_REPLICAS_AND_PRIMARY with local replica +TEST_F(RedisClusterLoadBalancerTest, AzAffinityReplicasAndPrimaryWithLocalReplica) { + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary, zone-a + makeHostWithZone("tcp://127.0.0.2:90", "zone-a"), // replica, zone-a (same as client) + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + slots->at(0).addReplica(hosts[1]->address()); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // Client is in zone-a, should prefer replica in zone-a + const std::vector> expected_assignments = { + {0, 1}, // replica in zone-a + }; + validateAssignment( + hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary, "zone-a"); +} + +// Tests for AZ_AFFINITY_REPLICAS_AND_PRIMARY - prefer local primary when no local replica +TEST_F(RedisClusterLoadBalancerTest, AzAffinityReplicasAndPrimaryLocalPrimary) { + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary, zone-a (same as client) + makeHostWithZone("tcp://127.0.0.2:90", "zone-b"), // replica, zone-b + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + slots->at(0).addReplica(hosts[1]->address()); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // Client is in zone-a, no local replica, but primary is in zone-a + const std::vector> expected_assignments = { + {0, 0}, // primary in zone-a (no local replica, but local primary) + }; + validateAssignment( + hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary, "zone-a"); +} + +// Tests for AZ_AFFINITY_REPLICAS_AND_PRIMARY - fall back to any replica when no local hosts +TEST_F(RedisClusterLoadBalancerTest, AzAffinityReplicasAndPrimaryFallbackToReplica) { + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary, zone-a + makeHostWithZone("tcp://127.0.0.2:90", "zone-b"), // replica, zone-b + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + slots->at(0).addReplica(hosts[1]->address()); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // Client is in zone-c (no local hosts), should fall back to any replica + const std::vector> expected_assignments = { + {0, 1}, // falls back to replica + }; + validateAssignment( + hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary, "zone-c"); +} + +// Tests for AZ_AFFINITY_REPLICAS_AND_PRIMARY - fall back to primary when no replicas +TEST_F(RedisClusterLoadBalancerTest, AzAffinityReplicasAndPrimaryNoReplica) { + // Hosts must have locality zones set - RedisShard reads zone from host->locality().zone() + Upstream::HostVector hosts{ + makeHostWithZone("tcp://127.0.0.1:90", "zone-a"), // primary only, zone-a + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // No replicas, should fall back to primary + const std::vector> expected_assignments = { + {0, 0}, // falls back to primary + }; + validateAssignment( + hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinityReplicasAndPrimary, "zone-c"); +} + +// Tests for AZ_AFFINITY with empty client zone - should behave like PreferReplica +TEST_F(RedisClusterLoadBalancerTest, AzAffinityEmptyClientZone) { + Upstream::HostVector hosts{ + Upstream::makeTestHost(info_, "tcp://127.0.0.1:90"), // primary + Upstream::makeTestHost(info_, "tcp://127.0.0.2:90"), // replica + }; + + ClusterSlotsPtr slots = std::make_unique>(std::vector{ + ClusterSlot(0, 16383, hosts[0]->address()), + }); + slots->at(0).addReplica(hosts[1]->address()); + + Upstream::HostMap all_hosts; + std::transform(hosts.begin(), hosts.end(), std::inserter(all_hosts, all_hosts.end()), makePair); + init(); + factory_->onClusterSlotUpdate(std::move(slots), all_hosts); + + // Empty client zone, should fall back to any replica + const std::vector> expected_assignments = { + {0, 1}, // any replica + }; + validateAssignment(hosts, expected_assignments, true, + NetworkFilters::Common::Redis::Client::ReadPolicy::AzAffinity, ""); +} + } // namespace Redis } // namespace Clusters } // namespace Extensions diff --git a/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc b/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc index a30ad3706e32d..8b1ed47de8f4f 100644 --- a/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc +++ b/test/extensions/filters/network/redis_proxy/command_splitter_impl_test.cc @@ -529,6 +529,50 @@ TEST_F(RedisSingleServerRequestTest, EvalShaSuccess) { store_.counter(fmt::format("redis.foo.command.{}.success", lower_command)).value()); }; +TEST_F(RedisSingleServerRequestTest, EvalRoSuccess) { + InSequence s; + + Common::Redis::RespValuePtr request{new Common::Redis::RespValue()}; + makeBulkStringArray(*request, {"eval_ro", "return {ARGV[1]}", "1", "key", "arg"}); + makeRequest("key", std::move(request)); + EXPECT_NE(nullptr, handle_); + + std::string lower_command = absl::AsciiStrToLower("eval_ro"); + + time_system_.setMonotonicTime(std::chrono::milliseconds(10)); + EXPECT_CALL(store_, deliverHistogramToSinks( + Property(&Stats::Metric::name, + fmt::format("redis.foo.command.{}.latency", lower_command)), + 10)); + respond(); + + EXPECT_EQ(1UL, store_.counter(fmt::format("redis.foo.command.{}.total", lower_command)).value()); + EXPECT_EQ(1UL, + store_.counter(fmt::format("redis.foo.command.{}.success", lower_command)).value()); +}; + +TEST_F(RedisSingleServerRequestTest, EvalShaRoSuccess) { + InSequence s; + + Common::Redis::RespValuePtr request{new Common::Redis::RespValue()}; + makeBulkStringArray(*request, {"EVALSHA_RO", "return {ARGV[1]}", "1", "keykey", "arg"}); + makeRequest("keykey", std::move(request)); + EXPECT_NE(nullptr, handle_); + + std::string lower_command = absl::AsciiStrToLower("evalsha_ro"); + + time_system_.setMonotonicTime(std::chrono::milliseconds(10)); + EXPECT_CALL(store_, deliverHistogramToSinks( + Property(&Stats::Metric::name, + fmt::format("redis.foo.command.{}.latency", lower_command)), + 10)); + respond(); + + EXPECT_EQ(1UL, store_.counter(fmt::format("redis.foo.command.{}.total", lower_command)).value()); + EXPECT_EQ(1UL, + store_.counter(fmt::format("redis.foo.command.{}.success", lower_command)).value()); +}; + TEST_F(RedisSingleServerRequestTest, EvalWrongNumberOfArgs) { InSequence s; diff --git a/test/extensions/quic/proof_source/pending_proof_source_factory_impl.cc b/test/extensions/quic/proof_source/pending_proof_source_factory_impl.cc index 9dd472518eb34..7887f9c7aa636 100644 --- a/test/extensions/quic/proof_source/pending_proof_source_factory_impl.cc +++ b/test/extensions/quic/proof_source/pending_proof_source_factory_impl.cc @@ -9,8 +9,10 @@ class PendingProofSource : public EnvoyQuicProofSource { public: PendingProofSource(Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) - : EnvoyQuicProofSource(listen_socket, filter_chain_manager, listener_stats, time_source) {} + Server::ListenerStats& listener_stats, TimeSource& time_source, + Stats::Scope& stats_scope) + : EnvoyQuicProofSource(listen_socket, filter_chain_manager, listener_stats, time_source, + stats_scope) {} protected: void signPayload(const quic::QuicSocketAddress& /*server_address*/, @@ -28,9 +30,9 @@ class PendingProofSource : public EnvoyQuicProofSource { std::unique_ptr PendingProofSourceFactoryImpl::createQuicProofSource( Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) { + Server::ListenerStats& listener_stats, TimeSource& time_source, Stats::Scope& stats_scope) { return std::make_unique(listen_socket, filter_chain_manager, listener_stats, - time_source); + time_source, stats_scope); } REGISTER_FACTORY(PendingProofSourceFactoryImpl, EnvoyQuicProofSourceFactoryInterface); diff --git a/test/extensions/quic/proof_source/pending_proof_source_factory_impl.h b/test/extensions/quic/proof_source/pending_proof_source_factory_impl.h index 0643b371c51fe..abd86c03dc8b7 100644 --- a/test/extensions/quic/proof_source/pending_proof_source_factory_impl.h +++ b/test/extensions/quic/proof_source/pending_proof_source_factory_impl.h @@ -21,7 +21,8 @@ class PendingProofSourceFactoryImpl : public EnvoyQuicProofSourceFactoryInterfac std::unique_ptr createQuicProofSource(Network::Socket& listen_socket, Network::FilterChainManager& filter_chain_manager, - Server::ListenerStats& listener_stats, TimeSource& time_source) override; + Server::ListenerStats& listener_stats, TimeSource& time_source, + Stats::Scope& stats_scope) override; }; DECLARE_FACTORY(PendingProofSourceFactoryImpl); diff --git a/test/extensions/request_id/uuid/BUILD b/test/extensions/request_id/uuid/BUILD index a139870f1828a..029b3cf91d184 100644 --- a/test/extensions/request_id/uuid/BUILD +++ b/test/extensions/request_id/uuid/BUILD @@ -18,5 +18,6 @@ envoy_cc_test( "//source/extensions/request_id/uuid:config", "//test/mocks/runtime:runtime_mocks", "//test/mocks/stream_info:stream_info_mocks", + "//test/test_common:simulated_time_system_lib", ], ) diff --git a/test/extensions/request_id/uuid/config_test.cc b/test/extensions/request_id/uuid/config_test.cc index 277519addee8d..054ab1f5699e9 100644 --- a/test/extensions/request_id/uuid/config_test.cc +++ b/test/extensions/request_id/uuid/config_test.cc @@ -5,6 +5,7 @@ #include "test/mocks/common.h" #include "test/mocks/stream_info/mocks.h" +#include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -16,24 +17,30 @@ namespace Extensions { namespace RequestId { TEST(UUIDRequestIDExtensionTest, SetRequestID) { - testing::StrictMock random; + testing::NiceMock random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); { // edge_request: true, keep_external_id: false. Http::TestRequestHeaderMapImpl request_headers; - // Without request ID. - EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id")); + // Without request ID. generateUuidV7() calls random.random() twice. uuid_utils.set(request_headers, true, false); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string first_request_id = std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, first_request_id.length()); + EXPECT_EQ('f', first_request_id[0]); // UUIDv7 request_id marker + EXPECT_EQ('7', first_request_id[14]); // UUIDv7 version // With request ID. Previous one will be overwritten. - EXPECT_CALL(random, uuid()).WillOnce(Return("second-request-id")); uuid_utils.set(request_headers, true, false); - EXPECT_EQ("second-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string second_request_id = + std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, second_request_id.length()); + EXPECT_EQ('f', second_request_id[0]); + EXPECT_EQ('7', second_request_id[14]); } { @@ -41,15 +48,14 @@ TEST(UUIDRequestIDExtensionTest, SetRequestID) { Http::TestRequestHeaderMapImpl request_headers; - // Without request ID. - EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id")); + // Without request ID. generateUuidV7() calls random.random() twice. uuid_utils.set(request_headers, true, true); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string first_request_id = std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, first_request_id.length()); // With request ID. Previous one will be kept. - EXPECT_CALL(random, uuid()).Times(0); uuid_utils.set(request_headers, true, true); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(first_request_id, request_headers.get_(Http::Headers::get().RequestId)); } { @@ -57,15 +63,14 @@ TEST(UUIDRequestIDExtensionTest, SetRequestID) { Http::TestRequestHeaderMapImpl request_headers; - // Without request ID. - EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id")); + // Without request ID. generateUuidV7() calls random.random() twice. uuid_utils.set(request_headers, false, false); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string first_request_id = std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, first_request_id.length()); // With request ID. Previous one will be kept. - EXPECT_CALL(random, uuid()).Times(0); uuid_utils.set(request_headers, false, false); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(first_request_id, request_headers.get_(Http::Headers::get().RequestId)); } { @@ -73,32 +78,34 @@ TEST(UUIDRequestIDExtensionTest, SetRequestID) { Http::TestRequestHeaderMapImpl request_headers; - // Without request ID. - EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id")); + // Without request ID. generateUuidV7() calls random.random() twice. uuid_utils.set(request_headers, false, true); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string first_request_id = std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, first_request_id.length()); // With request ID. Previous one will be kept. - EXPECT_CALL(random, uuid()).Times(0); uuid_utils.set(request_headers, false, true); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(first_request_id, request_headers.get_(Http::Headers::get().RequestId)); } } TEST(UUIDRequestIDExtensionTest, SetRequestIDWhenEmpty) { - testing::StrictMock random; + testing::NiceMock random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); { // Request ID not set. Http::TestRequestHeaderMapImpl request_headers; - // A new request ID will be set. - EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id")); + // A new request ID will be set. generateUuidV7() calls random.random() twice. uuid_utils.set(request_headers, false, true); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string first_request_id = std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, first_request_id.length()); + EXPECT_EQ('f', first_request_id[0]); // UUIDv7 request_id marker + EXPECT_EQ('7', first_request_id[14]); // UUIDv7 version } { @@ -109,10 +116,12 @@ TEST(UUIDRequestIDExtensionTest, SetRequestIDWhenEmpty) { "", }}; - // A new request ID will be set. - EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id")); + // A new request ID will be set. generateUuidV7() calls random.random() twice. uuid_utils.set(request_headers, false, true); - EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId)); + std::string first_request_id = std::string(request_headers.get_(Http::Headers::get().RequestId)); + EXPECT_EQ(36, first_request_id.length()); + EXPECT_EQ('f', first_request_id[0]); + EXPECT_EQ('7', first_request_id[14]); } { @@ -124,7 +133,6 @@ TEST(UUIDRequestIDExtensionTest, SetRequestIDWhenEmpty) { }}; // The request ID will be kept. - EXPECT_CALL(random, uuid()).Times(0); uuid_utils.set(request_headers, false, true); EXPECT_EQ("some-request-id", request_headers.get_(Http::Headers::get().RequestId)); } @@ -132,8 +140,9 @@ TEST(UUIDRequestIDExtensionTest, SetRequestIDWhenEmpty) { TEST(UUIDRequestIDExtensionTest, ClearExternalTraceReason) { testing::NiceMock random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); std::string uuid_with_trace_reason = random.uuid_; @@ -147,9 +156,8 @@ TEST(UUIDRequestIDExtensionTest, ClearExternalTraceReason) { }}; std::string expected_uuid_with_trace_reason = uuid_with_trace_reason; - expected_uuid_with_trace_reason[14] = '4'; // 'b' means NO_TRACE. + expected_uuid_with_trace_reason[14] = '7'; // Clear to NO_TRACE (UUIDv7 version bit). - EXPECT_CALL(random, uuid()).Times(0); uuid_utils.set(request_headers, true, true); // External request ID will be kept but the trace reason will be cleared. @@ -157,9 +165,10 @@ TEST(UUIDRequestIDExtensionTest, ClearExternalTraceReason) { } TEST(UUIDRequestIDExtensionTest, PreserveRequestIDInResponse) { - testing::StrictMock random; + testing::NiceMock random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); Http::TestRequestHeaderMapImpl request_headers; Http::TestResponseHeaderMapImpl response_headers; @@ -181,63 +190,77 @@ TEST(UUIDRequestIDExtensionTest, PreserveRequestIDInResponse) { } TEST(UUIDRequestIDExtensionTest, GetRequestIdAndModRequestIDBy) { - Random::RandomGeneratorImpl random; + testing::NiceMock random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); Http::TestRequestHeaderMapImpl request_headers; EXPECT_FALSE(uuid_utils.get(request_headers)); EXPECT_FALSE(uuid_utils.getInteger(request_headers).has_value()); + // UUID too short (< 36 chars). request_headers.setRequestId("fffffff"); EXPECT_EQ("fffffff", uuid_utils.get(request_headers).value()); EXPECT_FALSE(uuid_utils.getInteger(request_headers).has_value()); - request_headers.setRequestId("fffffffz-0012-0110-00ff-0c00400600ff"); - EXPECT_EQ("fffffffz-0012-0110-00ff-0c00400600ff", uuid_utils.get(request_headers).value()); + // UUID with invalid hex char 'z' in the last 8 chars (position 28-35). + request_headers.setRequestId("fffffffz-0012-0110-00ff-0c00400600fz"); + EXPECT_EQ("fffffffz-0012-0110-00ff-0c00400600fz", uuid_utils.get(request_headers).value()); EXPECT_FALSE(uuid_utils.getInteger(request_headers).has_value()); + // getInteger() now uses last 8 hex chars (position 28-35). + // UUID: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + // 0 8 13 18 23 28 35 + // ^^^^^^^^ (last 8 hex chars) + + // "00000000-0000-0000-0000-000000000000" -> last 8 chars = "00000000" = 0 request_headers.setRequestId("00000000-0000-0000-0000-000000000000"); EXPECT_EQ("00000000-0000-0000-0000-000000000000", uuid_utils.get(request_headers).value()); EXPECT_EQ(0, uuid_utils.getInteger(request_headers).value()); - request_headers.setRequestId("00000001-0000-0000-0000-000000000000"); - EXPECT_EQ("00000001-0000-0000-0000-000000000000", uuid_utils.get(request_headers).value()); + // "00000001-0000-0000-0000-000000000001" -> last 8 chars = "00000001" = 1 + request_headers.setRequestId("00000001-0000-0000-0000-000000000001"); + EXPECT_EQ("00000001-0000-0000-0000-000000000001", uuid_utils.get(request_headers).value()); EXPECT_EQ(1, uuid_utils.getInteger(request_headers).value()); - request_headers.setRequestId("0000000f-0000-0000-0000-00000000000a"); - EXPECT_EQ("0000000f-0000-0000-0000-00000000000a", uuid_utils.get(request_headers).value()); - EXPECT_EQ(15, uuid_utils.getInteger(request_headers).value()); + // "0000000f-0000-0000-0000-0000000000ff" -> last 8 chars = "000000ff" = 255 + request_headers.setRequestId("0000000f-0000-0000-0000-0000000000ff"); + EXPECT_EQ("0000000f-0000-0000-0000-0000000000ff", uuid_utils.get(request_headers).value()); + EXPECT_EQ(255, uuid_utils.getInteger(request_headers).value()); request_headers.setRequestId(""); EXPECT_EQ("", uuid_utils.get(request_headers).value()); EXPECT_FALSE(uuid_utils.getInteger(request_headers).has_value()); - request_headers.setRequestId("000000ff-0000-0000-0000-000000000000"); - EXPECT_EQ("000000ff-0000-0000-0000-000000000000", uuid_utils.get(request_headers).value()); - EXPECT_EQ(55, uuid_utils.getInteger(request_headers).value() % 100); - - request_headers.setRequestId("000000ff-0000-0000-0000-000000000000"); - EXPECT_EQ("000000ff-0000-0000-0000-000000000000", uuid_utils.get(request_headers).value()); - EXPECT_EQ(255, uuid_utils.getInteger(request_headers).value()); + // "000000ff-0000-0000-0000-000012345678" -> last 8 chars = "12345678" = 0x12345678 + request_headers.setRequestId("000000ff-0000-0000-0000-000012345678"); + EXPECT_EQ("000000ff-0000-0000-0000-000012345678", uuid_utils.get(request_headers).value()); + EXPECT_EQ(0x12345678, uuid_utils.getInteger(request_headers).value()); + // "a0090100-0012-0110-00ff-0c00400600ff" -> last 8 chars = "400600ff" = 0x400600ff request_headers.setRequestId("a0090100-0012-0110-00ff-0c00400600ff"); EXPECT_EQ("a0090100-0012-0110-00ff-0c00400600ff", uuid_utils.get(request_headers).value()); - EXPECT_EQ(8, uuid_utils.getInteger(request_headers).value() % 137); + EXPECT_EQ(0x400600ff, uuid_utils.getInteger(request_headers).value()); - request_headers.setRequestId("ffffffff-0012-0110-00ff-0c00400600ff"); - EXPECT_EQ("ffffffff-0012-0110-00ff-0c00400600ff", uuid_utils.get(request_headers).value()); - EXPECT_EQ(95, uuid_utils.getInteger(request_headers).value() % 100); + // "ffffffff-0012-0110-00ff-0c00ffffffff" -> last 8 chars = "ffffffff" = 0xffffffff + request_headers.setRequestId("ffffffff-0012-0110-00ff-0c00ffffffff"); + EXPECT_EQ("ffffffff-0012-0110-00ff-0c00ffffffff", uuid_utils.get(request_headers).value()); + EXPECT_EQ(0xffffffff, uuid_utils.getInteger(request_headers).value()); + // Test modulo operations for sampling distribution. + // "ffffffff-0012-0110-00ff-0c00400600ff" -> last 8 chars = "400600ff" = 0x400600ff = 1073873151 request_headers.setRequestId("ffffffff-0012-0110-00ff-0c00400600ff"); EXPECT_EQ("ffffffff-0012-0110-00ff-0c00400600ff", uuid_utils.get(request_headers).value()); - EXPECT_EQ(7295, uuid_utils.getInteger(request_headers).value() % 10000); + EXPECT_EQ(1073873151 % 100, uuid_utils.getInteger(request_headers).value() % 100); + EXPECT_EQ(1073873151 % 10000, uuid_utils.getInteger(request_headers).value() % 10000); } TEST(UUIDRequestIDExtensionTest, RequestIDModDistribution) { Random::RandomGeneratorImpl random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); Http::TestRequestHeaderMapImpl request_headers; const int mod = 100; @@ -246,13 +269,15 @@ TEST(UUIDRequestIDExtensionTest, RequestIDModDistribution) { int interesting_samples = 0; for (int i = 0; i < 500000; ++i) { - std::string uuid = random.uuid(); + // Generate UUIDv7 via uuid_utils.set() instead of random.uuid(). + uuid_utils.set(request_headers, true, false); + std::string uuid = std::string(request_headers.getRequestIdValue()); const char c = uuid[19]; - ASSERT_TRUE(uuid[14] == '4'); // UUID version 4 (random) + ASSERT_TRUE(uuid[0] == 'f'); // Request ID marker + ASSERT_TRUE(uuid[14] == '7'); // UUID version 7 ASSERT_TRUE(c == '8' || c == '9' || c == 'a' || c == 'b'); // UUID variant 1 (RFC4122) - request_headers.setRequestId(uuid); const uint64_t value = uuid_utils.getInteger(request_headers).value() % mod; if (value < required_percentage) { @@ -266,18 +291,25 @@ TEST(UUIDRequestIDExtensionTest, RequestIDModDistribution) { TEST(UUIDRequestIDExtensionTest, DISABLED_benchmark) { Random::RandomGeneratorImpl random; + Event::SimulatedTimeSystem time_system; + UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), + random, time_system); + Http::TestRequestHeaderMapImpl request_headers; for (int i = 0; i < 100000000; ++i) { - random.uuid(); + uuid_utils.set(request_headers, true, false); } } TEST(UUIDRequestIDExtensionTest, SetTraceStatus) { Random::RandomGeneratorImpl random; + Event::SimulatedTimeSystem time_system; UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), - random); + random, time_system); Http::TestRequestHeaderMapImpl request_headers; - request_headers.setRequestId(random.uuid()); + + // Generate UUIDv7 via uuid_utils.set() instead of random.uuid(). + uuid_utils.set(request_headers, true, false); EXPECT_EQ(Tracing::Reason::NotTraceable, uuid_utils.getTraceReason(request_headers)); @@ -301,11 +333,15 @@ TEST(UUIDRequestIDExtensionTest, SetTraceStatus) { TEST(UUIDRequestIDExtensionTest, SetTraceStatusPackingDisabled) { Random::RandomGeneratorImpl random; + Event::SimulatedTimeSystem time_system; envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig config; config.mutable_pack_trace_reason()->set_value(false); - UUIDRequestIDExtension uuid_utils(config, random); + UUIDRequestIDExtension uuid_utils(config, random, time_system); - std::string uuid_with_trace_reason = random.uuid(); + // Generate UUIDv7 and manually set trace reason marker. + Http::TestRequestHeaderMapImpl temp_headers; + uuid_utils.set(temp_headers, true, false); + std::string uuid_with_trace_reason = std::string(temp_headers.getRequestIdValue()); uuid_with_trace_reason[14] = 'b'; // 'b' means TRACE_CLIENT. Http::TestRequestHeaderMapImpl request_headers; @@ -319,6 +355,38 @@ TEST(UUIDRequestIDExtensionTest, SetTraceStatusPackingDisabled) { EXPECT_EQ(uuid_with_trace_reason, request_headers.getRequestIdValue()); } +TEST(UUIDRequestIDExtensionTest, GenerateUuidV7Format) { + testing::NiceMock random; + Event::SimulatedTimeSystem time_system; + UUIDRequestIDExtension uuid_utils( + envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(), random, time_system); + + Http::TestRequestHeaderMapImpl request_headers; + + EXPECT_CALL(random, random()).Times(2).WillRepeatedly(Return(0x123456789ABCDEFULL)); + uuid_utils.set(request_headers, true, false); + + std::string uuid = std::string(request_headers.getRequestIdValue()); + + // Verify UUID length. + EXPECT_EQ(36, uuid.length()); + + // Verify marker: request_id starts with 'f'. + EXPECT_EQ('f', uuid[0]); + + // Verify version: position 14 = '7' (UUIDv7). + EXPECT_EQ('7', uuid[14]); + + // Verify variant: position 19 = '8', '9', 'a', or 'b' (RFC 4122). + EXPECT_TRUE(uuid[19] == '8' || uuid[19] == '9' || uuid[19] == 'a' || uuid[19] == 'b'); + + // Verify UUID format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + EXPECT_EQ('-', uuid[8]); + EXPECT_EQ('-', uuid[13]); + EXPECT_EQ('-', uuid[18]); + EXPECT_EQ('-', uuid[23]); +} + } // namespace RequestId } // namespace Extensions } // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc index 628a73283724c..b22b7be9c5d5a 100644 --- a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc +++ b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc @@ -252,18 +252,20 @@ TEST_F(OpenTelemetryDriverTest, GenerateSpanContextWithoutHeadersTest) { Tracing::TestTraceContextImpl request_headers{ {":authority", "test.com"}, {":path", "/"}, {":method", "GET"}}; - // Mock the random call for generating trace and span IDs so we can check it later. - const uint64_t trace_id_high = 1; - const uint64_t trace_id_low = 2; + // Fix timestamp for deterministic UUIDv7 trace_id generation. + time_system_.setSystemTime(std::chrono::milliseconds(0)); + + // Mock the random calls for generating trace_id (UUIDv7: rand_a, rand_b) and span_id. + const uint64_t rand_a = 1; + const uint64_t rand_b = 2; const uint64_t new_span_id = 3; NiceMock& mock_random_generator_ = context_.server_factory_context_.api_.random_; - // The tracer should generate three random numbers for the trace high, trace low, and span id. { InSequence s; - EXPECT_CALL(mock_random_generator_, random()).WillOnce(Return(trace_id_high)); - EXPECT_CALL(mock_random_generator_, random()).WillOnce(Return(trace_id_low)); + EXPECT_CALL(mock_random_generator_, random()).WillOnce(Return(rand_a)); + EXPECT_CALL(mock_random_generator_, random()).WillOnce(Return(rand_b)); EXPECT_CALL(mock_random_generator_, random()).WillOnce(Return(new_span_id)); } @@ -278,8 +280,9 @@ TEST_F(OpenTelemetryDriverTest, GenerateSpanContextWithoutHeadersTest) { // Ends in 01 because span should be sampled. See // https://w3c.github.io/trace-context/#trace-flags. + // trace_id is UUIDv7: timestamp(0) + version(7) + rand_a(1) | variant(2) + rand_b(2) EXPECT_EQ(sampled_entry.has_value(), true); - EXPECT_EQ(sampled_entry.value(), "00-00000000000000010000000000000002-0000000000000003-01"); + EXPECT_EQ(sampled_entry.value(), "00-00000000000070018000000000000002-0000000000000003-01"); } // Verifies a span it not created when an invalid traceparent header is received @@ -606,6 +609,8 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanWithAttributes) { setupValidDriver(); Tracing::TestTraceContextImpl request_headers{ {":authority", "test.com"}, {":path", "/"}, {":method", "GET"}}; + // Fix timestamp for deterministic UUIDv7 trace_id generation. + time_system_.setSystemTime(std::chrono::milliseconds(0)); NiceMock& mock_random_generator_ = context_.server_factory_context_.api_.random_; int64_t generated_int = 1; @@ -660,12 +665,11 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanWithAttributes) { TestUtility::loadFromYaml(fmt::format(request_yaml, envoy_version, timestamp_ns, timestamp_ns), request_proto); - std::string generated_int_hex = Hex::uint64ToHex(generated_int); auto* expected_span = request_proto.mutable_resource_spans(0)->mutable_scope_spans(0)->mutable_spans(0); - expected_span->set_trace_id( - absl::HexStringToBytes(absl::StrCat(generated_int_hex, generated_int_hex))); - expected_span->set_span_id(absl::HexStringToBytes(absl::StrCat(generated_int_hex))); + // UUIDv7 trace_id: timestamp(0) + version(7) + rand_a(1) | variant(2) + rand_b(1) + expected_span->set_trace_id(absl::HexStringToBytes("00000000000070018000000000000001")); + expected_span->set_span_id(absl::HexStringToBytes(Hex::uint64ToHex(generated_int))); EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.opentelemetry.min_flush_spans", 5U)) .Times(1) @@ -682,6 +686,8 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanWithAttributesAndStatus) { setupValidDriver(); Tracing::TestTraceContextImpl request_headers{ {":authority", "test.com"}, {":path", "/"}, {":method", "GET"}}; + // Fix timestamp for deterministic UUIDv7 trace_id generation. + time_system_.setSystemTime(std::chrono::milliseconds(0)); NiceMock& mock_random_generator_ = context_.server_factory_context_.api_.random_; int64_t generated_int = 1; @@ -742,12 +748,11 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanWithAttributesAndStatus) { TestUtility::loadFromYaml(fmt::format(request_yaml, envoy_version, timestamp_ns, timestamp_ns), request_proto); - std::string generated_int_hex = Hex::uint64ToHex(generated_int); auto* expected_span = request_proto.mutable_resource_spans(0)->mutable_scope_spans(0)->mutable_spans(0); - expected_span->set_trace_id( - absl::HexStringToBytes(absl::StrCat(generated_int_hex, generated_int_hex))); - expected_span->set_span_id(absl::HexStringToBytes(absl::StrCat(generated_int_hex))); + // UUIDv7 trace_id: timestamp(0) + version(7) + rand_a(1) | variant(2) + rand_b(1) + expected_span->set_trace_id(absl::HexStringToBytes("00000000000070018000000000000001")); + expected_span->set_span_id(absl::HexStringToBytes(Hex::uint64ToHex(generated_int))); EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.opentelemetry.min_flush_spans", 5U)) .Times(1) @@ -764,6 +769,8 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPGRPCSpanWithAttributesAndStatus) { setupValidDriver(); Tracing::TestTraceContextImpl request_headers{ {":authority", "test.com"}, {":path", "/"}, {":method", "GET"}}; + // Fix timestamp for deterministic UUIDv7 trace_id generation. + time_system_.setSystemTime(std::chrono::milliseconds(0)); NiceMock& mock_random_generator_ = context_.server_factory_context_.api_.random_; int64_t generated_int = 1; @@ -832,12 +839,11 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPGRPCSpanWithAttributesAndStatus) { TestUtility::loadFromYaml(fmt::format(request_yaml, envoy_version, timestamp_ns, timestamp_ns), request_proto); - std::string generated_int_hex = Hex::uint64ToHex(generated_int); auto* expected_span = request_proto.mutable_resource_spans(0)->mutable_scope_spans(0)->mutable_spans(0); - expected_span->set_trace_id( - absl::HexStringToBytes(absl::StrCat(generated_int_hex, generated_int_hex))); - expected_span->set_span_id(absl::HexStringToBytes(absl::StrCat(generated_int_hex))); + // UUIDv7 trace_id: timestamp(0) + version(7) + rand_a(1) | variant(2) + rand_b(1) + expected_span->set_trace_id(absl::HexStringToBytes("00000000000070018000000000000001")); + expected_span->set_span_id(absl::HexStringToBytes(Hex::uint64ToHex(generated_int))); EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.opentelemetry.min_flush_spans", 5U)) .Times(1) @@ -947,6 +953,8 @@ TEST_F(OpenTelemetryDriverTest, ExportSpanWithCustomServiceName) { Tracing::TestTraceContextImpl request_headers{ {":authority", "test.com"}, {":path", "/"}, {":method", "GET"}}; + // Fix timestamp for deterministic UUIDv7 trace_id generation. + time_system_.setSystemTime(std::chrono::milliseconds(0)); NiceMock& mock_random_generator_ = context_.server_factory_context_.api_.random_; int64_t generated_int = 1; @@ -986,12 +994,11 @@ TEST_F(OpenTelemetryDriverTest, ExportSpanWithCustomServiceName) { TestUtility::loadFromYaml(fmt::format(request_yaml, envoy_version, timestamp_ns, timestamp_ns), request_proto); - std::string generated_int_hex = Hex::uint64ToHex(generated_int); auto* expected_span = request_proto.mutable_resource_spans(0)->mutable_scope_spans(0)->mutable_spans(0); - expected_span->set_trace_id( - absl::HexStringToBytes(absl::StrCat(generated_int_hex, generated_int_hex))); - expected_span->set_span_id(absl::HexStringToBytes(absl::StrCat(generated_int_hex))); + // UUIDv7 trace_id: timestamp(0) + version(7) + rand_a(1) | variant(2) + rand_b(1) + expected_span->set_trace_id(absl::HexStringToBytes("00000000000070018000000000000001")); + expected_span->set_span_id(absl::HexStringToBytes(Hex::uint64ToHex(generated_int))); EXPECT_CALL(runtime_.snapshot_, getInteger("tracing.opentelemetry.min_flush_spans", 5U)) .Times(1) diff --git a/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc b/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc index 0fe665b9c9c80..5f5efbeddf1bd 100644 --- a/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc @@ -59,9 +59,12 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, AlwaysOnSamplerIntegrationTest, // Sends a request with traceparent and tracestate header. TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentAndTracestate) { - Http::TestRequestHeaderMapImpl request_headers{ - {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, - {":authority", "host"}, {"tracestate", "key=value"}, {"traceparent", TRACEPARENT_VALUE}}; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-sendbird-tracestate", "key=value"}, + {"x-sendbird-traceparent", TRACEPARENT_VALUE}}; auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); @@ -71,14 +74,14 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentAndTracestate) { // traceparent should be set: traceid should be re-used, span id should be different absl::string_view traceparent_value = upstream_request_->headers() - .get(Http::LowerCaseString("traceparent"))[0] + .get(Http::LowerCaseString("x-sendbird-traceparent"))[0] ->value() .getStringView(); EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); // tracestate should be forwarded absl::string_view tracestate_value = upstream_request_->headers() - .get(Http::LowerCaseString("tracestate"))[0] + .get(Http::LowerCaseString("x-sendbird-tracestate"))[0] ->value() .getStringView(); EXPECT_EQ("key=value", tracestate_value); @@ -90,7 +93,7 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentOnly) { {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}, - {"traceparent", TRACEPARENT_VALUE}}; + {"x-sendbird-traceparent", TRACEPARENT_VALUE}}; auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); ASSERT_TRUE(response->waitForEndStream()); @@ -99,14 +102,14 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentOnly) { // traceparent should be set: traceid should be re-used, span id should be different absl::string_view traceparent_value = upstream_request_->headers() - .get(Http::LowerCaseString("traceparent"))[0] + .get(Http::LowerCaseString("x-sendbird-traceparent"))[0] ->value() .getStringView(); EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); // OTLP tracer adds an empty tracestate absl::string_view tracestate_value = upstream_request_->headers() - .get(Http::LowerCaseString("tracestate"))[0] + .get(Http::LowerCaseString("x-sendbird-tracestate"))[0] ->value() .getStringView(); EXPECT_EQ("", tracestate_value); @@ -125,11 +128,13 @@ TEST_P(AlwaysOnSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) { // traceparent will be added, trace_id and span_id will be generated, so there is nothing we can // assert - EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("traceparent")).size(), + EXPECT_EQ(upstream_request_->headers() + .get(::Envoy::Http::LowerCaseString("x-sendbird-traceparent")) + .size(), 1); // OTLP tracer adds an empty tracestate absl::string_view tracestate_value = upstream_request_->headers() - .get(Http::LowerCaseString("tracestate"))[0] + .get(Http::LowerCaseString("x-sendbird-tracestate"))[0] ->value() .getStringView(); EXPECT_EQ("", tracestate_value); diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc index a3887eaf5ab4e..c2dee1f01ffad 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc @@ -61,9 +61,12 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, DynatraceSamplerIntegrationTest, // Sends a request with traceparent and tracestate header. TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) { // tracestate does not contain a Dynatrace tag - Http::TestRequestHeaderMapImpl request_headers{ - {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, - {":authority", "host"}, {"tracestate", "key=value"}, {"traceparent", TRACEPARENT_VALUE}}; + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-sendbird-tracestate", "key=value"}, + {"x-sendbird-traceparent", TRACEPARENT_VALUE}}; auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); @@ -73,14 +76,14 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) { // traceparent should be set: traceid should be re-used, span id should be different absl::string_view traceparent_value = upstream_request_->headers() - .get(Http::LowerCaseString("traceparent"))[0] + .get(Http::LowerCaseString("x-sendbird-traceparent"))[0] ->value() .getStringView(); EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); // Dynatrace tracestate should be added to existing tracestate absl::string_view tracestate_value = upstream_request_->headers() - .get(Http::LowerCaseString("tracestate"))[0] + .get(Http::LowerCaseString("x-sendbird-tracestate"))[0] ->value() .getStringView(); // use StartsWith because path-info (last element in trace state) contains a random value @@ -96,7 +99,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) { {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}, - {"traceparent", TRACEPARENT_VALUE}}; + {"x-sendbird-traceparent", TRACEPARENT_VALUE}}; auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); ASSERT_TRUE(response->waitForEndStream()); @@ -105,14 +108,14 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) { // traceparent should be set: traceid should be re-used, span id should be different absl::string_view traceparent_value = upstream_request_->headers() - .get(Http::LowerCaseString("traceparent"))[0] + .get(Http::LowerCaseString("x-sendbird-traceparent"))[0] ->value() .getStringView(); EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); // Dynatrace tag should be added to tracestate absl::string_view tracestate_value = upstream_request_->headers() - .get(Http::LowerCaseString("tracestate"))[0] + .get(Http::LowerCaseString("x-sendbird-tracestate"))[0] ->value() .getStringView(); // use StartsWith because path-info (last element in trace state contains a random value) @@ -133,11 +136,13 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) { // traceparent will be added, trace_id and span_id will be generated, so there is nothing we can // assert - EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("traceparent")).size(), + EXPECT_EQ(upstream_request_->headers() + .get(::Envoy::Http::LowerCaseString("x-sendbird-traceparent")) + .size(), 1); // Dynatrace tag should be added to tracestate absl::string_view tracestate_value = upstream_request_->headers() - .get(Http::LowerCaseString("tracestate"))[0] + .get(Http::LowerCaseString("x-sendbird-tracestate"))[0] ->value() .getStringView(); EXPECT_TRUE(absl::StartsWith(tracestate_value, "5b3f9fed-980df25c@dt=fw4;0;0;0;0;0;0;")) diff --git a/test/extensions/tracers/opentelemetry/span_context_extractor_test.cc b/test/extensions/tracers/opentelemetry/span_context_extractor_test.cc index 5515d760d6bd9..31aacae6b2557 100644 --- a/test/extensions/tracers/opentelemetry/span_context_extractor_test.cc +++ b/test/extensions/tracers/opentelemetry/span_context_extractor_test.cc @@ -23,7 +23,8 @@ constexpr absl::string_view trace_flags = "01"; TEST(SpanContextExtractorTest, ExtractSpanContext) { Tracing::TestTraceContextImpl request_headers{ - {"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; + {"x-sendbird-traceparent", + fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -38,7 +39,7 @@ TEST(SpanContextExtractorTest, ExtractSpanContext) { TEST(SpanContextExtractorTest, ExtractSpanContextNotSampled) { const std::string trace_flags_unsampled{"00"}; Tracing::TestTraceContextImpl request_headers{ - {"traceparent", + {"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags_unsampled)}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -62,7 +63,8 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithoutHeader) { TEST(SpanContextExtractorTest, ThrowsExceptionWithTooLongHeader) { Tracing::TestTraceContextImpl request_headers{ - {"traceparent", fmt::format("000{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; + {"x-sendbird-traceparent", + fmt::format("000{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -73,7 +75,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithTooLongHeader) { TEST(SpanContextExtractorTest, ThrowsExceptionWithTooShortHeader) { Tracing::TestTraceContextImpl request_headers{ - {"traceparent", fmt::format("{}-{}-{}", trace_id, parent_id, trace_flags)}}; + {"x-sendbird-traceparent", fmt::format("{}-{}-{}", trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -84,7 +86,8 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithTooShortHeader) { TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidHyphenation) { Tracing::TestTraceContextImpl request_headers{ - {"traceparent", fmt::format("{}{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; + {"x-sendbird-traceparent", + fmt::format("{}{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -97,7 +100,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidSizes) { const std::string invalid_version{"0"}; const std::string invalid_trace_flags{"001"}; Tracing::TestTraceContextImpl request_headers{ - {"traceparent", + {"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", invalid_version, trace_id, parent_id, invalid_trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); @@ -110,7 +113,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidSizes) { TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidHex) { const std::string invalid_version{"ZZ"}; Tracing::TestTraceContextImpl request_headers{ - {"traceparent", + {"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", invalid_version, trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); @@ -123,7 +126,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithInvalidHex) { TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroTraceId) { const std::string invalid_trace_id{"00000000000000000000000000000000"}; Tracing::TestTraceContextImpl request_headers{ - {"traceparent", + {"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, invalid_trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); @@ -136,7 +139,7 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroTraceId) { TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroParentId) { const std::string invalid_parent_id{"0000000000000000"}; Tracing::TestTraceContextImpl request_headers{ - {"traceparent", + {"x-sendbird-traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, invalid_parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); @@ -148,7 +151,8 @@ TEST(SpanContextExtractorTest, ThrowsExceptionWithAllZeroParentId) { TEST(SpanContextExtractorTest, ExtractSpanContextWithEmptyTracestate) { Tracing::TestTraceContextImpl request_headers{ - {"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; + {"x-sendbird-traceparent", + fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -158,8 +162,9 @@ TEST(SpanContextExtractorTest, ExtractSpanContextWithEmptyTracestate) { TEST(SpanContextExtractorTest, ExtractSpanContextWithTracestate) { Tracing::TestTraceContextImpl request_headers{ - {"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}, - {"tracestate", "sample-tracestate"}}; + {"x-sendbird-traceparent", + fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}, + {"x-sendbird-tracestate", "sample-tracestate"}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -168,7 +173,7 @@ TEST(SpanContextExtractorTest, ExtractSpanContextWithTracestate) { } TEST(SpanContextExtractorTest, IgnoreTracestateWithoutTraceparent) { - Tracing::TestTraceContextImpl request_headers{{"tracestate", "sample-tracestate"}}; + Tracing::TestTraceContextImpl request_headers{{"x-sendbird-tracestate", "sample-tracestate"}}; SpanContextExtractor span_context_extractor(request_headers); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); @@ -178,9 +183,10 @@ TEST(SpanContextExtractorTest, IgnoreTracestateWithoutTraceparent) { TEST(SpanContextExtractorTest, ExtractSpanContextWithMultipleTracestateEntries) { Http::TestRequestHeaderMapImpl request_headers{ - {"traceparent", fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}, - {"tracestate", "sample-tracestate"}, - {"tracestate", "sample-tracestate-2"}}; + {"x-sendbird-traceparent", + fmt::format("{}-{}-{}-{}", version, trace_id, parent_id, trace_flags)}, + {"x-sendbird-tracestate", "sample-tracestate"}, + {"x-sendbird-tracestate", "sample-tracestate-2"}}; Tracing::HttpTraceContext trace_context(request_headers); SpanContextExtractor span_context_extractor(trace_context); absl::StatusOr span_context = span_context_extractor.extractSpanContext(); diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index 3882cd9665c07..9b325748001ad 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -94,11 +94,13 @@ TEST_P(QuicHttpIntegrationTest, RuntimeEnableDraft29) { } TEST_P(QuicHttpIntegrationTest, CertCompressionEnabled) { + config_helper_.addRuntimeOverride("envoy.reloadable_features.tls_support_certificate_compression", + "true"); initialize(); EXPECT_LOG_CONTAINS_ALL_OF( - Envoy::ExpectedLogMessages( - {{"trace", "Cert compression successful"}, {"trace", "Cert decompression successful"}}), + Envoy::ExpectedLogMessages({{"trace", "Cert brotli compression successful"}, + {"trace", "Cert brotli decompression successful"}}), { testRouterHeaderOnlyRequestAndResponse(); }); } diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index b279330f19d17..df26bb2b767a2 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -393,6 +393,7 @@ Prereq QDCOUNT QPACK QUIC +QUICHE QoS qat qatzip @@ -480,6 +481,7 @@ TE TFO TID TLS +TLSEXT TLSv TLV TMPDIR