1
0
mirror of https://github.com/facebook/proxygen.git synced 2026-10-24 01:33:54 +03:00

Skip codec counters no session can record

Summary:
`TLHTTPCodecStats` creates every frame counter on every instance, but some frames can't occur for a given protocol or direction. `TLProxyStats` now passes the protocol and direction of each of its four instances, and the stats skip these:
- **HTTP/3:**
  - RST_STREAM, PING and WINDOW_UPDATE, which [HQ codecs leave to the QUIC transport](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/codec/HQFramedCodec.h?lines=160-166,178-191,205-211).
  - Ingress GOAWAY codes other than `NO_ERROR`, since HQ GOAWAY frames carry no code and [the control codec reports NO_ERROR](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/codec/HQControlCodec.cpp?lines=152).
  - Egress GOAWAY codes other than `NO_ERROR` ([HQSession](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/session/HQSession.cpp?lines=796-797)) and `PROTOCOL_ERROR` ([coro](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/coro/HTTPCoroSession.cpp?lines=1408-1412)).
  - Upstream ingress priority, since [only clients send PRIORITY_UPDATE](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/codec/HQControlCodec.cpp?lines=69-79).
- **Downstream:** ingress `syn_reply` and `push_promise`, and egress `syn_stream`. The filter counts all downstream ingress as `syn_stream` and all egress as `syn_reply` ([filter](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/stats/HTTPCodecStatsFilter.cpp?lines=33-43,102-115)), and codecs reject a PUSH_PROMISE sent to a server ([H2](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/codec/HTTP2Codec.cpp?lines=865-868), [HQ](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/codec/HQStreamCodec.cpp?lines=69-73)).
- **Upstream:** egress `syn_reply` and `push_promise`, since every push path checks `isDownstream()` ([HTTPSession](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/session/HTTPSession.cpp?lines=566), [coro](https://www.internalfb.com/code/fbsource/[335fa6e737ff6d5533dca2e6cd815835018d7d07]/fbcode/proxygen/lib/http/coro/HTTPCoroSession.cpp?lines=3732-3734)).

This also deletes the `ex_stream` counters from `TLHTTPCodecStats` and `AsyncHTTPCodecStats`; nothing calls `record{In,E}gressExStream`.

All 282 keys were 0 on every RevProxy task for the last 14 days ([receipts](https://www.internalfb.com/intern/paste/P2533245134/)). These are protocol facts, so EdgeTee, BigCache and reload_proxy2, which share `TLProxyStats`, lose the same counters. The one-argument constructor still creates everything. Recording a skipped counter anyway logs an error naming it, rate limited with `XLOG_EVERY_N(ERR, 1000)`, so a broken assumption shows up in the logs instead of silently dropping counts.

Fleet-wide for RevProxy this saves 150.1M per-task timeseries in ODS category 849 (`proxygen`) and 9.6M aggregate timeseries in category 272 (`traffic`), per `di.ods_gorilla_unique` on 2026-09-29. None of these keys are in category 2143 (`proxygen_customers`).

The H2 prospect stats tests checked that three of these counters stayed unchanged. This drops those checks.

Reviewed By: DavidLu1997

Differential Revision: D123138311

fbshipit-source-id: 44a18faed3c2ce974cd040622c9be9c34412af7c
This commit is contained in:
Darrin D'Mello
2026-10-06 19:15:08 -07:00
committed by meta-codesync[bot]
parent d8d72ad02c
commit 5649ee0f0b
2 changed files with 137 additions and 78 deletions

View File

@@ -8,6 +8,10 @@
#include <proxygen/lib/http/stats/HTTPCodecStats.h>
#include <folly/Conv.h>
#include <folly/Range.h>
#include <folly/logging/xlog.h>
using facebook::fb303::RATE;
using facebook::fb303::SUM;
@@ -33,47 +37,97 @@ static std::array<const char*, 14> kErrorStrings{
namespace proxygen {
namespace {
// TLHTTPCodecStats only skips counters that a session of its protocol and
// direction can never record, so recording one anyway means that reasoning no
// longer holds.
void addOrLogSkipped(folly::Optional<StatsWrapper::TLTimeseries>& counter,
const std::string& prefix,
folly::StringPiece name,
folly::StringPiece code = "") {
if (counter) {
counter->add(1);
return;
}
XLOG_EVERY_N(ERR, 1000) << "Recorded " << prefix << "_" << name << code
<< ", which TLHTTPCodecStats skipped as impossible "
"for this protocol and direction";
}
} // namespace
// TLHTTPCodecStats
TLHTTPCodecStats::TLHTTPCodecStats(const std::string& prefix)
: openConn_(prefix + "_conn.sum"),
TLHTTPCodecStats::TLHTTPCodecStats(
const std::string& prefix,
folly::Optional<CodecProtocol> protocol,
folly::Optional<TransportDirection> direction)
: prefix_(prefix),
openConn_(prefix + "_conn.sum"),
ingressSynStream_(prefix + "_ingress_syn_stream", SUM, RATE),
ingressSynReply_(prefix + "_ingress_syn_reply", SUM, RATE),
ingressPushPromise_(prefix + "_ingress_push_promise", SUM, RATE),
ingressExStream_(prefix + "_ingress_ex_stream", SUM, RATE),
ingressData_(prefix + "_ingress_data", SUM, RATE),
ingressRst_(prefix + "_ingress_rst", SUM, RATE),
ingressSettings_(prefix + "_ingress_settings", SUM, RATE),
ingressPingRequest_(prefix + "_ingress_ping_request", SUM, RATE),
ingressPingReply_(prefix + "_ingress_ping_reply", SUM, RATE),
ingressGoaway_(prefix + "_ingress_goaway", SUM, RATE),
ingressGoawayDrain_(prefix + "_ingress_goaway_drain", SUM, RATE),
ingressWindowUpdate_(prefix + "_ingress_window_update", SUM, RATE),
ingressPriority_(prefix + "_ingress_priority", SUM, RATE),
egressSynStream_(prefix + "_egress_syn_stream", SUM, RATE),
egressSynReply_(prefix + "_egress_syn_reply", SUM, RATE),
egressPushPromise_(prefix + "_egress_push_promise", SUM, RATE),
egressExStream_(prefix + "_egress_ex_stream", SUM, RATE),
egressData_(prefix + "_egress_data", SUM, RATE),
egressRst_(prefix + "_egress_rst", SUM, RATE),
egressSettings_(prefix + "_egress_settings", SUM, RATE),
egressPingRequest_(prefix + "_egress_ping_request", SUM, RATE),
egressPingReply_(prefix + "_egress_ping_reply", SUM, RATE),
egressGoaway_(prefix + "_egress_goaway", SUM, RATE),
egressGoawayDrain_(prefix + "_egress_goaway_drain", SUM, RATE),
egressWindowUpdate_(prefix + "_egress_window_update", SUM, RATE),
egressPriority_(prefix + "_egress_priority", SUM, RATE) {
ingressRstStatus_.reserve(kErrorStrings.size());
egressRstStatus_.reserve(kErrorStrings.size());
ingressGoawayStatus_.reserve(kErrorStrings.size());
egressGoawayStatus_.reserve(kErrorStrings.size());
for (auto errString : kErrorStrings) {
ingressRstStatus_.emplace_back(prefix + "_ingress_rst_" + errString, SUM);
egressRstStatus_.emplace_back(prefix + "_egress_rst_" + errString, SUM);
ingressGoawayStatus_.emplace_back(prefix + "_ingress_goaway_" + errString,
SUM);
egressGoawayStatus_.emplace_back(prefix + "_egress_goaway_" + errString,
SUM);
// HQ leaves RST_STREAM, PING and WINDOW_UPDATE to the QUIC transport, and
// its GOAWAY carries no error code: HQ codecs report every ingress GOAWAY as
// NO_ERROR, sessions only send NO_ERROR or (coro) PROTOCOL_ERROR, and only
// clients send PRIORITY_UPDATE.
const bool hq = protocol && isHQCodecProtocol(*protocol);
// Downstream codecs reject PUSH_PROMISE and only ever parse requests and
// send responses; upstream sessions only send requests and never push.
const bool downstream = direction == TransportDirection::DOWNSTREAM;
const bool upstream = direction == TransportDirection::UPSTREAM;
if (!downstream) {
ingressSynReply_.emplace(prefix + "_ingress_syn_reply", SUM, RATE);
ingressPushPromise_.emplace(prefix + "_ingress_push_promise", SUM, RATE);
egressSynStream_.emplace(prefix + "_egress_syn_stream", SUM, RATE);
}
if (!upstream) {
egressSynReply_.emplace(prefix + "_egress_syn_reply", SUM, RATE);
egressPushPromise_.emplace(prefix + "_egress_push_promise", SUM, RATE);
}
if (!(hq && upstream)) {
ingressPriority_.emplace(prefix + "_ingress_priority", SUM, RATE);
}
if (!hq) {
ingressRst_.emplace(prefix + "_ingress_rst", SUM, RATE);
ingressPingRequest_.emplace(prefix + "_ingress_ping_request", SUM, RATE);
ingressPingReply_.emplace(prefix + "_ingress_ping_reply", SUM, RATE);
ingressWindowUpdate_.emplace(prefix + "_ingress_window_update", SUM, RATE);
egressRst_.emplace(prefix + "_egress_rst", SUM, RATE);
egressPingRequest_.emplace(prefix + "_egress_ping_request", SUM, RATE);
egressPingReply_.emplace(prefix + "_egress_ping_reply", SUM, RATE);
egressWindowUpdate_.emplace(prefix + "_egress_window_update", SUM, RATE);
}
ingressRstStatus_.resize(kErrorStrings.size());
egressRstStatus_.resize(kErrorStrings.size());
ingressGoawayStatus_.resize(kErrorStrings.size());
egressGoawayStatus_.resize(kErrorStrings.size());
for (size_t i = 0; i < kErrorStrings.size(); ++i) {
const std::string errString = kErrorStrings[i];
const auto code = ErrorCode(i);
if (!hq) {
ingressRstStatus_[i].emplace(
folly::to<std::string>(prefix, "_ingress_rst_", errString), SUM);
egressRstStatus_[i].emplace(
folly::to<std::string>(prefix, "_egress_rst_", errString), SUM);
}
if (!hq || code == ErrorCode::NO_ERROR) {
ingressGoawayStatus_[i].emplace(
folly::to<std::string>(prefix, "_ingress_goaway_", errString), SUM);
}
if (!hq || code == ErrorCode::NO_ERROR ||
code == ErrorCode::PROTOCOL_ERROR) {
egressGoawayStatus_[i].emplace(
folly::to<std::string>(prefix, "_egress_goaway_", errString), SUM);
}
}
}
@@ -84,34 +138,32 @@ void TLHTTPCodecStats::recordIngressSynStream() {
ingressSynStream_.add(1);
}
void TLHTTPCodecStats::recordIngressSynReply() {
ingressSynReply_.add(1);
addOrLogSkipped(ingressSynReply_, prefix_, "ingress_syn_reply");
}
void TLHTTPCodecStats::recordIngressPushPromise() {
ingressPushPromise_.add(1);
}
void TLHTTPCodecStats::recordIngressExStream() {
ingressExStream_.add(1);
addOrLogSkipped(ingressPushPromise_, prefix_, "ingress_push_promise");
}
void TLHTTPCodecStats::recordIngressData() {
ingressData_.add(1);
}
void TLHTTPCodecStats::recordIngressRst(ErrorCode statusCode) {
ingressRst_.add(1);
addOrLogSkipped(ingressRst_, prefix_, "ingress_rst");
auto index = uint32_t(statusCode);
if (index >= kErrorStrings.size()) {
LOG(ERROR) << "Unknown ingress reset status code=" << index;
index = (uint32_t)ErrorCode::PROTOCOL_ERROR;
}
ingressRstStatus_[index].add(1);
addOrLogSkipped(
ingressRstStatus_[index], prefix_, "ingress_rst_", kErrorStrings[index]);
}
void TLHTTPCodecStats::recordIngressSettings() {
ingressSettings_.add(1);
}
void TLHTTPCodecStats::recordIngressPingRequest() {
ingressPingRequest_.add(1);
addOrLogSkipped(ingressPingRequest_, prefix_, "ingress_ping_request");
}
void TLHTTPCodecStats::recordIngressPingReply() {
ingressPingReply_.add(1);
addOrLogSkipped(ingressPingReply_, prefix_, "ingress_ping_reply");
}
void TLHTTPCodecStats::recordIngressGoaway(ErrorCode statusCode) {
ingressGoaway_.add(1);
@@ -120,49 +172,50 @@ void TLHTTPCodecStats::recordIngressGoaway(ErrorCode statusCode) {
LOG(ERROR) << "Unknown ingress goaway status code=" << index;
index = (uint32_t)ErrorCode::PROTOCOL_ERROR;
}
ingressGoawayStatus_[index].add(1);
addOrLogSkipped(ingressGoawayStatus_[index],
prefix_,
"ingress_goaway_",
kErrorStrings[index]);
}
void TLHTTPCodecStats::recordIngressGoawayDrain() {
ingressGoawayDrain_.add(1);
}
void TLHTTPCodecStats::recordIngressWindowUpdate() {
ingressWindowUpdate_.add(1);
addOrLogSkipped(ingressWindowUpdate_, prefix_, "ingress_window_update");
}
void TLHTTPCodecStats::recordIngressPriority() {
ingressPriority_.add(1);
addOrLogSkipped(ingressPriority_, prefix_, "ingress_priority");
}
void TLHTTPCodecStats::recordEgressSynStream() {
egressSynStream_.add(1);
addOrLogSkipped(egressSynStream_, prefix_, "egress_syn_stream");
}
void TLHTTPCodecStats::recordEgressSynReply() {
egressSynReply_.add(1);
addOrLogSkipped(egressSynReply_, prefix_, "egress_syn_reply");
}
void TLHTTPCodecStats::recordEgressPushPromise() {
egressPushPromise_.add(1);
}
void TLHTTPCodecStats::recordEgressExStream() {
egressExStream_.add(1);
addOrLogSkipped(egressPushPromise_, prefix_, "egress_push_promise");
}
void TLHTTPCodecStats::recordEgressData() {
egressData_.add(1);
}
void TLHTTPCodecStats::recordEgressRst(ErrorCode statusCode) {
egressRst_.add(1);
addOrLogSkipped(egressRst_, prefix_, "egress_rst");
auto index = uint32_t(statusCode);
if (index >= kErrorStrings.size()) {
LOG(ERROR) << "Unknown egress reset status code=" << index;
index = (uint32_t)ErrorCode::PROTOCOL_ERROR;
}
egressRstStatus_[index].add(1);
addOrLogSkipped(
egressRstStatus_[index], prefix_, "egress_rst_", kErrorStrings[index]);
}
void TLHTTPCodecStats::recordEgressSettings() {
egressSettings_.add(1);
}
void TLHTTPCodecStats::recordEgressPingRequest() {
egressPingRequest_.add(1);
addOrLogSkipped(egressPingRequest_, prefix_, "egress_ping_request");
}
void TLHTTPCodecStats::recordEgressPingReply() {
egressPingReply_.add(1);
addOrLogSkipped(egressPingReply_, prefix_, "egress_ping_reply");
}
void TLHTTPCodecStats::recordEgressGoaway(ErrorCode statusCode) {
egressGoaway_.add(1);
@@ -171,13 +224,16 @@ void TLHTTPCodecStats::recordEgressGoaway(ErrorCode statusCode) {
LOG(ERROR) << "Unknown egress goaway status code=" << index;
index = (uint32_t)ErrorCode::PROTOCOL_ERROR;
}
egressGoawayStatus_[index].add(1);
addOrLogSkipped(egressGoawayStatus_[index],
prefix_,
"egress_goaway_",
kErrorStrings[index]);
}
void TLHTTPCodecStats::recordEgressGoawayDrain() {
egressGoawayDrain_.add(1);
}
void TLHTTPCodecStats::recordEgressWindowUpdate() {
egressWindowUpdate_.add(1);
addOrLogSkipped(egressWindowUpdate_, prefix_, "egress_window_update");
}
void TLHTTPCodecStats::recordEgressPriority() {
egressPriority_.add(1);

View File

@@ -8,7 +8,10 @@
#pragma once
#include <folly/Optional.h>
#include <proxygen/lib/http/codec/CodecProtocol.h>
#include <proxygen/lib/http/codec/ErrorCode.h>
#include <proxygen/lib/http/codec/TransportDirection.h>
#include <proxygen/lib/stats/StatsWrapper.h>
#include <string>
@@ -27,7 +30,6 @@ class HTTPCodecStats {
virtual void recordIngressSynStream() = 0;
virtual void recordIngressSynReply() = 0;
virtual void recordIngressPushPromise() = 0;
virtual void recordIngressExStream() = 0;
virtual void recordIngressData() = 0;
virtual void recordIngressRst(ErrorCode statusCode) = 0;
virtual void recordIngressSettings() = 0;
@@ -41,7 +43,6 @@ class HTTPCodecStats {
virtual void recordEgressSynStream() = 0;
virtual void recordEgressSynReply() = 0;
virtual void recordEgressPushPromise() = 0;
virtual void recordEgressExStream() = 0;
virtual void recordEgressData() = 0;
virtual void recordEgressRst(ErrorCode statusCode) = 0;
virtual void recordEgressSettings() = 0;
@@ -58,7 +59,12 @@ class HTTPCodecStats {
*/
class TLHTTPCodecStats : public HTTPCodecStats {
public:
explicit TLHTTPCodecStats(const std::string& prefix);
// Given the session's protocol and direction, skips the counters such a
// session can never record.
explicit TLHTTPCodecStats(
const std::string& prefix,
folly::Optional<CodecProtocol> protocol = folly::none,
folly::Optional<TransportDirection> direction = folly::none);
explicit TLHTTPCodecStats(const TLHTTPCodecStats&) = delete;
TLHTTPCodecStats& operator=(const TLHTTPCodecStats&) = delete;
~TLHTTPCodecStats() override = default;
@@ -68,7 +74,6 @@ class TLHTTPCodecStats : public HTTPCodecStats {
void recordIngressSynStream() override;
void recordIngressSynReply() override;
void recordIngressPushPromise() override;
void recordIngressExStream() override;
void recordIngressData() override;
void recordIngressRst(ErrorCode statusCode) override;
void recordIngressSettings() override;
@@ -82,7 +87,6 @@ class TLHTTPCodecStats : public HTTPCodecStats {
void recordEgressSynStream() override;
void recordEgressSynReply() override;
void recordEgressPushPromise() override;
void recordEgressExStream() override;
void recordEgressData() override;
void recordEgressRst(ErrorCode statusCode) override;
void recordEgressSettings() override;
@@ -94,37 +98,36 @@ class TLHTTPCodecStats : public HTTPCodecStats {
void recordEgressPriority() override;
private:
std::string prefix_;
StatsWrapper::TLCounter openConn_;
StatsWrapper::TLTimeseries ingressSynStream_;
StatsWrapper::TLTimeseries ingressSynReply_;
StatsWrapper::TLTimeseries ingressPushPromise_;
StatsWrapper::TLTimeseries ingressExStream_;
folly::Optional<StatsWrapper::TLTimeseries> ingressSynReply_;
folly::Optional<StatsWrapper::TLTimeseries> ingressPushPromise_;
StatsWrapper::TLTimeseries ingressData_;
StatsWrapper::TLTimeseries ingressRst_;
std::vector<StatsWrapper::TLTimeseries> ingressRstStatus_;
folly::Optional<StatsWrapper::TLTimeseries> ingressRst_;
std::vector<folly::Optional<StatsWrapper::TLTimeseries>> ingressRstStatus_;
StatsWrapper::TLTimeseries ingressSettings_;
StatsWrapper::TLTimeseries ingressPingRequest_;
StatsWrapper::TLTimeseries ingressPingReply_;
folly::Optional<StatsWrapper::TLTimeseries> ingressPingRequest_;
folly::Optional<StatsWrapper::TLTimeseries> ingressPingReply_;
StatsWrapper::TLTimeseries ingressGoaway_;
StatsWrapper::TLTimeseries ingressGoawayDrain_;
std::vector<StatsWrapper::TLTimeseries> ingressGoawayStatus_;
StatsWrapper::TLTimeseries ingressWindowUpdate_;
StatsWrapper::TLTimeseries ingressPriority_;
std::vector<folly::Optional<StatsWrapper::TLTimeseries>> ingressGoawayStatus_;
folly::Optional<StatsWrapper::TLTimeseries> ingressWindowUpdate_;
folly::Optional<StatsWrapper::TLTimeseries> ingressPriority_;
StatsWrapper::TLTimeseries egressSynStream_;
StatsWrapper::TLTimeseries egressSynReply_;
StatsWrapper::TLTimeseries egressPushPromise_;
StatsWrapper::TLTimeseries egressExStream_;
folly::Optional<StatsWrapper::TLTimeseries> egressSynStream_;
folly::Optional<StatsWrapper::TLTimeseries> egressSynReply_;
folly::Optional<StatsWrapper::TLTimeseries> egressPushPromise_;
StatsWrapper::TLTimeseries egressData_;
StatsWrapper::TLTimeseries egressRst_;
std::vector<StatsWrapper::TLTimeseries> egressRstStatus_;
folly::Optional<StatsWrapper::TLTimeseries> egressRst_;
std::vector<folly::Optional<StatsWrapper::TLTimeseries>> egressRstStatus_;
StatsWrapper::TLTimeseries egressSettings_;
StatsWrapper::TLTimeseries egressPingRequest_;
StatsWrapper::TLTimeseries egressPingReply_;
folly::Optional<StatsWrapper::TLTimeseries> egressPingRequest_;
folly::Optional<StatsWrapper::TLTimeseries> egressPingReply_;
StatsWrapper::TLTimeseries egressGoaway_;
StatsWrapper::TLTimeseries egressGoawayDrain_;
std::vector<StatsWrapper::TLTimeseries> egressGoawayStatus_;
StatsWrapper::TLTimeseries egressWindowUpdate_;
std::vector<folly::Optional<StatsWrapper::TLTimeseries>> egressGoawayStatus_;
folly::Optional<StatsWrapper::TLTimeseries> egressWindowUpdate_;
StatsWrapper::TLTimeseries egressPriority_;
};