diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index 95b48583bb3..129114ed344 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -2007,6 +2007,94 @@ Origin Server Connect Attempts the connection. Useful when the origin supports keep-alive, removing the time needed to set up a new connection from the next request at the expense of added (inactive) connections. +.. ts:cv:: CONFIG proxy.config.http.per_server.connection.metric_enabled INT 0 + :reloadable: + :overridable: + + Enable per upstream server connection metrics. These metrics are dynamically named, one set per + upstream server group, so the number of them scales with the number of distinct upstream servers + seen. See :ref:`per-server-connection-metrics`. + + ===== ====================================================================================== + Value Effect + ===== ====================================================================================== + ``0`` No per server connection metrics. + ``1`` Per server connection metrics are collected for each upstream server group. + ===== ====================================================================================== + + What is published from them is controlled separately by + :ts:cv:`proxy.config.http.per_server.connection.metric_aggregate`, which by default publishes the + per group metrics themselves. + + Because this is overridable, metrics can be enabled for the upstreams of interest and left off + for the rest, for example with :ref:`admin-plugins-conf-remap` on a specific mapping. + + The value is applied when a connection group is created. Where two mappings that disagree about + this setting resolve to the same group -- that is, the same key under + :ts:cv:`proxy.config.http.per_server.connection.match` -- the transaction that creates the group + determines its metrics, and later transactions do not change them. A group is discarded once its + connection count reaches zero, so *raising* the level of publication is picked up the next time + that upstream is reopened: enabling metrics, or enabling the aggregates, takes effect as upstreams + reconnect. Lowering it does not. Metrics are never retired once published, so disabling this + setting, or switching + :ts:cv:`proxy.config.http.per_server.connection.metric_aggregate` to ``2``, leaves the names that + are already published in place, frozen at their last sampled value, until |TS| is restarted. This + affects only which metrics exist; enforcement of + :ts:cv:`proxy.config.http.per_server.connection.max` uses the group's own connection count and is + unaffected. + +.. ts:cv:: CONFIG proxy.config.http.per_server.connection.metric_aggregate INT 0 + :reloadable: + :overridable: + + Control what is published from the per server connection metrics enabled by + :ts:cv:`proxy.config.http.per_server.connection.metric_enabled`. Has no effect when that setting + is ``0``. + + A per hostname aggregate sums a counter across every group belonging to that hostname that has + aggregation enabled, and exists only for + :ts:cv:`match type ` ``both``, since that is the + only match type whose group key carries the hostname. See :ref:`per-server-connection-metrics`. + + ===== ====================================================================================== + Value Effect + ===== ====================================================================================== + ``0`` No aggregates. The per group metrics are published under their own names. + ``1`` Publish the per hostname aggregates and the per group metrics. + ``2`` Publish only the per hostname aggregates. The per group metrics from which they are + computed are collected but not published, which keeps the number of published metrics + proportional to hostnames rather than to groups. + ===== ====================================================================================== + + With value ``2``, a group that has no aggregate to belong to -- any match type other than + ``both`` -- has its per group metrics published anyway, since otherwise nothing at all would be + reported for it. + + Values ``0`` and ``1`` can produce a very large number of metrics when the match type includes the + address or port, since there is then one set per address and port rather than one per hostname. + + Like :ts:cv:`proxy.config.http.per_server.connection.metric_enabled`, this is applied when a + connection group is created, with the same consequence for mappings that disagree and resolve to + the same group. A group joins its hostname's aggregate only if the mapping that first opened that + upstream had aggregation enabled, so mappings that disagree for one hostname produce an aggregate + that covers only part of it. + + The reload is one-directional for the same reason given under + :ts:cv:`proxy.config.http.per_server.connection.metric_enabled`. Raising the value takes effect + as upstreams reconnect, but moving to ``2`` does not hide per group metrics that are already + published, and moving from ``1`` to ``0`` does not stop the hostname aggregates from publishing. + Reducing the number of published metrics therefore requires a restart, which matters most for + ``2``, the value chosen specifically to bound that number. + +.. ts:cv:: CONFIG proxy.config.http.per_server.connection.metric_prefix STRING NULL + :reloadable: + + An optional prefix inserted into the per server connection metric names, between the fixed + ``proxy.process.http.per_server..`` portion of the name and the upstream server group + or hostname. Useful to distinguish metrics from separate + :ts:cv:`match ` configurations sharing the same + upstream. See :ref:`per-server-connection-metrics`. + .. ts:cv:: CONFIG proxy.config.http.connect_attempts_rr_retries INT 3 :reloadable: :overridable: diff --git a/doc/admin-guide/monitoring/statistics/core/http-connection.en.rst b/doc/admin-guide/monitoring/statistics/core/http-connection.en.rst index 7b3b23940f0..47075ce6fa9 100644 --- a/doc/admin-guide/monitoring/statistics/core/http-connection.en.rst +++ b/doc/admin-guide/monitoring/statistics/core/http-connection.en.rst @@ -201,6 +201,77 @@ HTTP Connection Current number of TCP connections for tunnels where the far end is the server, except for those counted by ``proxy.process.tunnel.current_server_connections_tls`` +.. _per-server-connection-metrics: + +Per Server Connection Metrics +----------------------------- + +Unlike the metrics above these do not have fixed names. They are created dynamically, one set per +upstream server group as defined by :ts:cv:`proxy.config.http.per_server.connection.match`, and, +when the match type is ``both``, one aggregate set per hostname. Whether they are collected at all is +controlled by :ts:cv:`proxy.config.http.per_server.connection.metric_enabled`, and which of them are +published by :ts:cv:`proxy.config.http.per_server.connection.metric_aggregate`. An optional +:ts:cv:`proxy.config.http.per_server.connection.metric_prefix` can be inserted into the names. + +Per group names are ``proxy.process.http.per_server..``, where ```` depends on +the match type: an IP address, an ``address:port`` pair, a hostname, or, for ``both``, +``.``. Per hostname names are +``proxy.process.http.per_server..``. Aggregates exist only for match type +``both``, because that is the only match type whose group key carries the hostname. An ``ip`` or +``port`` group is keyed on the address alone and is shared by every hostname that resolves to it, so +there is no single hostname to aggregate it under. For match type ``host`` the group name is already +the bare hostname, so an aggregate would carry the same name as the single group it summarises. + +For a group, ```` is one of: + +current_connection + Gauge. The number of connections currently open to the group. + +total_connection + Counter. The total number of connections ever opened to the group. Never decreases. + +blocked_connection + Counter. The total number of connection attempts to the group blocked by + :ts:cv:`proxy.config.http.per_server.connection.max`. Never decreases. + +For a hostname aggregate, ```` is one of those three, each summed across the groups of that +hostname which have aggregation enabled, plus: + +current_connection_max + Gauge. The largest ``current_connection`` value among the groups of that hostname at the moment + of sampling, so the maximum rather than the sum of the groups' current counts. This is useful + because :ts:cv:`proxy.config.http.per_server.connection.max` is enforced per group rather than + per hostname, so the busiest group is what determines whether connections are about to be + blocked. Like ``current_connection`` it rises and falls with traffic and is not a high-water + mark. There is no per group ``current_connection_max``; it exists only as a hostname aggregate. + +Because :ts:cv:`proxy.config.http.per_server.connection.metric_aggregate` is overridable, a group +joins its hostname's aggregate only if the mapping that first opened that upstream had aggregation +enabled. Mappings that disagree for one hostname therefore produce an aggregate over part of it: the +sums cover a subset of the groups and ``current_connection_max`` takes its maximum over that same +subset, with nothing in the metric to indicate it. Keeping the setting uniform across the mappings +for a hostname avoids this. + +Because :ts:cv:`proxy.config.http.per_server.connection.match` is also overridable, one hostname can +use match type ``host`` on one mapping and ``both`` on another. The ``host`` group and the hostname +aggregate are then published under the same name and merged into a single metric that carries both, +so a hostname should use one match type throughout. + +Every published per server metric is recomputed periodically, currently every 5 seconds, rather than +on every connection event, so a reader sees a value up to that interval old. This is true of the +hostname aggregates and of the published per group metrics alike: those are +mirrored from the internal ones by the same periodic mechanism, not written as connections open and +close. It applies to ``current_connection_max`` too, which reports the maximum across groups as of +the last sample rather than a running peak. To obtain the peak over a longer window, compute a +maximum over time from this gauge in the monitoring system. + +At :ts:cv:`metric_aggregate ` value ``2`` +the per group metrics still exist internally, since the aggregates are computed from them, but are not +published. They can be listed with ``traffic_ctl metric match per_server --include-hidden``, which +reads them directly and so is not subject to the sampling delay above. That visibility is intended +for debugging and is not a stable interface: the existence, granularity and naming of the per group +metrics may change independently of the published aggregates. + HTTP/2 ------ diff --git a/doc/admin-guide/plugins/lua.en.rst b/doc/admin-guide/plugins/lua.en.rst index c5e7f784ce3..fb99e51e399 100644 --- a/doc/admin-guide/plugins/lua.en.rst +++ b/doc/admin-guide/plugins/lua.en.rst @@ -4746,6 +4746,8 @@ Http config constants TS_LUA_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH + TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_ENABLED + TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_AGGREGATE TS_LUA_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES TS_LUA_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES_DOWN_SERVER TS_LUA_CONFIG_HTTP_CONNECT_DOWN_POLICY diff --git a/doc/admin-guide/plugins/prefetch.en.rst b/doc/admin-guide/plugins/prefetch.en.rst index 62efb348c95..e9dd28c0ce1 100644 --- a/doc/admin-guide/plugins/prefetch.en.rst +++ b/doc/admin-guide/plugins/prefetch.en.rst @@ -34,6 +34,15 @@ On every **incoming** URL request, the plugin can decide to pre-fetch the **next object** or more objects based on the common URL path pattern and a pre-defined pre-fetch policy. +.. note:: + + The plugin only prefetches on requests that go through a cache lookup. + On transactions where ATS skips the cache lookup -- non-cacheable + methods, or caching turned off (:ts:cv:`proxy.config.http.cache.http` + set to ``0``) -- the plugin does not prefetch. Issuing prefetches into + a cache that will not be consulted would do real work for no cache + benefit. + Currently, most HLS video urls follow a predictable pattern, with most URLs containing a segment number. Since the segments are ~10s of content, the normal usage pattern is to fetch the incremental segment every few seconds. The CDN diff --git a/doc/admin-guide/plugins/rate_limit.en.rst b/doc/admin-guide/plugins/rate_limit.en.rst index 7f3d5ccf1aa..c96f6a3684b 100644 --- a/doc/admin-guide/plugins/rate_limit.en.rst +++ b/doc/admin-guide/plugins/rate_limit.en.rst @@ -92,8 +92,11 @@ are available: .. option:: --maxage - An optional ``max-age`` for how long a transaction can sit in the delay queue. - The value (default 0) is the age in seconds. + An optional maximum age for how long a transaction can sit in the delay queue. + The value (default 0) is the age in **milliseconds**. + + Note that the equivalent YAML setting, ``max_age`` under a ``queue`` node, is in + seconds. .. option:: --prefix @@ -149,7 +152,7 @@ and nodes are documented below. rate: 200 queue: size: 1000 - max-age: 30 + max_age: 30 metrics: tag: example.com prefix: ddos @@ -163,11 +166,11 @@ and nodes are documented below. buckets: 10 size: 15 percentage: 90 - max-age: 300 + max_age: 300 perma-block: limit: 100 threshold: 1 - max-age: 1800 + max_age: 1800 lists: - name: internal cidr: @@ -212,9 +215,9 @@ For the top level `selector` node, the following options are available: how many queued transactions we will allow. When this threshold is reached, all additional connections are immediately errored out in the TLS handshake. - The queue option can include a `size` and a `max-age` option. The size is - default to ``UINT_MAX``, which is essentially unlimited. The max-age is - default to ``0``, which means no age limit. + The queue option can include a `size` and a `max_age` option. The size defaults + to ``UINT_MAX``, which is essentially unlimited. The max_age is in seconds and + defaults to ``0``, which means no age limit. No queue is enabled without this configuration directive, but it can also be disabled explicitly if the size is set to ``0``. @@ -268,7 +271,7 @@ and the following options: This is the minimum percentage of the ``limit`` that the pressure must be at, before we start blocking IPs. The default is ``0.9`` which means ``90%`` of the limit. -.. option:: max-age +.. option:: max_age This is used for aging out entries out of the LRU, the default is ``0`` which means no aging happens. Even with no aging, entries will eventually fall out of buckets @@ -289,7 +292,7 @@ blocked for a long time. The configuration for this bucket is: This option specifies from which bucket an IP is allowed to move from into the perma block bucket. A good value here is likely ``0`` or ``1``, which is very conservative. -.. option:: max-age +.. option:: max_age Like above, but only applies to the long term (`perma-block`) bucket. Default is ``0``, which means no aging to this bucket is applied. diff --git a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst index 43d98b40ad7..11675c09b0f 100644 --- a/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst +++ b/doc/developer-guide/api/functions/TSHttpOverridableConfig.en.rst @@ -155,6 +155,8 @@ TSOverridableConfigKey Value Confi :enumerator:`TS_CONFIG_HTTP_PER_PARENT_CONNECT_ATTEMPTS` :ts:cv:`proxy.config.http.parent_proxy.per_parent_connect_attempts` :enumerator:`TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH` :ts:cv:`proxy.config.http.per_server.connection.match` :enumerator:`TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX` :ts:cv:`proxy.config.http.per_server.connection.max` +:enumerator:`TS_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_AGGREGATE` :ts:cv:`proxy.config.http.per_server.connection.metric_aggregate` +:enumerator:`TS_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_ENABLED` :ts:cv:`proxy.config.http.per_server.connection.metric_enabled` :enumerator:`TS_CONFIG_HTTP_POST_CHECK_CONTENT_LENGTH_ENABLED` :ts:cv:`proxy.config.http.post.check.content_length.enabled` :enumerator:`TS_CONFIG_HTTP_REDIRECT_USE_ORIG_CACHE_KEY` :ts:cv:`proxy.config.http.redirect_use_orig_cache_key` :enumerator:`TS_CONFIG_HTTP_REQUEST_BUFFER_ENABLED` :ts:cv:`proxy.config.http.request_buffer_enabled` diff --git a/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst b/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst index 4ce7c1712d6..381b6f19e4f 100644 --- a/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst +++ b/doc/developer-guide/api/types/TSOverridableConfigKey.en.rst @@ -150,6 +150,8 @@ Enumeration Members .. enumerator:: TS_CONFIG_HTTP_ALLOW_HALF_OPEN .. enumerator:: TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX .. enumerator:: TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH +.. enumerator:: TS_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_ENABLED +.. enumerator:: TS_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_AGGREGATE .. enumerator:: TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_POLICY .. enumerator:: TS_CONFIG_SSL_CLIENT_VERIFY_SERVER_PROPERTIES .. enumerator:: TS_CONFIG_SSL_CLIENT_SNI_POLICY diff --git a/include/iocore/net/ConnectionTracker.h b/include/iocore/net/ConnectionTracker.h index ae3691fdbe2..e18f6aa66c6 100644 --- a/include/iocore/net/ConnectionTracker.h +++ b/include/iocore/net/ConnectionTracker.h @@ -75,11 +75,42 @@ class ConnectionTracker /// String equivalents for @c MatchType. static const std::array(MATCH_BOTH) + 1> MATCH_TYPE_NAME; + /** Whether, and how, the per hostname aggregate metrics are published. + * + * This is independent of @c TxnConfig::metric_enabled, which decides only whether per server + * metrics exist for a group at all. The per group metrics are always created in the hidden metric + * store; what varies here is what gets published from them: + * - @c AGGREGATE_NONE: no aggregate. The per group metrics are published under their own names. + * This is the default and matches the behavior of releases that had no aggregate support. + * - @c AGGREGATE_GROUP: the per hostname aggregates are published, and so are the per group + * metrics they are computed from. + * - @c AGGREGATE_ONLY: the per hostname aggregates are published and the per group metrics stay + * hidden, which keeps the published metric count proportional to hostnames rather than to + * groups. Where a group has no aggregate to belong to -- see @c Group::host_metric_name, which + * only yields a name for match type @c MATCH_BOTH -- the per group metrics are published + * anyway, since otherwise nothing at all would be reported for that group. + * + * Keeping the per group metrics in the hidden store in every case means changing this at runtime + * is only a change of what is registered for publication, with no metric to migrate between the + * two stores. + * + * The records layer validates and clamps this to 0..2. A plugin setting the overridable config + * directly is not clamped, see @c METRIC_AGGREGATE_CONV; any other value behaves as + * @c AGGREGATE_GROUP, publishing both the aggregate and the per group metrics. + */ + enum MetricAggregate : int { + AGGREGATE_NONE = 0, ///< No hostname aggregate; the per group metrics are published. + AGGREGATE_GROUP = 1, ///< Hostname aggregates published, along with the per group metrics. + AGGREGATE_ONLY = 2, ///< Hostname aggregates published, per group metrics kept hidden. + }; + /// Per transaction configuration values. struct TxnConfig { - int server_max{0}; ///< Maximum concurrent server connections. - int server_min{0}; ///< Minimum keepalive server connections. - MatchType server_match{MATCH_IP}; ///< Server match type. + int server_max{0}; ///< Maximum concurrent server connections. + int server_min{0}; ///< Minimum keepalive server connections. + MatchType server_match{MATCH_IP}; ///< Server match type. + int metric_enabled{0}; ///< Whether per server metrics exist for a group. + MetricAggregate metric_aggregate{AGGREGATE_NONE}; ///< What is published, see @c MetricAggregate. }; /** Static configuration values. */ @@ -90,7 +121,6 @@ class ConnectionTracker std::chrono::seconds client_alert_delay{60}; ///< Alert delay in seconds. std::chrono::seconds server_alert_delay{60}; ///< Alert delay in seconds. - bool metric_enabled{false}; ///< Enabling per server metrics. std::string metric_prefix; ///< Per server metric prefix. swoc::IPRangeSet client_exempt_list; ///< The set of IP addresses to not block due client connection counting. mutable ts::bravo::shared_mutex client_exempt_list_mutex; ///< Protects client_exempt_list from concurrent access. @@ -106,6 +136,7 @@ class ConnectionTracker static constexpr std::string_view CONFIG_SERVER_VAR_MATCH{"proxy.config.http.per_server.connection.match"}; static constexpr std::string_view CONFIG_SERVER_VAR_ALERT_DELAY{"proxy.config.http.per_server.connection.alert_delay"}; static constexpr std::string_view CONFIG_SERVER_VAR_METRIC_ENABLED{"proxy.config.http.per_server.connection.metric_enabled"}; + static constexpr std::string_view CONFIG_SERVER_VAR_METRIC_AGGREGATE{"proxy.config.http.per_server.connection.metric_aggregate"}; static constexpr std::string_view CONFIG_SERVER_VAR_METRIC_PREFIX{"proxy.config.http.per_server.connection.metric_prefix"}; /// A record for the outbound connection count. @@ -145,7 +176,8 @@ class ConnectionTracker std::atomic _in_queue{0}; ///< # of connections queued, waiting for a connection. std::atomic _last_alert{0}; ///< Absolute time of the last alert. - // Recording data as metrics + // Recording data as metrics. These are always in the hidden metric store when created; see + // @c MetricAggregate for how they are published. ts::Metrics::Gauge::AtomicType *_count_metric = nullptr; ts::Metrics::Counter::AtomicType *_count_total_metric = nullptr; ts::Metrics::Counter::AtomicType *_blocked_metric = nullptr; @@ -155,8 +187,11 @@ class ConnectionTracker * @param key A populated @c Key structure - values are copied to the @c Group. * @param fqdn The full FQDN. * @param min_keep_alive The minimum number of origin keep alive connections to maintain. + * @param metric_enabled Whether the transaction creating this group wants per server metrics. + * @param metric_aggregate What that transaction wants published, see @c MetricAggregate. */ - Group(DirectionType direction, Key const &key, std::string_view fqdn, int min_keep_alive); + Group(DirectionType direction, Key const &key, std::string_view fqdn, int min_keep_alive, int metric_enabled = 0, + MetricAggregate metric_aggregate = AGGREGATE_NONE); ~Group(); /// Key equality checker. static bool equal(Key const &lhs, Key const &rhs); @@ -171,6 +206,28 @@ class ConnectionTracker std::time_t get_last_alert_epoch_time() const; static std::string metric_name(const Key &key, std::string_view fqdn, std::string metric_prefix); + /** Name of the metric which aggregates a value across all groups of a hostname. + * + * Only @c MATCH_BOTH keys carry both a hostname and an address, so it is the only match type + * whose groups can be gathered by hostname at all. @c MATCH_IP and @c MATCH_PORT key on the + * address alone and one such group is shared by every hostname resolving to it, so there is no + * single hostname to aggregate it under. For @c MATCH_HOST there is exactly one group per + * hostname, so an aggregate would be over a set of one, and @c Group::metric_name already + * returns the FQDN alone for that match type - identical to what this would return, so + * publishing both would collide on one name. + * + * Note that reasoning holds within a single match type. @c TxnConfig::server_match is + * overridable, so one hostname can be @c MATCH_HOST on one mapping and @c MATCH_BOTH on + * another, and then that group's own published name and this aggregate name are the same + * string and are merged into one derived metric. + * + * @param key The group key. + * @param fqdn The full FQDN. + * @param metric_prefix The configured metric prefix. + * @return The metric name, or an empty string if @a key is not @c MATCH_BOTH. + */ + static std::string host_metric_name(const Key &key, std::string_view fqdn, std::string metric_prefix); + /// Release the reference count to this group and remove it from the /// group table if it is no longer referenced. void release(); @@ -347,6 +404,8 @@ class ConnectionTracker static const MgmtConverter MIN_SERVER_CONV; static const MgmtConverter MAX_SERVER_CONV; static const MgmtConverter SERVER_MATCH_CONV; + static const MgmtConverter METRIC_ENABLED_CONV; + static const MgmtConverter METRIC_AGGREGATE_CONV; protected: static GlobalConfig *_global_config; ///< Global configuration data. @@ -433,6 +492,15 @@ ConnectionTracker::Group::metric_name(const Key &key, std::string_view fqdn, std return metric_prefix.empty() ? std::move(metric_name) : metric_prefix + "." + metric_name; } +inline std::string +ConnectionTracker::Group::host_metric_name(const Key &key, std::string_view fqdn, std::string metric_prefix) +{ + if (MATCH_BOTH != key._match_type) { + return {}; // Only MATCH_BOTH keys carry the hostname needed to gather groups under it. + } + return metric_prefix.empty() ? std::string(fqdn) : metric_prefix + "." + std::string(fqdn); +} + inline bool ConnectionTracker::TxnState::is_active() const { @@ -489,9 +557,13 @@ ConnectionTracker::TxnState::clear() inline void ConnectionTracker::TxnState::update_max_count(int count) { - auto cmax = _g->_count_max.load(); - if (count > cmax) { - _g->_count_max.compare_exchange_weak(cmax, count); + auto cmax = _g->_count_max.load(std::memory_order_relaxed); + + while (count > cmax) { + if (_g->_count_max.compare_exchange_weak(cmax, count, std::memory_order_relaxed, std::memory_order_relaxed)) { + break; + } + // cmax was reloaded by the failed exchange; retry if we are still larger. } } diff --git a/include/proxy/http/OverridableConfigDefs.h b/include/proxy/http/OverridableConfigDefs.h index bf00fc3def9..88be7f41d52 100644 --- a/include/proxy/http/OverridableConfigDefs.h +++ b/include/proxy/http/OverridableConfigDefs.h @@ -252,6 +252,8 @@ X(HTTP_NEGATIVE_REVALIDATING_LIST, negative_revalidating_list, "proxy.config.http.negative_revalidating_list", STRING, HttpStatusCodeList_Conv) \ X(HTTP_CACHE_POST_METHOD, cache_post_method, "proxy.config.http.cache.post_method", INT, GENERIC) \ X(HTTP_CACHE_TARGETED_CACHE_CONTROL_HEADERS, targeted_cache_control_headers, "proxy.config.http.cache.targeted_cache_control_headers", STRING, TargetedCacheControlHeaders_Conv) \ - X(SSL_CLIENT_CA_CERT_PATH, ssl_client_ca_cert_path, "proxy.config.ssl.client.CA.cert.path", STRING, NONE) + X(SSL_CLIENT_CA_CERT_PATH, ssl_client_ca_cert_path, "proxy.config.ssl.client.CA.cert.path", STRING, NONE) \ + X(HTTP_PER_SERVER_CONNECTION_METRIC_ENABLED, connection_tracker_config.metric_enabled, ConnectionTracker::CONFIG_SERVER_VAR_METRIC_ENABLED, INT, ConnectionTracker_METRIC_ENABLED_CONV) \ + X(HTTP_PER_SERVER_CONNECTION_METRIC_AGGREGATE, connection_tracker_config.metric_aggregate, ConnectionTracker::CONFIG_SERVER_VAR_METRIC_AGGREGATE, INT, ConnectionTracker_METRIC_AGGREGATE_CONV) // clang-format on diff --git a/include/ts/apidefs.h.in b/include/ts/apidefs.h.in index f34ffdb1a3f..c9e95f05435 100644 --- a/include/ts/apidefs.h.in +++ b/include/ts/apidefs.h.in @@ -918,6 +918,8 @@ enum TSOverridableConfigKey { TS_CONFIG_HTTP_CACHE_POST_METHOD, TS_CONFIG_HTTP_CACHE_TARGETED_CACHE_CONTROL_HEADERS, TS_CONFIG_SSL_CLIENT_CA_CERT_PATH, + TS_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_ENABLED, + TS_CONFIG_HTTP_PER_SERVER_CONNECTION_METRIC_AGGREGATE, TS_CONFIG_LAST_ENTRY, }; diff --git a/plugins/compress/compress.cc b/plugins/compress/compress.cc index 58fd52802fb..3994be9b72e 100644 --- a/plugins/compress/compress.cc +++ b/plugins/compress/compress.cc @@ -772,23 +772,16 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration } static void -add_vary_header_for_compressible_content(TSHttpTxn txnp, bool server, HostConfiguration * /* hc ATS_UNUSED */) +add_vary_header_to_server_response(TSHttpTxn txnp) { TSMBuffer resp_buf; TSMLoc resp_loc; - // Get the response headers - if (server) { - if (TS_SUCCESS != TSHttpTxnServerRespGet(txnp, &resp_buf, &resp_loc)) { - return; - } - } else { - if (TS_SUCCESS != TSHttpTxnCachedRespGet(txnp, &resp_buf, &resp_loc)) { - return; - } + if (TS_SUCCESS != TSHttpTxnServerRespGet(txnp, &resp_buf, &resp_loc)) { + return; } - // Add Vary: Accept-Encoding header + // Add Vary: Accept-Encoding header to the origin response before caching. if (vary_header(resp_buf, resp_loc) != TS_SUCCESS) { error("failed to add Vary header for compressible content"); TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); @@ -798,6 +791,25 @@ add_vary_header_for_compressible_content(TSHttpTxn txnp, bool server, HostConfig TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } +static void +add_vary_header_to_client_response(TSHttpTxn txnp) +{ + TSMBuffer resp_buf; + TSMLoc resp_loc; + + if (TS_SUCCESS != TSHttpTxnClientRespGet(txnp, &resp_buf, &resp_loc)) { + return; + } + + if (vary_header(resp_buf, resp_loc) != TS_SUCCESS) { + error("failed to add Vary header to client response"); + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); + return; + } + + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); +} + static void compress_transform_add(TSHttpTxn txnp, HostConfiguration *hc, int compress_type, int algorithms) { @@ -824,7 +836,7 @@ compress_transform_add(TSHttpTxn txnp, HostConfiguration *hc, int compress_type, } static void -handle_compression_and_vary(TSHttpTxn txnp, bool server, HostConfiguration *hc, int *compress_type, int *algorithms) +handle_compression_and_vary(TSCont contp, TSHttpTxn txnp, bool server, HostConfiguration *hc, int *compress_type, int *algorithms) { // Check if content is compressible and add compression if client accepts it bool content_is_compressible; @@ -834,7 +846,11 @@ handle_compression_and_vary(TSHttpTxn txnp, bool server, HostConfiguration *hc, // Add Vary: Accept-Encoding for all compressible content to ensure proper HTTP caching if (content_is_compressible) { - add_vary_header_for_compressible_content(txnp, server, hc); + if (server) { + add_vary_header_to_server_response(txnp); + } else { + TSHttpTxnHookAdd(txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, contp); + } } } @@ -882,7 +898,7 @@ transform_plugin(TSCont contp, TSEvent event, void *edata) } } - handle_compression_and_vary(txnp, true, hc, &compress_type, &algorithms); + handle_compression_and_vary(contp, txnp, true, hc, &compress_type, &algorithms); } break; @@ -908,7 +924,7 @@ transform_plugin(TSCont contp, TSEvent event, void *edata) if (TS_ERROR != TSHttpTxnCacheLookupStatusGet(txnp, &obj_status) && (TS_CACHE_LOOKUP_HIT_FRESH == obj_status)) { if (hc != nullptr) { info("handling compression of cached object"); - handle_compression_and_vary(txnp, false, hc, &compress_type, &algorithms); + handle_compression_and_vary(contp, txnp, false, hc, &compress_type, &algorithms); } } else { // Prepare for going to origin @@ -917,6 +933,10 @@ transform_plugin(TSCont contp, TSEvent event, void *edata) } } break; + case TS_EVENT_HTTP_SEND_RESPONSE_HDR: + add_vary_header_to_client_response(txnp); + break; + case TS_EVENT_HTTP_TXN_CLOSE: // Release the ocnif lease, and destroy this continuation TSContDestroy(contp); diff --git a/plugins/prefetch/plugin.cc b/plugins/prefetch/plugin.cc index 22fe984f3da..d6eb81e186e 100644 --- a/plugins/prefetch/plugin.cc +++ b/plugins/prefetch/plugin.cc @@ -533,7 +533,6 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) // For these cases we need to access the client request switch (event) { - case TS_EVENT_HTTP_POST_REMAP: case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE: case TS_EVENT_HTTP_SEND_RESPONSE_HDR: if (TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &reqBuffer, &reqHdrLoc)) { @@ -547,15 +546,16 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) } switch (event) { - case TS_EVENT_HTTP_POST_REMAP: { - /* Use the cache key since this has better lookup behavior when using plugins like the cachekey plugin, - * for example multiple URIs can match a single cache key */ + case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE: { + /* Use the cache key (multiple URIs can map to one key). CACHE_LOOKUP_COMPLETE is + * the earliest hook where TSHttpTxnCacheLookupUrlGet returns a populated URL. */ if (data->frontend() && data->secondPass()) { /* Create a separate cache key name space to be used only for front-end and second-pass fetch policy checks. */ data->_cachekey.assign("/prefetch"); } if (!appendCacheKey(txnp, reqBuffer, data->_cachekey)) { PrefetchError("failed to get the cache key"); + TSHandleMLocRelease(reqBuffer, TS_NULL_MLOC, reqHdrLoc); TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR); return 0; } @@ -568,17 +568,15 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) data->_fetchable = state->acquire(data->_cachekey); PrefetchDebug("request is %s fetchable", data->_fetchable ? " " : " not "); } - } - } - } break; - - case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE: { - if (data->frontend()) { - /* front-end instance */ - if (data->secondPass()) { + } else { /* second-pass */ - data->_fetchable = state->acquire(data->_cachekey); - data->_fetchable = data->_fetchable && state->uniqueAcquire(data->_cachekey); + if (state->acquire(data->_cachekey)) { + if (state->uniqueAcquire(data->_cachekey)) { + data->_fetchable = true; + } else { + state->release(data->_cachekey); + } + } PrefetchDebug("request is %s fetchable", data->_fetchable ? " " : " not "); if (isFetchable(txnp, data)) { @@ -785,8 +783,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) } /* Release the request MLoc */ - if (event == TS_EVENT_HTTP_POST_REMAP || event == TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE || - event == TS_EVENT_HTTP_SEND_RESPONSE_HDR) { + if (event == TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE || event == TS_EVENT_HTTP_SEND_RESPONSE_HDR) { TSHandleMLocRelease(reqBuffer, TS_NULL_MLOC, reqHdrLoc); } @@ -919,7 +916,6 @@ TSRemapDoRemap(void *instance, TSHttpTxn txnp, TSRemapRequestInfo *rri) TSCont cont = TSContCreate(contHandleFetch, TSMutexCreate()); TSContDataSet(cont, static_cast(data)); - TSHttpTxnHookAdd(txnp, TS_HTTP_POST_REMAP_HOOK, cont); TSHttpTxnHookAdd(txnp, TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, cont); TSHttpTxnHookAdd(txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, cont); TSHttpTxnHookAdd(txnp, TS_HTTP_TXN_CLOSE_HOOK, cont); diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 1d4d3c9d32f..158c06fa72b 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -7341,6 +7341,10 @@ _memberp_to_generic(MgmtFloat *ptr, MgmtConverter const *&conv) -> typename std: case TS_CONFIG_##KEY: ret = &overridableHttpConfig->MEMBER; conv = &ConnectionTracker::MAX_SERVER_CONV; break; #define _CONF_CASE_ConnectionTracker_SERVER_MATCH_CONV(KEY, MEMBER) \ case TS_CONFIG_##KEY: ret = &overridableHttpConfig->MEMBER; conv = &ConnectionTracker::SERVER_MATCH_CONV; break; +#define _CONF_CASE_ConnectionTracker_METRIC_ENABLED_CONV(KEY, MEMBER) \ + case TS_CONFIG_##KEY: ret = &overridableHttpConfig->MEMBER; conv = &ConnectionTracker::METRIC_ENABLED_CONV; break; +#define _CONF_CASE_ConnectionTracker_METRIC_AGGREGATE_CONV(KEY, MEMBER) \ + case TS_CONFIG_##KEY: ret = &overridableHttpConfig->MEMBER; conv = &ConnectionTracker::METRIC_AGGREGATE_CONV; break; // Custom converter: Parses/formats host resolution preference strings. #define _CONF_CASE_HttpTransact_HOST_RES_CONV(KEY, MEMBER) \ @@ -7399,6 +7403,8 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr #undef _CONF_CASE_ConnectionTracker_MIN_SERVER_CONV #undef _CONF_CASE_ConnectionTracker_MAX_SERVER_CONV #undef _CONF_CASE_ConnectionTracker_SERVER_MATCH_CONV +#undef _CONF_CASE_ConnectionTracker_METRIC_ENABLED_CONV +#undef _CONF_CASE_ConnectionTracker_METRIC_AGGREGATE_CONV #undef _CONF_CASE_HttpTransact_HOST_RES_CONV #undef _CONF_CASE_TargetedCacheControlHeaders_Conv #undef _CONF_CASE_DISPATCH diff --git a/src/iocore/net/ConnectionTracker.cc b/src/iocore/net/ConnectionTracker.cc index 45ce7e60f07..85519616328 100644 --- a/src/iocore/net/ConnectionTracker.cc +++ b/src/iocore/net/ConnectionTracker.cc @@ -26,6 +26,8 @@ #include "records/RecCore.h" #include "swoc/IPAddr.h" +#include + using namespace std::literals; ConnectionTracker::TableSingleton ConnectionTracker::_inbound_table; @@ -70,10 +72,28 @@ const MgmtConverter ConnectionTracker::SERVER_MATCH_CONV{ } }}; +// Neither of these clamps, for the same reason as SERVER_MATCH_CONV above: the InkAPITest +// regression test requires an arbitrary integer to round trip through the setter and getter. The +// records paths do the range checking instead -- records.yaml validates the value and the reload +// callbacks below clamp -- so an out of range value is only reachable by a plugin that sets one +// deliberately. Both settings degrade safely if that happens: any non-zero metric_enabled enables +// metrics, and any metric_aggregate outside 0..2 publishes both the aggregate and the per group +// metrics, the same as AGGREGATE_GROUP. +const MgmtConverter ConnectionTracker::METRIC_ENABLED_CONV{ + [](const void *data) -> MgmtInt { return static_cast(*static_cast(data)); }, + [](void *data, MgmtInt i) -> void { + *static_cast(data) = static_cast(i); + }}; + +const MgmtConverter ConnectionTracker::METRIC_AGGREGATE_CONV{ + [](const void *data) -> MgmtInt { return static_cast(*static_cast(data)); }, + [](void *data, MgmtInt i) -> void { + *static_cast(data) = static_cast(i); + }}; + const std::array(ConnectionTracker::MATCH_BOTH) + 1> ConnectionTracker::MATCH_TYPE_NAME{ {"ip"sv, "port"sv, "host"sv, "both"sv} }; - // Make sure the clock is millisecond resolution or finer. static_assert(ConnectionTracker::Group::Clock::period::num == 1); static_assert(ConnectionTracker::Group::Clock::period::den >= 1000); @@ -153,10 +173,24 @@ Config_Update_Conntrack_Client_Alert_Delay(const char *name, RecDataT dtype, Rec bool Config_Update_Conntrack_Metric_Enabled(const char * /* name ATS_UNUSED */, RecDataT dtype, RecData data, void *cookie) { - auto config = static_cast(cookie); + auto config = static_cast(cookie); if (RECD_INT == dtype) { - config->metric_enabled = data.rec_int; + config->metric_enabled = std::clamp(static_cast(data.rec_int), 0, 1); + return true; + } + return false; +} + +bool +Config_Update_Conntrack_Metric_Aggregate(const char * /* name ATS_UNUSED */, RecDataT dtype, RecData data, void *cookie) +{ + auto config = static_cast(cookie); + + if (RECD_INT == dtype) { + auto level = std::clamp(static_cast(data.rec_int), static_cast(ConnectionTracker::AGGREGATE_NONE), + static_cast(ConnectionTracker::AGGREGATE_ONLY)); + config->metric_aggregate = static_cast(level); return true; } return false; @@ -268,7 +302,6 @@ ConnectionTracker::GlobalConfig::GlobalConfig(GlobalConfig const &other) { this->client_alert_delay = other.client_alert_delay; this->server_alert_delay = other.server_alert_delay; - this->metric_enabled = other.metric_enabled; this->metric_prefix = other.metric_prefix; // Lock the source to safely copy the exempt list. @@ -286,7 +319,6 @@ ConnectionTracker::GlobalConfig::operator=(GlobalConfig const &other) if (this != &other) { this->client_alert_delay = other.client_alert_delay; this->server_alert_delay = other.server_alert_delay; - this->metric_enabled = other.metric_enabled; this->metric_prefix = other.metric_prefix; // Lock both source and destination to safely copy the exempt list. // Lock in a consistent order to avoid deadlock (lock 'other' first, then 'this'). @@ -312,7 +344,8 @@ ConnectionTracker::config_init(GlobalConfig *global, TxnConfig *txn, RecConfigUp Enable_Config_Var(CONFIG_SERVER_VAR_MAX, &Config_Update_Conntrack_Max, config_cb, txn); Enable_Config_Var(CONFIG_SERVER_VAR_MATCH, &Config_Update_Conntrack_Match, config_cb, txn); Enable_Config_Var(CONFIG_SERVER_VAR_ALERT_DELAY, &Config_Update_Conntrack_Server_Alert_Delay, config_cb, global); - Enable_Config_Var(CONFIG_SERVER_VAR_METRIC_ENABLED, &Config_Update_Conntrack_Metric_Enabled, config_cb, global); + Enable_Config_Var(CONFIG_SERVER_VAR_METRIC_ENABLED, &Config_Update_Conntrack_Metric_Enabled, config_cb, txn); + Enable_Config_Var(CONFIG_SERVER_VAR_METRIC_AGGREGATE, &Config_Update_Conntrack_Metric_Aggregate, config_cb, txn); Enable_Config_Var(CONFIG_SERVER_VAR_METRIC_PREFIX, &Config_Update_Conntrack_Metric_Prefix, config_cb, global); } @@ -421,7 +454,8 @@ ConnectionTracker::obtain_outbound(TxnConfig const &txn_cnf, std::string_view fq if (loc != _outbound_table._table.end()) { zret._g = loc->second; } else { - zret._g = std::make_shared(Group::DirectionType::OUTBOUND, key, fqdn, txn_cnf.server_min); + zret._g = std::make_shared(Group::DirectionType::OUTBOUND, key, fqdn, txn_cnf.server_min, txn_cnf.metric_enabled, + txn_cnf.metric_aggregate); // Note that we must use zret._g's key, not the above key, because Key's // members are references to the Group's members. Thus the above key's // members are invalid after this function. @@ -430,7 +464,8 @@ ConnectionTracker::obtain_outbound(TxnConfig const &txn_cnf, std::string_view fq return zret; } -ConnectionTracker::Group::Group(DirectionType direction, Key const &key, std::string_view fqdn, int min_keep_alive) +ConnectionTracker::Group::Group(DirectionType direction, Key const &key, std::string_view fqdn, int min_keep_alive, + int metric_enabled, MetricAggregate metric_aggregate) : _direction{direction}, _hash(key._hash), _match_type(key._match_type), @@ -440,11 +475,46 @@ ConnectionTracker::Group::Group(DirectionType direction, Key const &key, std::st { Metrics::Gauge::increment(net_rsb.connection_tracker_table_size); // only add metrics for server connections - if (_global_config->metric_enabled && direction == DirectionType::OUTBOUND) { + if (metric_enabled && direction == DirectionType::OUTBOUND) { std::string _metric_name = metric_name(key, fqdn, _global_config->metric_prefix); - _count_metric = Metrics::Gauge::createPtr("proxy.process.http.per_server.current_connection.", _metric_name); - _count_total_metric = Metrics::Counter::createPtr("proxy.process.http.per_server.total_connection.", _metric_name); - _blocked_metric = Metrics::Counter::createPtr("proxy.process.http.per_server.blocked_connection.", _metric_name); + // Per group metrics always live in the hidden store. metric_aggregate controls what is + // published from them (see MetricAggregate), not whether they exist. + _count_metric = Metrics::Gauge::createHiddenPtr("proxy.process.http.per_server.current_connection.", _metric_name); + _count_total_metric = Metrics::Counter::createHiddenPtr("proxy.process.http.per_server.total_connection.", _metric_name); + _blocked_metric = Metrics::Counter::createHiddenPtr("proxy.process.http.per_server.blocked_connection.", _metric_name); + + // Only MATCH_BOTH groups have siblings sharing a hostname to aggregate across. + std::string _host_metric_name = host_metric_name(key, fqdn, _global_config->metric_prefix); + bool const has_aggregate = !_host_metric_name.empty(); + + if (has_aggregate && metric_aggregate != AGGREGATE_NONE) { + Metrics::Derived::add_source("proxy.process.http.per_server.current_connection." + _host_metric_name, + Metrics::MetricType::GAUGE, _count_metric, Metrics::Derived::Op::SUM); + Metrics::Derived::add_source("proxy.process.http.per_server.total_connection." + _host_metric_name, + Metrics::MetricType::COUNTER, _count_total_metric, Metrics::Derived::Op::SUM); + Metrics::Derived::add_source("proxy.process.http.per_server.blocked_connection." + _host_metric_name, + Metrics::MetricType::COUNTER, _blocked_metric, Metrics::Derived::Op::SUM); + // The largest current count among this hostname's groups, sampled. Deliberately taken over + // the instantaneous gauge rather than each group's all time peak, so the value falls again + // and a maximum over time can be computed by whatever scrapes it. + Metrics::Derived::add_source("proxy.process.http.per_server.current_connection_max." + _host_metric_name, + Metrics::MetricType::GAUGE, _count_metric, Metrics::Derived::Op::MAX); + } + + // AGGREGATE_ONLY suppresses the per group metrics to keep the published count proportional to + // hostnames. Without an aggregate to stand in for them there would be nothing at all reported + // for this group, so in that case publish them regardless. + if (metric_aggregate != AGGREGATE_ONLY || !has_aggregate) { + // Mirror the per group metrics into the published store under their own name. A single + // source SUM combines nothing, but the published value is still a sample: it is whatever + // the last derived tick read, and it reads 0 from creation until that first tick. + Metrics::Derived::add_source("proxy.process.http.per_server.current_connection." + _metric_name, Metrics::MetricType::GAUGE, + _count_metric, Metrics::Derived::Op::SUM); + Metrics::Derived::add_source("proxy.process.http.per_server.total_connection." + _metric_name, Metrics::MetricType::COUNTER, + _count_total_metric, Metrics::Derived::Op::SUM); + Metrics::Derived::add_source("proxy.process.http.per_server.blocked_connection." + _metric_name, Metrics::MetricType::COUNTER, + _blocked_metric, Metrics::Derived::Op::SUM); + } if (dbg_ctl.on()) { swoc::LocalBufferWriter<256> w; diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 9107b6164d8..9645a19a925 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -5851,7 +5851,7 @@ HttpSM::do_http_server_open(bool raw, bool only_direct) // See if the outbound connection tracker data is needed. If so, get it here for consistency. if (t_state.txn_conf->connection_tracker_config.server_max > 0 || t_state.txn_conf->connection_tracker_config.server_min > 0 || - t_state.http_config_param->global_connection_tracker_config.metric_enabled) { + t_state.txn_conf->connection_tracker_config.metric_enabled) { t_state.outbound_conn_track_state = ConnectionTracker::obtain_outbound(t_state.txn_conf->connection_tracker_config, std::string_view{t_state.current.server->name}, t_state.current.server->dst_addr); @@ -5879,9 +5879,11 @@ HttpSM::do_http_server_open(bool raw, bool only_direct) ct_state.update_max_count(ccount); } else if (t_state.txn_conf->connection_tracker_config.server_min > 0 || - t_state.http_config_param->global_connection_tracker_config.metric_enabled) { + t_state.txn_conf->connection_tracker_config.metric_enabled) { auto &ct_state = t_state.outbound_conn_track_state; - ct_state.reserve(); + // Feed the count through as well, otherwise the group's peak stays at zero whenever metrics + // are enabled without a configured maximum. + ct_state.update_max_count(ct_state.reserve()); } // We did not manage to get an existing session and need to open a new connection diff --git a/src/records/RecCore.cc b/src/records/RecCore.cc index 9639430d242..91dd163717c 100644 --- a/src/records/RecCore.cc +++ b/src/records/RecCore.cc @@ -516,18 +516,20 @@ RecGetRecordCounter(const char *name, bool lock) RecErrT RecLookupRecord(const char *name, void (*callback)(const RecRecord *, void *), void *data, bool lock) { - RecErrT err = REC_ERR_FAIL; - ts::Metrics &metrics = ts::Metrics::instance(); - auto it = metrics.find(name); + RecErrT err = REC_ERR_FAIL; + ts::Metrics &metrics = ts::Metrics::instance(); + ts::Metrics::IdType metric_id; - if (it != metrics.end()) { + // A metric's storage is stable after creation. Avoid find()/end() here because end() is the current insertion position and + // can advance between those two calls while another thread registers a metric. + if (auto *metric = metrics.lookup(name, &metric_id); metric != nullptr) { RecRecord r{}; - auto &&[name, type, val] = *it; r.rec_type = RECT_PLUGIN; - r.data_type = type == ts::Metrics::MetricType::COUNTER ? RECD_COUNTER : RECD_INT; - r.name = name.data(); - r.data.rec_int = val; + r.data_type = metrics.type(metric_id) == ts::Metrics::MetricType::COUNTER ? RECD_COUNTER : RECD_INT; + r.name = name; + r.data.rec_int = metric->load(); + r.registered = true; callback(&r, data); err = REC_ERR_OKAY; diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index 6032422b9f0..3d381bc71f2 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -404,7 +404,9 @@ static constexpr RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.http.per_server.connection.min", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL} , - {RECT_CONFIG, "proxy.config.http.per_server.connection.metric_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-1]", RECA_NULL} + {RECT_CONFIG, "proxy.config.http.per_server.connection.metric_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-1]$", RECA_NULL} + , + {RECT_CONFIG, "proxy.config.http.per_server.connection.metric_aggregate", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-2]$", RECA_NULL} , {RECT_CONFIG, "proxy.config.http.per_server.connection.metric_prefix", RECD_STRING, "", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL} , diff --git a/src/records/unit_tests/test_RecRegister.cc b/src/records/unit_tests/test_RecRegister.cc index 77d4c287625..38fb1adbf03 100644 --- a/src/records/unit_tests/test_RecRegister.cc +++ b/src/records/unit_tests/test_RecRegister.cc @@ -22,8 +22,12 @@ #include "iocore/eventsystem/EventSystem.h" #include "iocore/eventsystem/RecProcess.h" #include "tscore/Layout.h" +#include "tsutil/Metrics.h" #include "test_Diags.h" +#include +#include + TEST_CASE("RecRegisterConfig - Type Dispatch", "[librecords][RecConfig]") { SECTION("RecRegisterConfigInt") @@ -87,3 +91,32 @@ TEST_CASE("RecRegisterStat - Type Dispatch", "[librecords][RecStat]") REQUIRE(value == 500); } } + +TEST_CASE("RecLookupRecord - Concurrent metric registration", "[librecords][RecLookup]") +{ + constexpr char record_name[] = "proxy.test.concurrent.string_value"; + constexpr char record_value[] = "stable"; + + REQUIRE(RecRegisterConfigString(RECT_CONFIG, record_name, record_value, RECU_DYNAMIC, RECC_NULL, nullptr, REC_SOURCE_NULL) == + REC_ERR_OKAY); + + std::atomic finished{false}; + std::thread register_metrics([&]() { + for (int i = 0; i < 100000; ++i) { + ts::Metrics::Counter::createSpan(1); + } + finished.store(true, std::memory_order_release); + }); + + bool all_lookups_succeeded = true; + + do { + if (RecGetRecordStringAlloc(record_name) != record_value) { + all_lookups_succeeded = false; + break; + } + } while (!finished.load(std::memory_order_acquire)); + register_metrics.join(); + + CHECK(all_lookups_succeeded); +} diff --git a/tests/gold_tests/h3/h3_sni_check.test.py b/tests/gold_tests/h3/h3_sni_check.test.py index 08778162aa4..875254d4912 100644 --- a/tests/gold_tests/h3/h3_sni_check.test.py +++ b/tests/gold_tests/h3/h3_sni_check.test.py @@ -115,7 +115,7 @@ def run(self): "SNI not found", "ATS should see the SNI presented by client.") if self.gold_file: - tr.Processes.Default.Streams.all = self.gold_file + tr.Processes.Default.Streams.All = self.gold_file # TEST 1: Client request with SNI. diff --git a/tests/gold_tests/ip_allow/ip_allow.test.py b/tests/gold_tests/ip_allow/ip_allow.test.py index 0454a4e373f..ed9943476e0 100644 --- a/tests/gold_tests/ip_allow/ip_allow.test.py +++ b/tests/gold_tests/ip_allow/ip_allow.test.py @@ -298,7 +298,7 @@ def run(self): "client.*allowed by ip-allow policy", "Request should be allowed by ip_allow") if self.gold_file: - tr.Processes.Default.Streams.all = self.gold_file + tr.Processes.Default.Streams.All = self.gold_file # ip_allow tests for h3. diff --git a/tests/gold_tests/origin_connection/per_server_connection_max.test.py b/tests/gold_tests/origin_connection/per_server_connection_max.test.py index e7bad788ab3..05f2c5587c1 100644 --- a/tests/gold_tests/origin_connection/per_server_connection_max.test.py +++ b/tests/gold_tests/origin_connection/per_server_connection_max.test.py @@ -1,5 +1,7 @@ ''' -Verify the behavior of proxy.config.http.per_server.connection.max. +Verify the behavior of proxy.config.http.per_server.connection.max and the per server +connection metrics (proxy.config.http.per_server.connection.metric_enabled and +proxy.config.http.per_server.connection.metric_aggregate). ''' # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file @@ -22,6 +24,51 @@ Test.SkipIf(Condition.CurlUsingUnixDomainSocket()) +# The per hostname aggregates are derived metrics. Metrics::Derived::update_derived() runs from +# raw_stat_sync_cont (src/iocore/eventsystem/RecProcess.cc), which is scheduled every +# proxy.config.raw_stat_sync_interval_ms. That record defaults to 5000ms, which would force every +# assertion here to sleep more than five seconds; each ATS instance below shortens it so the waits +# can be short instead. The record is startup only, so it has to be set in records.yaml rather than +# adjusted at runtime. Reading before a tick lands silently compares against zeros. +_STAT_SYNC_INTERVAL_MS: int = 500 + +# How long to wait before reading a derived metric. Several sync periods, to absorb ET_TASK +# scheduling jitter and the traffic_ctl round trip rather than racing the tick. +_STAT_SYNC_WAIT_SECONDS: int = 2 + +# The records.yaml settings every ATS instance in this file needs for the waits above to hold. +_STAT_SYNC_RECORDS: dict = { + 'proxy.config.raw_stat_sync_interval_ms': _STAT_SYNC_INTERVAL_MS, +} + +# NOTE: assigning to a Streams attribute REPLACES any tester already set for that stream +# (TesterSet.Assign), so every assertion after the first on the same stream must use '+=' or it +# silently discards the earlier ones. The same applies to StillRunningAfter and the other process +# state checks: they are TesterSets too. + +# One microDNS server shared by every ATS instance here. All of them want the same thing, a +# wildcard answer of 127.0.0.1, and each extra process costs about five seconds when the test tears +# down, so this file uses a single server rather than one per test class. +_dns = Test.MakeDNServer("dns", default='127.0.0.1') +_dns_started: bool = False + + +def _use_shared_dns(tr) -> None: + """Make the shared nameserver available to a TestRun. + + Only the first run may start it: StartBefore is tracked per TestRun, so asking twice would try + to start an already running process. Later runs just assert it is still alive. + """ + global _dns_started + + if _dns_started: + # '+=': StillRunningAfter is a TesterSet like Streams, so '=' would discard any + # process check the caller has already set on this run. + tr.StillRunningAfter += _dns + else: + tr.Processes.Default.StartBefore(_dns) + _dns_started = True + class PerServerConnectionMaxTest: """Define an object to test our max origin connection behavior.""" @@ -37,7 +84,7 @@ def __init__(self) -> None: def _configure_dns(self) -> None: """Configure a nameserver for the test.""" - self._dns = Test.MakeDNServer("dns", default='127.0.0.1') + self._dns = _dns def _configure_server(self) -> None: """Configure the server to be used in the test.""" @@ -49,11 +96,15 @@ def _configure_trafficserver(self) -> None: self._ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{self._server.Variables.http_port}') self._ts.Disk.records_config.update( { + **_STAT_SYNC_RECORDS, 'proxy.config.dns.nameservers': f"127.0.0.1:{self._dns.Variables.Port}", 'proxy.config.dns.resolv_conf': 'NULL', 'proxy.config.diags.debug.enabled': 1, 'proxy.config.diags.debug.tags': 'http|conn_track', 'proxy.config.http.per_server.connection.max': self._origin_max_connections, + # The match here is 'port', which has no hostname aggregate, so the per group + # metrics themselves are what gets checked below. That is what the default + # metric_aggregate of 0 publishes, so only metric_enabled is needed. 'proxy.config.http.per_server.connection.metric_enabled': 1, 'proxy.config.http.per_server.connection.metric_prefix': 'foo', 'proxy.config.http.per_server.connection.match': 'port', @@ -64,21 +115,31 @@ def _configure_trafficserver(self) -> None: def _test_metrics(self) -> None: """Use traffic_ctl to test metrics.""" + group_name = f'foo.127.0.0.1:{self._server.Variables.http_port}' + tr = Test.AddTestRun("Check connection metrics") - tr.Processes.Default.Command = 'traffic_ctl metric match per_server' + # The per group metrics are published by mirroring the hidden ones through a derived + # metric, so a sync tick has to pass before they carry a value. + tr.Processes.Default.Command = f'sleep {_STAT_SYNC_WAIT_SECONDS}; traffic_ctl metric match per_server' tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.TimeOut = _STAT_SYNC_WAIT_SECONDS + 30 tr.Processes.Default.Streams.All = Testers.ContainsExpression( - f'per_server.total_connection.foo.127.0.0.1:{self._server.Variables.http_port} 4', - 'incorrect statistic return, or possible error.') - tr.Processes.Default.Streams.All = Testers.ContainsExpression( - f'per_server.blocked_connection.foo.127.0.0.1:{self._server.Variables.http_port} 1', - 'incorrect statistic return, or possible error.') + f'per_server.total_connection.{group_name} 4', 'incorrect statistic return, or possible error.') + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + 'INVALID_INCOMING_DATA', 'The metric query must not be rejected.') + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + f'per_server.blocked_connection.{group_name} 1', 'incorrect statistic return, or possible error.') + + # A 'port' match has one group per address:port and no hostname, so no aggregate should be + # registered for it at all. + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + 'per_server.current_connection_max.', 'A non-"both" match type must not register a hostname aggregate.') def run(self) -> None: """Configure the TestRun.""" tr = Test.AddTestRun('Verify we enforce proxy.config.http.per_server.connection.max') - tr.Processes.Default.StartBefore(self._dns) + _use_shared_dns(tr) tr.Processes.Default.StartBefore(self._server) tr.Processes.Default.StartBefore(self._ts) @@ -88,31 +149,45 @@ def run(self) -> None: class ConnectMethodTest: - """Test our max origin connection behavior with CONNECT traffic.""" + """Test our max origin connection behavior with CONNECT traffic. + + Also covers the two aggregate-publishing modes of + proxy.config.http.per_server.connection.metric_aggregate: + - 2 (AGGREGATE_ONLY): only the per hostname aggregate is published; the per group metrics + stay hidden and are visible only with --include-hidden. + - 1 (AGGREGATE_GROUP): the per hostname aggregate is published, and the per group metrics + are also mirrored into the published store. + + The match here defaults to 'both' and there is exactly one group for this hostname, so the + aggregate is a trivial sum over that single group. MultiGroupAggregateTest below covers the + case where an aggregate genuinely spans more than one group. + """ _process_counter: int = 0 _client_counter: int = 0 - def __init__(self, max_conn) -> None: + def __init__(self, max_conn, metric_aggregate=2) -> None: """Configure the server processes in preparation for the TestRun.""" + self._metric_aggregate = metric_aggregate self._configure_dns() self._configure_origin_server() - self._configure_trafficserver(max_conn) + self._configure_trafficserver(max_conn, metric_aggregate) ConnectMethodTest._process_counter += 1 def _configure_dns(self) -> None: """Configure a nameserver for the test.""" - self._dns = Test.MakeDNServer(f"dns_{ConnectMethodTest._process_counter}", default='127.0.0.1') + self._dns = _dns def _configure_origin_server(self) -> None: """Configure the httpbin origin server.""" self._server = Test.MakeHttpBinServer(f"server_{ConnectMethodTest._process_counter}") - def _configure_trafficserver(self, max_conn) -> None: - self._ts = Test.MakeATSProcess("ts2_" + str(max_conn)) + def _configure_trafficserver(self, max_conn, metric_aggregate) -> None: + self._ts = Test.MakeATSProcess(f"ts2_{max_conn}_{metric_aggregate}") self._ts.Disk.records_config.update( { + **_STAT_SYNC_RECORDS, 'proxy.config.dns.nameservers': f"127.0.0.1:{self._dns.Variables.Port}", 'proxy.config.dns.resolv_conf': 'NULL', 'proxy.config.diags.debug.enabled': 1, @@ -120,6 +195,7 @@ def _configure_trafficserver(self, max_conn) -> None: 'proxy.config.http.server_ports': f"{self._ts.Variables.port} {self._ts.Variables.uds_path}", 'proxy.config.http.connect_ports': f"{self._server.Variables.Port}", 'proxy.config.http.per_server.connection.metric_enabled': 1, + 'proxy.config.http.per_server.connection.metric_aggregate': metric_aggregate, 'proxy.config.http.per_server.connection.max': max_conn, }) @@ -136,22 +212,52 @@ def _configure_client_with_slow_response(self, tr) -> 'Test.Process': return p def _test_metrics(self, blocked) -> None: - """Use traffic_ctl to test metrics.""" + """Use traffic_ctl to test metrics, honoring the configured publication level.""" + host_name = 'www.this.origin.com' + group_name = f'{host_name}.127.0.0.1:{self._server.Variables.Port}' + tr = Test.AddTestRun("Check connection metrics") - tr.Processes.Default.Command = 'traffic_ctl metric match per_server; sleep 2' + tr.Processes.Default.Command = f'sleep {_STAT_SYNC_WAIT_SECONDS}; traffic_ctl metric match per_server' tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.TimeOut = _STAT_SYNC_WAIT_SECONDS + 30 + + # The per hostname aggregate is published in both modes under test. tr.Processes.Default.Streams.All = Testers.ContainsExpression( - f'per_server.total_connection.www.this.origin.com.127.0.0.1:{self._server.Variables.Port} 5', - 'incorrect statistic return, or possible error.') - tr.Processes.Default.Streams.All = Testers.ContainsExpression( - f'per_server.blocked_connection.www.this.origin.com.127.0.0.1:{self._server.Variables.Port} {blocked}', - 'incorrect statistic return, or possible error.') + f'per_server.total_connection.{host_name} 5', 'incorrect statistic return, or possible error.') + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + f'per_server.blocked_connection.{host_name} {blocked}', 'incorrect statistic return, or possible error.') + + if self._metric_aggregate == 1: + # AGGREGATE_GROUP additionally mirrors the per group metrics into the published store. + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + f'per_server.total_connection.{group_name} 5', 'The per group metric should be published at AGGREGATE_GROUP.') + else: + # AGGREGATE_ONLY keeps the per group metrics hidden, so none of the three per group + # names may appear in a normal query. current_connection_max is not among them: it only + # ever exists as a hostname aggregate, never per group. + for counter in ('current_connection', 'total_connection', 'blocked_connection'): + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + f'per_server.{counter}.{group_name} ', f'per_server.{counter}.{group_name} must stay hidden at AGGREGATE_ONLY.') + + # The per group metrics must be visible with --include-hidden at either level. This is also + # the end to end test for that traffic_ctl option. + tr2 = Test.AddTestRun("Check hidden per group connection metrics") + tr2.Processes.Default.Command = 'traffic_ctl metric match per_server --include-hidden' + tr2.Processes.Default.ReturnCode = 0 + tr2.Processes.Default.Env = self._ts.Env + # No sleep needed: the hidden per group metrics are written directly on each connection, + # unlike the derived aggregates. + tr2.Processes.Default.Streams.All = Testers.ContainsExpression( + f'per_server.total_connection.{group_name} 5', + 'The per group metric should be visible with --include-hidden at any level.') + tr2.Processes.Default.Streams.All += Testers.ExcludesExpression( + 'INVALID_INCOMING_DATA', 'The --include-hidden query must not be rejected by the RPC decoder.') def run(self, blocked, gold_file) -> None: """Verify per_server.connection.max with CONNECT traffic.""" tr = Test.AddTestRun() - tr.Processes.Default.StartBefore(self._dns) + _use_shared_dns(tr) tr.Processes.Default.StartBefore(self._server) tr.Processes.Default.StartBefore(self._ts) @@ -179,6 +285,304 @@ def run(self, blocked, gold_file) -> None: self._test_metrics(blocked) +class MultiGroupAggregateTest: + """Verify a per hostname aggregate that genuinely spans more than one group. + + The other tests here resolve a hostname to a single 127.0.0.1:port, so their "aggregate" is + trivially a set of one. Here two remap rules point at the same hostname ('multi.origin.com') + on two different origin ports, so under match 'both' the connection tracker creates two + distinct groups sharing one host aggregate. The two groups are given different concurrency so + the SUM and the MAX are distinguishable from each other. + + current_connection and current_connection_max are instantaneous gauges recomputed from the live + per group values every ~5s, so they rise and fall with traffic rather than remembering a peak. + Observing a non-zero value therefore requires holding connections open across a sync tick. The + most robust assertion, and the one that actually distinguishes this instantaneous behavior from + a monotone peak, is that both gauges return to 0 once traffic drains and another tick passes. + """ + + _process_counter: int = 0 + _client_counter: int = 0 + + # Concurrent slow requests per group. Deliberately different so SUM (5) and MAX (3) differ. + _group_a_concurrency: int = 2 + _group_b_concurrency: int = 3 + + # How long each request holds its connection open. Must comfortably exceed + # _STAT_SYNC_WAIT_SECONDS so a sync tick is guaranteed to land while the connections are still + # open. Well under the 10 second cap httpbin puts on /delay/, so no clamping applies. + _hold_seconds: int = 6 + + def __init__(self) -> None: + """Configure the test processes in preparation for the TestRun.""" + self._configure_dns() + self._configure_origin_servers() + self._configure_trafficserver() + MultiGroupAggregateTest._process_counter += 1 + + def _configure_dns(self) -> None: + """Configure a nameserver for the test.""" + self._dns = _dns + + def _configure_origin_servers(self) -> None: + """Configure the two httpbin origins which stand in for two groups of one hostname.""" + self._server_a = Test.MakeHttpBinServer(f"magg_server_a_{MultiGroupAggregateTest._process_counter}") + self._server_b = Test.MakeHttpBinServer(f"magg_server_b_{MultiGroupAggregateTest._process_counter}") + + def _configure_trafficserver(self) -> None: + """Configure Traffic Server with two remap rules to the same hostname on different ports.""" + self._ts = Test.MakeATSProcess(f"magg_ts_{MultiGroupAggregateTest._process_counter}") + self._ts.Disk.records_config.update( + { + **_STAT_SYNC_RECORDS, + 'proxy.config.dns.nameservers': f"127.0.0.1:{self._dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|dns|hostdb|conn_track', + 'proxy.config.http.per_server.connection.metric_enabled': 1, + # Aggregates only: the per group metrics stay hidden, which is what this test is + # about reading through the aggregate. + 'proxy.config.http.per_server.connection.metric_aggregate': 2, + 'proxy.config.http.per_server.connection.match': 'both', + }) + self._ts.Disk.remap_config.AddLines( + [ + f"map http://multi.origin.com/a/ http://multi.origin.com:{self._server_a.Variables.Port}/", + f"map http://multi.origin.com/b/ http://multi.origin.com:{self._server_b.Variables.Port}/", + ]) + + def _make_slow_client(self, tr, path) -> 'Test.Process': + """Configure a client which makes a slow request through one of the two remapped groups.""" + p = tr.Processes.Process(f'magg_client_{MultiGroupAggregateTest._client_counter}') + MultiGroupAggregateTest._client_counter += 1 + tr.MakeCurlCommand( + f"-v --fail -s -x 127.0.0.1:{self._ts.Variables.port} " + f"'http://multi.origin.com/{path}/delay/{MultiGroupAggregateTest._hold_seconds}'", + p=p, + ts=self._ts) + return p + + def _test_metrics_while_held(self) -> None: + """While the slow requests are still in flight, verify the live gauges reflect them.""" + total = MultiGroupAggregateTest._group_a_concurrency + MultiGroupAggregateTest._group_b_concurrency + group_max = max(MultiGroupAggregateTest._group_a_concurrency, MultiGroupAggregateTest._group_b_concurrency) + + tr = Test.AddTestRun("Check the host aggregate spans both groups while connections are held open") + tr.Processes.Default.Command = f'sleep {_STAT_SYNC_WAIT_SECONDS}; traffic_ctl metric match per_server' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.TimeOut = _STAT_SYNC_WAIT_SECONDS + 30 + tr.Processes.Default.Streams.All = Testers.ContainsExpression( + f'per_server.total_connection.multi.origin.com {total}', + 'The host aggregate total_connection should be the SUM across both groups ' + f'({MultiGroupAggregateTest._group_a_concurrency} + {MultiGroupAggregateTest._group_b_concurrency}).') + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + f'per_server.current_connection.multi.origin.com {total}', + 'While held open, the host aggregate current_connection should be the SUM of the ' + 'currently open connections across both groups.') + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + f'per_server.current_connection_max.multi.origin.com {group_max}', + 'While held open, current_connection_max should be the largest single group current ' + 'count (MAX), not the sum across the two groups.') + + def _test_metrics_after_drain(self) -> None: + """After traffic drains and a further sync tick passes, both live gauges must read 0. + + This validates the behavior the design exists to provide: an instantaneous gauge, unlike a + monotone peak, comes back down. + """ + tr = Test.AddTestRun("Check the host aggregate drains back to 0 after traffic stops") + # The slow requests are already _STAT_SYNC_WAIT_SECONDS old by now; wait for the rest of + # their hold time and then for another sync tick to observe the drop to 0. + wait = max(0, MultiGroupAggregateTest._hold_seconds - _STAT_SYNC_WAIT_SECONDS) + _STAT_SYNC_WAIT_SECONDS + tr.Processes.Default.Command = f'sleep {wait}; traffic_ctl metric match per_server' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.TimeOut = wait + 30 + tr.Processes.Default.Streams.All = Testers.ContainsExpression( + 'per_server.current_connection.multi.origin.com 0', + 'Once all connections close, the host aggregate current_connection must drain to 0.') + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + 'per_server.current_connection_max.multi.origin.com 0', + 'Once all connections close, current_connection_max must also come back down to 0: it ' + 'is a live gauge, not a monotone peak.') + + def run(self) -> None: + """Drive concurrent traffic through both groups, then check the aggregate metrics.""" + tr = Test.AddTestRun() + _use_shared_dns(tr) + tr.Processes.Default.StartBefore(self._server_a) + tr.Processes.Default.StartBefore(self._server_b) + tr.Processes.Default.StartBefore(self._ts) + + clients = [self._make_slow_client(tr, 'a') for _ in range(MultiGroupAggregateTest._group_a_concurrency)] + clients += [self._make_slow_client(tr, 'b') for _ in range(MultiGroupAggregateTest._group_b_concurrency)] + for p in clients: + tr.Processes.Default.StartBefore(p) + + # Let the slow requests connect and overlap before checking anything; they stay open for + # _hold_seconds from about this point. + tr.Processes.Default.Command = 'sleep 1' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.TimeOut = 30 + + self._test_metrics_while_held() + self._test_metrics_after_drain() + + +class MetricOverrideTest: + """Verify proxy.config.http.per_server.connection.metric_enabled is overridable per remap rule. + + Metrics are enabled globally and one of the two remap rules turns them off with + conf_remap. The two rules point at different origin ports and the match is 'port', so each gets + its own group and the two decisions cannot influence each other. + """ + + def __init__(self) -> None: + """Configure the test processes in preparation for the TestRun.""" + self._dns = _dns + self._server_on = Test.MakeHttpBinServer("server_metric_on") + self._server_off = Test.MakeHttpBinServer("server_metric_off") + self._configure_trafficserver() + + def _configure_trafficserver(self) -> None: + """Configure Traffic Server to be used in the test.""" + self._ts = Test.MakeATSProcess("ts_metric_override") + self._ts.Disk.records_config.update( + { + **_STAT_SYNC_RECORDS, + 'proxy.config.dns.nameservers': f"127.0.0.1:{self._dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|conn_track', + # Enabled globally; the second remap rule below opts out. + 'proxy.config.http.per_server.connection.metric_enabled': 1, + 'proxy.config.http.per_server.connection.match': 'port', + }) + self._ts.Disk.remap_config.AddLines( + [ + f'map http://metric-on.com/ http://127.0.0.1:{self._server_on.Variables.Port}/', + f'map http://metric-off.com/ http://127.0.0.1:{self._server_off.Variables.Port}/' + ' @plugin=conf_remap.so' + ' @pparam=proxy.config.http.per_server.connection.metric_enabled=0', + ]) + + def _test_metrics(self) -> None: + """Use traffic_ctl to verify which per server metrics exist.""" + on_group = f'127.0.0.1:{self._server_on.Variables.Port}' + off_group = f'127.0.0.1:{self._server_off.Variables.Port}' + + tr = Test.AddTestRun("Check that only the non-overridden remap has per server metrics") + tr.Processes.Default.Command = f'sleep {_STAT_SYNC_WAIT_SECONDS}; traffic_ctl metric match per_server' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.TimeOut = _STAT_SYNC_WAIT_SECONDS + 30 + tr.Processes.Default.Streams.All = Testers.ContainsExpression( + f'per_server.total_connection.{on_group} 1', 'The remap with metrics enabled should have per server metrics.') + # The group for the overridden remap must not exist at all, hidden or otherwise, so this + # also holds with --include-hidden below. + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + f'per_server.total_connection.{off_group}', 'The remap with metrics disabled should have no per server metrics.') + + tr2 = Test.AddTestRun("The overridden remap has no hidden per server metrics either") + tr2.Processes.Default.Command = 'traffic_ctl metric match per_server --include-hidden' + tr2.Processes.Default.ReturnCode = 0 + tr2.Processes.Default.Env = self._ts.Env + tr2.Processes.Default.Streams.All = Testers.ContainsExpression( + f'per_server.total_connection.{on_group} 1', 'The enabled remap group should be present in the hidden store.') + tr2.Processes.Default.Streams.All += Testers.ExcludesExpression( + f'per_server.total_connection.{off_group}', 'No group should be created at all for the overridden remap.') + + def run(self) -> None: + """Configure the TestRun.""" + tr = Test.AddTestRun('Verify metric_enabled is overridable per remap rule') + _use_shared_dns(tr) + tr.Processes.Default.StartBefore(self._server_on) + tr.Processes.Default.StartBefore(self._server_off) + tr.Processes.Default.StartBefore(self._ts) + tr.MakeCurlCommandMulti( + f"{{curl}} -v -s -H 'Host: metric-on.com' http://127.0.0.1:{self._ts.Variables.port}/get" + f" --next -v -s -H 'Host: metric-off.com' http://127.0.0.1:{self._ts.Variables.port}/get") + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.TimeOut = 30 + tr.StillRunningAfter += self._ts + + self._test_metrics() + + +class AggregateOnlyWithoutHostAggregateTest: + """Verify metric_aggregate 2 still publishes per group metrics when there is no aggregate. + + metric_aggregate 2 (AGGREGATE_ONLY) normally leaves the per group metrics hidden and publishes + only the per hostname aggregate. That aggregate exists only under match 'both', which is the + only match type with more than one group per hostname (Group::host_metric_name returns empty + for the others). With match 'port' there is therefore nothing for the aggregate to stand in + for, so the per group metrics have to be published regardless, or level 2 would report nothing + at all for this group. + + Every other test in this file that sets metric_aggregate 2 uses match 'both', so without this + case a regression that dropped the fallback would leave the suite green. + """ + + def __init__(self) -> None: + """Configure the test processes in preparation for the TestRun.""" + self._dns = _dns + self._server = Test.MakeHttpBinServer("agg_only_no_host_server") + self._configure_trafficserver() + + def _configure_trafficserver(self) -> None: + """Configure Traffic Server for aggregates only against a match type with no aggregate.""" + self._ts = Test.MakeATSProcess("ts_agg_only_no_host") + self._ts.Disk.records_config.update( + { + **_STAT_SYNC_RECORDS, + 'proxy.config.dns.nameservers': f"127.0.0.1:{self._dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|conn_track', + 'proxy.config.http.per_server.connection.metric_enabled': 1, + 'proxy.config.http.per_server.connection.metric_aggregate': 2, + # 'port' has no per hostname aggregate, which is the point of this test. + 'proxy.config.http.per_server.connection.match': 'port', + }) + self._ts.Disk.remap_config.AddLine(f'map http://agg-only.com/ http://127.0.0.1:{self._server.Variables.Port}/') + + def _test_metrics(self) -> None: + """Verify the per group metrics are published despite metric_aggregate 2.""" + group = f'127.0.0.1:{self._server.Variables.Port}' + + tr = Test.AddTestRun("Check the per group metrics are published when no aggregate exists") + tr.Processes.Default.Command = f'sleep {_STAT_SYNC_WAIT_SECONDS}; traffic_ctl metric match per_server' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Env = self._ts.Env + tr.Processes.Default.TimeOut = _STAT_SYNC_WAIT_SECONDS + 30 + # Published, not just hidden: this query does not pass --include-hidden. + tr.Processes.Default.Streams.All = Testers.ContainsExpression( + f'per_server.total_connection.{group} 1', + 'At metric_aggregate 2 with a match type that has no aggregate, the per group metric ' + 'must still be published.') + # The hostname never appears in a metric name under match 'port', so its absence confirms + # the published metric came from the per group fallback and not from an aggregate. + tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + 'per_server.total_connection.agg-only.com', 'No per hostname aggregate should exist for match "port".') + + def run(self) -> None: + """Drive one request through the origin, then check the metrics.""" + tr = Test.AddTestRun('Verify metric_aggregate 2 falls back to per group metrics') + _use_shared_dns(tr) + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + tr.MakeCurlCommand(f"-v -s -H 'Host: agg-only.com' http://127.0.0.1:{self._ts.Variables.port}/get", ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.TimeOut = 30 + tr.StillRunningAfter += self._ts + + self._test_metrics() + + PerServerConnectionMaxTest().run() -ConnectMethodTest(3).run(blocked=2, gold_file="gold/two_503_congested.gold") -ConnectMethodTest(0).run(blocked=0, gold_file="gold/two_200_ok.gold") +ConnectMethodTest(3, metric_aggregate=2).run(blocked=2, gold_file="gold/two_503_congested.gold") +ConnectMethodTest(0, metric_aggregate=1).run(blocked=0, gold_file="gold/two_200_ok.gold") +MultiGroupAggregateTest().run() +MetricOverrideTest().run() +AggregateOnlyWithoutHostAggregateTest().run() diff --git a/tests/gold_tests/origin_connection/per_server_metric_enabled.test.py b/tests/gold_tests/origin_connection/per_server_metric_enabled.test.py index 510265c3df9..30091ecac51 100644 --- a/tests/gold_tests/origin_connection/per_server_metric_enabled.test.py +++ b/tests/gold_tests/origin_connection/per_server_metric_enabled.test.py @@ -36,6 +36,12 @@ class PerServerMetricEnabledTest: _replay_file: str = 'per_server_metric_enabled.replay.yaml' _keep_alive_timeout: int = 2 + # The per group metric asserted below is published by mirroring the internal one through a + # derived metric, refreshed every proxy.config.raw_stat_sync_interval_ms. The 5000ms default + # would leave at most one second of margin inside this test's wait, so shorten the interval + # rather than racing it. The record is startup only and so has to be set in records.yaml. + _stat_sync_interval_ms: int = 500 + def __init__(self) -> None: """Configure the test processes in preparation for the TestRun.""" self._configure_server() @@ -51,6 +57,7 @@ def _configure_trafficserver(self) -> None: self._ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{self._server.Variables.http_port}') self._ts.Disk.records_config.update( { + 'proxy.config.raw_stat_sync_interval_ms': self._stat_sync_interval_ms, 'proxy.config.diags.debug.enabled': 1, 'proxy.config.diags.debug.tags': 'http_ss|conn_track', 'proxy.config.http.per_server.connection.metric_enabled': 1, diff --git a/tests/gold_tests/pluginTest/compress/compress-vary-cached-response.test.py b/tests/gold_tests/pluginTest/compress/compress-vary-cached-response.test.py new file mode 100644 index 00000000000..033555fc16a --- /dev/null +++ b/tests/gold_tests/pluginTest/compress/compress-vary-cached-response.test.py @@ -0,0 +1,101 @@ +''' +Regression test for cached Vary header updates when the client omits +Accept-Encoding. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Regression test for compress Vary header updates on cached responses when the +client does not send Accept-Encoding. +''' + +Test.SkipUnless(Condition.PluginExists('compress.so')) + +server = Test.MakeOriginServer("server") +request_header = {"headers": "GET /object HTTP/1.1\r\nHost: seed.example\r\n\r\n", "timestamp": "1469733493.993", "body": ""} +response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-Control: public, max-age=3600\r\n" + "Content-Type: text/javascript\r\n" + "Content-Length: 22\r\n\r\n", + "timestamp": "1469733493.993", + "body": "var cached_value = 1;\n" +} +server.addResponse("sessionlog.json", request_header, response_header) + +ts = Test.MakeATSProcess("ts", enable_cache=True) +ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'compress', + 'proxy.config.diags.output.diag': 'L', + }) +ts.Setup.Copy("etc/compress-cache-false.config") +ts.Disk.remap_config.AddLine(f'map http://seed.example/ http://127.0.0.1:{server.Variables.Port}/') +ts.Disk.remap_config.AddLine( + f'map http://compress.example/ http://127.0.0.1:{server.Variables.Port}/' + f' @plugin=compress.so @pparam={Test.RunDirectory}/compress-cache-false.config') + +ts.Disk.diags_log.Content += Testers.ContainsExpression( + 'handling compression of cached object', 'compress plugin must process a cached response') +ts.Disk.diags_log.Content += Testers.ExcludesExpression('cannot add/update the Vary header', 'cached response must not be mutated') +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + 'failed to add Vary header for compressible content', 'origin Vary update must succeed') +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + 'failed to add Vary header to client response', 'client Vary update must succeed') + +seed = Test.AddTestRun('seed cache without compress plugin') +seed.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port)) +seed.Processes.Default.StartBefore(ts) +seed.Processes.Default.Command = ( + f'curl --http1.1 -sS -o /dev/null --proxy http://127.0.0.1:{ts.Variables.port}' + ' http://seed.example/object') +seed.Processes.Default.ReturnCode = 0 +seed.StillRunningAfter = server +seed.StillRunningAfter = ts + +first_cached = Test.AddTestRun('add Vary to cached client response') +first_cached.Processes.Default.Command = ( + f'curl --http1.1 -sS -o /dev/null --proxy http://127.0.0.1:{ts.Variables.port}' + ' http://compress.example/object') +first_cached.Processes.Default.ReturnCode = 0 +first_cached.StillRunningAfter = server +first_cached.StillRunningAfter = ts + +headers_path = f'{Test.RunDirectory}/cached_headers.txt' +body_path = f'{Test.RunDirectory}/cached_body.txt' +second_cached = Test.AddTestRun('verify cached client response Vary header') +second_cached.Processes.Default.Command = ( + f'curl --http1.1 -sS -D {headers_path} -o {body_path}' + f' --proxy http://127.0.0.1:{ts.Variables.port}' + ' http://compress.example/object') +second_cached.Processes.Default.ReturnCode = 0 +second_cached.StillRunningAfter = server +second_cached.StillRunningAfter = ts + +verify = Test.AddTestRun('verify cached response headers and body') +verify.Processes.Default.Command = ( + f"grep -i '^Vary:.*Accept-Encoding' {headers_path}" + f" && ! grep -i '^Content-Encoding:' {headers_path}" + f" && diff {body_path} - <<'EOF'\n" + 'var cached_value = 1;\n' + 'EOF') +verify.Processes.Default.ReturnCode = 0 +verify.StillRunningAfter = server +verify.StillRunningAfter = ts diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_no_cachekey.test.py b/tests/gold_tests/pluginTest/prefetch/prefetch_no_cachekey.test.py new file mode 100644 index 00000000000..c12e27607b5 --- /dev/null +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_no_cachekey.test.py @@ -0,0 +1,76 @@ +''' +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Test prefetch.so plugin without cachekey.so loaded ahead of it. Exercises the +path where TSHttpTxnCacheLookupUrlGet must be served by the core's own cache +lookup URL initialization rather than a prior plugin's TSHttpTxnCacheLookupUrlSet. +''' + +server = Test.MakeOriginServer("server") +for i in list(range(1, 1 + 3)): + request_header = { + "headers": + f"GET /texts/demo-{i}.txt HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cannot be omitted. + "\r\n", + "timestamp": "1469733493.993", + "body": "" + } + response_header = { + "headers": "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=85000\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for demo-{i}.txt.\n" + } + server.addResponse("sessionlog.json", request_header, response_header) + +dns = Test.MakeDNServer("dns") + +ts = Test.MakeATSProcess("ts") +ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|dns|prefetch', + 'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': "NULL", + }) +ts.Disk.remap_config.AddLine( + f"map http://domain.in http://127.0.0.1:{server.Variables.Port}" + " @plugin=prefetch.so" + " @pparam=--front=true" + + " @pparam=--fetch-policy=simple" + r" @pparam=--fetch-path-pattern=/(.*-)(\d+)(.*)/$1{$2+1}$3/" + " @pparam=--fetch-count=3") +ts.ReturnCode = Any(0, -2) + +tr = Test.AddTestRun() +tr.Processes.Default.StartBefore(server) +tr.Processes.Default.StartBefore(dns) +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = 'echo start TS, HTTP server and DNS.' +tr.Processes.Default.ReturnCode = 0 + +tr = Test.AddTestRun() +tr.MakeCurlCommand(f'--verbose --proxy 127.0.0.1:{ts.Variables.port} http://domain.in/texts/demo-1.txt', ts=ts) +tr.Processes.Default.ReturnCode = 0 + +Test.AddAwaitFileContainsTestRun('Await transactions to finish logging.', ts.Disk.traffic_out.Name, 'demo-4.txt') + +tr = Test.AddTestRun() +tr.Processes.Default.Command = (f"grep 'GET http://' {ts.Disk.traffic_out.Name} | grep -v '127.0.0.1'") +tr.Streams.stdout = "prefetch_simple.gold" +tr.Processes.Default.ReturnCode = 0 diff --git a/tests/gold_tests/timeout/default_inactivity_timeout.test.py b/tests/gold_tests/timeout/default_inactivity_timeout.test.py index a56bdd604a1..3633d263965 100644 --- a/tests/gold_tests/timeout/default_inactivity_timeout.test.py +++ b/tests/gold_tests/timeout/default_inactivity_timeout.test.py @@ -93,7 +93,7 @@ def run(self) -> None: # Set up expectectations for the timeout closing the connection. tr.Processes.Default.ReturnCode = 1 - tr.Processes.Default.Streams.all = self.client_gold_file + tr.Processes.Default.Streams.All = self.client_gold_file test = TestDefaultInactivityTimeout("global config", use_override=False) diff --git a/tests/gold_tests/timeout/gold/client_default_inactivity_timeout.gold b/tests/gold_tests/timeout/gold/client_default_inactivity_timeout.gold index 6f195d5c7b0..703959de799 100644 --- a/tests/gold_tests/timeout/gold/client_default_inactivity_timeout.gold +++ b/tests/gold_tests/timeout/gold/client_default_inactivity_timeout.gold @@ -1,4 +1,4 @@ -``` +`` ``PARSE_INCOMPLETE ``Failed HTTP/1 transaction with key: timeout2 `` diff --git a/tests/gold_tests/timeout/quic_no_activity_timeout.test.py b/tests/gold_tests/timeout/quic_no_activity_timeout.test.py index 2f61dd0a132..8d48319ab51 100644 --- a/tests/gold_tests/timeout/quic_no_activity_timeout.test.py +++ b/tests/gold_tests/timeout/quic_no_activity_timeout.test.py @@ -102,7 +102,7 @@ def run(self, check_for_max_idle_timeout=False): tr.Processes.Default.ReturnCode = 0 if self.gold_file: - tr.Processes.Default.Streams.all = self.gold_file + tr.Processes.Default.Streams.All = self.gold_file # Tests start. diff --git a/tests/gold_tests/tls/allow-plain.test.py b/tests/gold_tests/tls/allow-plain.test.py index d4791383f4f..7e9b9457b6b 100644 --- a/tests/gold_tests/tls/allow-plain.test.py +++ b/tests/gold_tests/tls/allow-plain.test.py @@ -74,7 +74,7 @@ tr.Processes.Default.ReturnCode = 0 tr.StillRunningAfter = server tr.StillRunningAfter = ts -tr.Processes.Default.Streams.all = Testers.ContainsExpression("TLS", "Should negiotiate TLS") +tr.Processes.Default.Streams.All = Testers.ContainsExpression("TLS", "Should negiotiate TLS") # non-TLS curl should also work to the same port tr2 = Test.AddTestRun() @@ -85,7 +85,7 @@ tr2.Processes.Default.ReturnCode = 0 tr2.StillRunningAfter = server tr2.StillRunningAfter = ts -tr2.Processes.Default.Streams.all = Testers.ExcludesExpression("TLS", "Should not negiotiate TLS") +tr2.Processes.Default.Streams.All = Testers.ExcludesExpression("TLS", "Should not negiotiate TLS") # Make sure a post > 32K works. Early version forgot to free a reader which caused a stall once the initial buffer filled # Seems like we needed to make a second resquest to trigger the issue @@ -97,4 +97,4 @@ tr3.Processes.Default.ReturnCode = 0 tr3.StillRunningAfter = server tr3.StillRunningAfter = ts -tr3.Processes.Default.Streams.all = Testers.ExcludesExpression("TLS", "Should not negiotiate TLS") +tr3.Processes.Default.Streams.All = Testers.ExcludesExpression("TLS", "Should not negiotiate TLS") diff --git a/tests/gold_tests/tls/tls_hooks_client_verify.test.py b/tests/gold_tests/tls/tls_hooks_client_verify.test.py index f25e80b4890..dad3d5becaa 100644 --- a/tests/gold_tests/tls/tls_hooks_client_verify.test.py +++ b/tests/gold_tests/tls/tls_hooks_client_verify.test.py @@ -45,6 +45,8 @@ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.exec_thread.autoconfig.scale': 1.0, 'proxy.config.ssl.CA.cert.filename': '{0}/signer.pem'.format(ts.Variables.SSLDir), + # The origin serves a self-signed cert; this test verifies inbound client certs. + 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', 'proxy.config.url_remap.pristine_host_hdr': 1 }) @@ -81,7 +83,7 @@ .format(ts.Variables.ssl_port), ts=ts) tr.Processes.Default.ReturnCode = 0 -tr.Processes.Default.Streams.all = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") +tr.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") tr2 = Test.AddTestRun("request bad name") tr2.StillRunningAfter = ts @@ -91,7 +93,7 @@ .format(ts.Variables.ssl_port), ts=ts) tr2.Processes.Default.ReturnCode = 35 -tr2.Processes.Default.Streams.all = Testers.ContainsExpression("error", "Curl attempt should have failed") +tr2.Processes.Default.Streams.All = Testers.ContainsExpression("error", "Curl attempt should have failed") tr3 = Test.AddTestRun("request badly signed cert") tr3.Setup.Copy("ssl/server.pem") @@ -103,7 +105,7 @@ ts.Variables.ssl_port), ts=ts) tr3.Processes.Default.ReturnCode = 35 -tr3.Processes.Default.Streams.all = Testers.ContainsExpression("error", "Curl attempt should have failed") +tr3.Processes.Default.Streams.All = Testers.ContainsExpression("error", "Curl attempt should have failed") ts.Disk.traffic_out.Content += Testers.ContainsExpression( r"Client verify callback 0 [\da-fx]+? - event is good good HS", "verify callback happens 2 times") diff --git a/tests/gold_tests/tls/tls_sni_groups.test.py b/tests/gold_tests/tls/tls_sni_groups.test.py index 16c1cce280a..e2dea0f0e7a 100644 --- a/tests/gold_tests/tls/tls_sni_groups.test.py +++ b/tests/gold_tests/tls/tls_sni_groups.test.py @@ -76,7 +76,7 @@ tr.StillRunningAfter = ts ts.Disk.traffic_out.Content += Testers.ContainsExpression( "Setting groups list from server_groups_list to x25519", "Should log setting the server groups") -tr.Processes.Default.Streams.all = Testers.IncludesExpression( +tr.Processes.Default.Streams.All = Testers.IncludesExpression( f"SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384 / x25519", "Curl should log using x25519 in the SSL connection") tr = Test.AddTestRun("Test 1: fail") @@ -102,6 +102,6 @@ tr.StillRunningAfter = ts ts.Disk.traffic_out.Content += Testers.ContainsExpression( "Setting groups list from server_groups_list to X25519MLKEM768", "Should log setting the server groups") - tr.Processes.Default.Streams.all = Testers.IncludesExpression( + tr.Processes.Default.Streams.All = Testers.IncludesExpression( f"SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384 / X25519MLKEM768", f"Curl should log using X25519MLKEM768 in the SSL connection")