diff --git a/quic/api/Observer.h b/quic/api/Observer.h index 669abda2b..fb981d731 100644 --- a/quic/api/Observer.h +++ b/quic/api/Observer.h @@ -28,6 +28,56 @@ class QuicSocket; */ class Observer { public: + /** + * Observer configuration. + * + * Specifies events observer wants to receive. + */ + struct Config { + // following flags enable support for various callbacks. + // observer and socket lifecycle callbacks are always enabled. + bool evbEvents{false}; + bool appLimitedEvents{false}; + bool lossEvents{false}; + bool pmtuEvents{false}; + bool rttSamples{false}; + + /** + * Returns a config where all events are enabled. + */ + static Config getConfigAllEventsEnabled() { + Config config = {}; + config.evbEvents = true; + config.appLimitedEvents = true; + config.rttSamples = true; + config.lossEvents = true; + config.pmtuEvents = true; + return config; + } + }; + + /** + * Constructor for observer, uses default config (all callbacks disabled). + */ + Observer() : Observer(Config()) {} + + /** + * Constructor for observer. + * + * @param config Config, defaults to auxilary instrumentaton disabled. + */ + explicit Observer(const Config& observerConfig) + : observerConfig_(observerConfig) {} + + virtual ~Observer() = default; + + /** + * Returns observers configuration. + */ + const Config& getConfig() { + return observerConfig_; + } + struct LostPacket { explicit LostPacket( bool lostbytimeout, @@ -133,8 +183,6 @@ class Observer { ProbeSizeRaiserType probeSizeRaiserType; }; - virtual ~Observer() = default; - /** * observerAttach() will be invoked when an observer is added. * @@ -251,6 +299,10 @@ class Observer { virtual void pmtuUpperBoundDetected( QuicSocket*, /* socket */ const PMTUUpperBoundEvent& /* pmtuUpperBoundEvent */) {} + + protected: + // observer configuration; cannot be changed post instantiation + const Config observerConfig_; }; // Container for instrumentation observers. diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index c6a002ccc..114072c70 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -2742,7 +2742,9 @@ void QuicTransportBase::writeSocketData() { // notify via connection call and any observer callbacks connCallback_->onAppRateLimited(); for (const auto& cb : *observers_) { - cb->appRateLimited(this); + if (cb->getConfig().appLimitedEvents) { + cb->appRateLimited(this); + } } } } @@ -2948,7 +2950,9 @@ void QuicTransportBase::attachEventBase(folly::EventBase* evb) { updateWriteLooper(false); for (const auto& cb : *observers_) { - cb->evbAttach(this, evb_); + if (cb->getConfig().evbEvents) { + cb->evbAttach(this, evb_); + } } } @@ -2970,7 +2974,9 @@ void QuicTransportBase::detachEventBase() { writeLooper_->detachEventBase(); for (const auto& cb : *observers_) { - cb->evbDetach(this, evb_); + if (cb->getConfig().evbEvents) { + cb->evbDetach(this, evb_); + } } evb_ = nullptr; } diff --git a/quic/api/test/Mocks.h b/quic/api/test/Mocks.h index b62d1ff0f..829d4f540 100644 --- a/quic/api/test/Mocks.h +++ b/quic/api/test/Mocks.h @@ -309,6 +309,9 @@ class MockLoopDetectorCallback : public LoopDetectorCallback { class MockObserver : public Observer { public: + MockObserver() : Observer(Observer::Config()) {} + MockObserver(const Observer::Config& observerConfig) + : Observer(observerConfig) {} GMOCK_METHOD1_(, noexcept, , observerAttach, void(QuicSocket*)); GMOCK_METHOD1_(, noexcept, , observerDetach, void(QuicSocket*)); GMOCK_METHOD1_(, noexcept, , destroy, void(QuicSocket*)); diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 53c57e991..c4e835b74 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -3560,7 +3560,9 @@ TEST_F(QuicTransportImplTest, ObserverMultipleAttachDestroyTransport) { } TEST_F(QuicTransportImplTest, ObserverDetachAndAttachEvb) { - auto cb = std::make_unique>(); + Observer::Config config = {}; + config.evbEvents = true; + auto cb = std::make_unique>(config); folly::EventBase evb2; EXPECT_CALL(*cb, observerAttach(transport.get())); diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 430c1f994..67fb4fc79 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -370,7 +370,10 @@ TEST_F(QuicTransportTest, NotAppLimitedWithNoWritableBytesWithObservers) { return 0; })); - auto cb = std::make_unique>(); + Observer::Config config = {}; + config.appLimitedEvents = true; + auto cb = std::make_unique>(config); + EXPECT_CALL(*cb, observerAttach(transport_.get())); transport_->addObserver(cb.get()); @@ -401,7 +404,10 @@ TEST_F(QuicTransportTest, NotAppLimitedWithLargeBufferWithObservers) { EXPECT_CALL(*rawCongestionController, getWritableBytes()) .WillRepeatedly(Return(5000)); - auto cb = std::make_unique>(); + Observer::Config config = {}; + config.appLimitedEvents = true; + auto cb = std::make_unique>(config); + EXPECT_CALL(*cb, observerAttach(transport_.get())); transport_->addObserver(cb.get()); @@ -419,8 +425,10 @@ TEST_F(QuicTransportTest, NotAppLimitedWithLargeBufferWithObservers) { } TEST_F(QuicTransportTest, AppLimitedWithObservers) { - auto cb1 = std::make_unique>(); - auto cb2 = std::make_unique>(); + Observer::Config config = {}; + config.appLimitedEvents = true; + auto cb1 = std::make_unique>(config); + auto cb2 = std::make_unique>(config); EXPECT_CALL(*cb1, observerAttach(transport_.get())); EXPECT_CALL(*cb2, observerAttach(transport_.get())); transport_->addObserver(cb1.get()); diff --git a/quic/d6d/QuicD6DStateFunctions.cpp b/quic/d6d/QuicD6DStateFunctions.cpp index 7a0d26b62..f0f0867c1 100644 --- a/quic/d6d/QuicD6DStateFunctions.cpp +++ b/quic/d6d/QuicD6DStateFunctions.cpp @@ -39,7 +39,9 @@ static TimePoint reportUpperBound(QuicConnectionStateBase& conn) { for (const auto& observer : *(conn.observers)) { conn.pendingCallbacks.emplace_back( [observer, upperBoundEvent](QuicSocket* qSocket) { - observer->pmtuUpperBoundDetected(qSocket, upperBoundEvent); + if (observer->getConfig().pmtuEvents) { + observer->pmtuUpperBoundDetected(qSocket, upperBoundEvent); + } }); } } @@ -73,7 +75,9 @@ static TimePoint reportBlackhole( for (const auto& observer : *(conn.observers)) { conn.pendingCallbacks.emplace_back( [observer, blackholeEvent](QuicSocket* qSocket) { - observer->pmtuBlackholeDetected(qSocket, blackholeEvent); + if (observer->getConfig().pmtuEvents) { + observer->pmtuBlackholeDetected(qSocket, blackholeEvent); + } }); } } diff --git a/quic/d6d/test/QuicD6DStateFunctionsTest.cpp b/quic/d6d/test/QuicD6DStateFunctionsTest.cpp index e894cdb00..cd05cb4a8 100644 --- a/quic/d6d/test/QuicD6DStateFunctionsTest.cpp +++ b/quic/d6d/test/QuicD6DStateFunctionsTest.cpp @@ -191,7 +191,9 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInBase) { const uint16_t expectPMTU = 1400; auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; @@ -232,7 +234,9 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingOne) { const uint16_t expectPMTU = 1400; auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; @@ -274,7 +278,9 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingMax) { const uint16_t oversize = 1500; auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; @@ -326,7 +332,9 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInError) { QuicConnectionStateBase conn(QuicNodeType::Server); auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; @@ -367,7 +375,9 @@ TEST_F(QuicD6DStateFunctionsTest, BlackholeInSearching) { QuicConnectionStateBase conn(QuicNodeType::Server); auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; @@ -431,7 +441,9 @@ TEST_F(QuicD6DStateFunctionsTest, BlackholeInSearchComplete) { QuicConnectionStateBase conn(QuicNodeType::Server); auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; @@ -494,7 +506,9 @@ TEST_F(QuicD6DStateFunctionsTest, ReachMaxPMTU) { QuicConnectionStateBase conn(QuicNodeType::Server); auto& d6d = conn.d6d; auto now = Clock::now(); - auto mockObserver = std::make_unique>(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique>(config); auto observers = std::make_shared(); observers->emplace_back(mockObserver.get()); conn.observers = observers; diff --git a/quic/loss/QuicLossFunctions.h b/quic/loss/QuicLossFunctions.h index 4438f1b83..b79d69634 100644 --- a/quic/loss/QuicLossFunctions.h +++ b/quic/loss/QuicLossFunctions.h @@ -300,7 +300,9 @@ folly::Optional detectLossPackets( for (const auto& observer : *(conn.observers)) { conn.pendingCallbacks.emplace_back( [observer, observerLossEvent](QuicSocket* qSocket) { - observer->packetLossDetected(qSocket, observerLossEvent); + if (observer->getConfig().lossEvents) { + observer->packetLossDetected(qSocket, observerLossEvent); + } }); } } diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 7e421eb1e..6a9895a0c 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -1876,7 +1876,9 @@ TEST_F(QuicLossFunctionsTest, PersistentCongestion) { TEST_F(QuicLossFunctionsTest, TestReorderLossObserverCallback) { auto observers = std::make_shared(); - auto ib = MockObserver(); + Observer::Config config = {}; + config.lossEvents = true; + auto ib = MockObserver(config); auto conn = createConn(); // Register 1 life cycle observer observers->emplace_back(&ib); @@ -1934,7 +1936,9 @@ TEST_F(QuicLossFunctionsTest, TestReorderLossObserverCallback) { TEST_F(QuicLossFunctionsTest, TestTimeoutLossObserverCallback) { auto observers = std::make_shared(); - auto ib = MockObserver(); + Observer::Config config = {}; + config.lossEvents = true; + auto ib = MockObserver(config); auto conn = createConn(); // Register 1 life cycle observer observers->emplace_back(&ib); @@ -1993,7 +1997,9 @@ TEST_F(QuicLossFunctionsTest, TestTimeoutLossObserverCallback) { TEST_F(QuicLossFunctionsTest, TestTimeoutAndReorderLossObserverCallback) { auto observers = std::make_shared(); - auto ib = MockObserver(); + Observer::Config config = {}; + config.lossEvents = true; + auto ib = MockObserver(config); auto conn = createConn(); // Register 1 life cycle observer observers->emplace_back(&ib); diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index 184edd4f8..e5c54f186 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -563,7 +563,9 @@ void QuicServerTransport::maybeStartD6DProbing() { conn_->pendingEvents.d6d.sendProbeDelay = kDefaultD6DKickStartDelay; QUIC_STATS(conn_->statsCallback, onConnectionD6DStarted); for (const auto& cb : *(conn_->observers)) { - cb->pmtuProbingStarted(this); + if (cb->getConfig().pmtuEvents) { + cb->pmtuProbingStarted(this); + } } } } diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 19cffedbc..8e3127684 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -4249,7 +4249,9 @@ TEST_P( } TEST_P(QuicServerTransportHandshakeTest, TestD6DStartCallback) { - auto mockObserver = std::make_unique(); + Observer::Config config = {}; + config.pmtuEvents = true; + auto mockObserver = std::make_unique(config); server->addObserver(mockObserver.get()); // Set oneRttReader so that maybeStartD6DPriobing passes its check auto codec = std::make_unique(QuicNodeType::Server); diff --git a/quic/state/AckHandlers.cpp b/quic/state/AckHandlers.cpp index 1efdce505..54295ba97 100644 --- a/quic/state/AckHandlers.cpp +++ b/quic/state/AckHandlers.cpp @@ -149,7 +149,9 @@ void processAckFrame( for (const auto& observer : *(conn.observers)) { conn.pendingCallbacks.emplace_back( [observer, packetRTT](QuicSocket* qSocket) { - observer->rttSampleGenerated(qSocket, packetRTT); + if (observer->getConfig().rttSamples) { + observer->rttSampleGenerated(qSocket, packetRTT); + } }); } updateRtt(conn, rttSample, frame.ackDelay); diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index 701945399..57886a092 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -1121,7 +1121,10 @@ TEST_P(AckHandlersTest, TestRTTPacketObserverCallback) { conn.congestionController = std::move(mockCongestionController); // Register 1 observer - auto ib = MockObserver(); + Observer::Config config = {}; + config.rttSamples = true; + auto ib = MockObserver(config); + auto observers = std::make_shared(); observers->emplace_back(&ib); conn.observers = observers; diff --git a/quic/tools/tperf/tperf.cpp b/quic/tools/tperf/tperf.cpp index a68ca9eb8..0ea2c3d4c 100644 --- a/quic/tools/tperf/tperf.cpp +++ b/quic/tools/tperf/tperf.cpp @@ -133,6 +133,7 @@ ProbeSizeRaiserType parseRaiserType(uint32_t type) { class TPerfObserver : public Observer { public: + TPerfObserver(const Observer::Config& config) : Observer(config) {} void appRateLimited(QuicSocket* /* socket */) override { if (FLAGS_log_app_rate_limited) { LOG(INFO) << "appRateLimited detected"; @@ -186,7 +187,16 @@ class TPerfObserver : public Observer { */ class TPerfAcceptObserver : public AcceptObserver { public: - TPerfAcceptObserver() : tperfObserver_(std::make_unique()) {} + TPerfAcceptObserver() { + // Create an observer config, only enabling events we are interested in + // receiving. + Observer::Config config = {}; + config.appLimitedEvents = true; + config.pmtuEvents = true; + config.rttSamples = true; + config.lossEvents = true; + tperfObserver_ = std::make_unique(config); + } void accept(QuicTransportBase* transport) noexcept override { transport->addObserver(tperfObserver_.get());