Skip to content

Commit

Permalink
Renamed methods in ExtMonitor.
Browse files Browse the repository at this point in the history
Signed-off-by: Christoph Pakulski <paker8848@gmail.com>
  • Loading branch information
cpakulski committed Jul 17, 2024
1 parent a90e1c0 commit 440628c
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 25 deletions.
6 changes: 3 additions & 3 deletions envoy/upstream/outlier_detection.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ class ExtMonitor {
virtual ~ExtMonitor() {}

// Method to report a result to extensions.
virtual void reportResult(const ExtResult&) PURE;
virtual void
setCallback(std::function<void(uint32_t, std::string, absl::optional<std::string>)>) PURE;
virtual void putResult(const ExtResult&) PURE;
virtual void setExtMonitorCallback(
std::function<void(uint32_t, std::string, absl::optional<std::string>)>) PURE;
virtual void reset() PURE;
};

Expand Down
6 changes: 3 additions & 3 deletions source/common/upstream/outlier_detection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void DetectorHostMonitorImpl::putHttpResponseCode(uint64_t response_code) {
// Wrap reported HTTP code into outlier detection extension's wrapper and forward
// it to configured extensions.
monitors_set_.forEach([response_code](const ExtMonitorPtr& monitor) {
monitor->reportResult(HttpCode(response_code));
monitor->putResult(HttpCode(response_code));
});
}
std::function<void(uint32_t, std::string, absl::optional<std::string>)>
Expand Down Expand Up @@ -214,7 +214,7 @@ void DetectorHostMonitorImpl::putResult(Result result, absl::optional<uint64_t>
// Only monitors "interested" in local origin event will process the result.
// Those not "interested" will ignore the call.
monitors_set_.forEach(
[result](const ExtMonitorPtr& monitor) { monitor->reportResult(LocalOriginEvent(result)); });
[result](const ExtMonitorPtr& monitor) { monitor->putResult(LocalOriginEvent(result)); });
}

void DetectorHostMonitorImpl::localOriginFailure() {
Expand Down Expand Up @@ -330,7 +330,7 @@ void DetectorConfig::createMonitorExtensions(
for (const auto& create_fn : monitor_create_fns_) {
auto extension = create_fn();
ASSERT(extension != nullptr);
extension->setCallback(callback);
extension->setExtMonitorCallback(callback);
ext_set.addMonitor(std::move(extension));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ bool LocalOriginEventsBucket::match(const ExtResult& event) const {
(local_origin_event.result() == Result::ExtOriginRequestSuccess)));
}

void ExtMonitorBase::reportResult(const ExtResult& result) {
void ExtMonitorBase::putResult(const ExtResult& result) {
if (buckets_.empty()) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ class ExtMonitorBase : public ExtMonitor {
ExtMonitorBase(const std::string& name, uint32_t enforce) : name_(name), enforce_(enforce) {}
ExtMonitorBase() = delete;
virtual ~ExtMonitorBase() {}
void reportResult(const ExtResult&) override;
void putResult(const ExtResult&) override;

void setCallback(
void setExtMonitorCallback(
std::function<void(uint32_t, std::string, absl::optional<std::string>)> callback) override {
callback_ = callback;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class MonitorTest : public testing::Test {
std::unique_ptr<MockMonitor> monitor_;
};

TEST_F(MonitorTest, NoBuckets) { monitor_->reportResult(HttpCode(200)); }
TEST_F(MonitorTest, NoBuckets) { monitor_->putResult(HttpCode(200)); }

TEST_F(MonitorTest, SingleBucketNotMatchingType) {
addBucket1();
Expand All @@ -117,7 +117,7 @@ TEST_F(MonitorTest, SingleBucketNotMatchingType) {

// Monitor is interested only in Results of ExtResultType::TEST
// type and here ExtResultType::LOCAL_ORIGIN is sent.
monitor_->reportResult(LocalOriginEvent(Result::ExtOriginRequestSuccess));
monitor_->putResult(LocalOriginEvent(Result::ExtOriginRequestSuccess));
}

// Type of the reported "result" matches the type of the
Expand All @@ -131,13 +131,14 @@ TEST_F(MonitorTest, SingleBucketNotMatchingResult) {
// implementation of the monitor, it may decrease or reset internal
// monitor's counters.
bool callback_called = false;
monitor_->setCallback([&callback_called](uint32_t, std::string, absl::optional<std::string>) {
callback_called = true;
});
monitor_->setExtMonitorCallback(
[&callback_called](uint32_t, std::string, absl::optional<std::string>) {
callback_called = true;
});
EXPECT_CALL(*bucket1_, match(_)).WillOnce(Return(false));
EXPECT_CALL(*monitor_, onSuccess);

monitor_->reportResult(HttpCode(200));
monitor_->putResult(HttpCode(200));

ASSERT_FALSE(callback_called);
}
Expand All @@ -150,14 +151,15 @@ TEST_F(MonitorTest, SingleBucketMatchingResultNotTripped) {
// implementation of the monitor, it may increase internal
// monitor's counters and "trip" the monitor.
bool callback_called = false;
monitor_->setCallback([&callback_called](uint32_t, std::string, absl::optional<std::string>) {
callback_called = true;
});
monitor_->setExtMonitorCallback(
[&callback_called](uint32_t, std::string, absl::optional<std::string>) {
callback_called = true;
});
EXPECT_CALL(*bucket1_, match(_)).WillOnce(Return(true));
// Return that the monitor has not been tripped.
EXPECT_CALL(*monitor_, onError).WillOnce(Return(false));

monitor_->reportResult(HttpCode(200));
monitor_->putResult(HttpCode(200));

// Callback has not been called, because onError returned false,
// meaning that monitor has not tripped yet.
Expand All @@ -172,7 +174,7 @@ TEST_F(MonitorTest, SingleBucketMatchingResultTripped) {
// implementation of the monitor, it may increase internal
// monitor's counters and "trip" the monitor.
bool callback_called = false;
monitor_->setCallback(
monitor_->setExtMonitorCallback(
[&callback_called](uint32_t enforcing, std::string name, absl::optional<std::string>) {
callback_called = true;
ASSERT_EQ(name, monitor_name_);
Expand All @@ -184,7 +186,7 @@ TEST_F(MonitorTest, SingleBucketMatchingResultTripped) {
// After tripping, the monitor is reset
EXPECT_CALL(*monitor_, onReset);

monitor_->reportResult(HttpCode(200));
monitor_->putResult(HttpCode(200));

// Callback has been called, because onError returned true,
// meaning that monitor has tripped.
Expand All @@ -199,7 +201,7 @@ TEST_F(MonitorTest, TwoBucketsNotMatching) {
EXPECT_CALL(*bucket2_, match(_)).WillOnce(Return(false));
EXPECT_CALL(*monitor_, onSuccess);

monitor_->reportResult(HttpCode(200));
monitor_->putResult(HttpCode(200));
}

TEST_F(MonitorTest, TwoBucketsFirstMatching) {
Expand All @@ -211,15 +213,15 @@ TEST_F(MonitorTest, TwoBucketsFirstMatching) {
EXPECT_CALL(*bucket2_, match(_)).Times(0);
EXPECT_CALL(*monitor_, onError).WillOnce(Return(false));

monitor_->reportResult(HttpCode(200));
monitor_->putResult(HttpCode(200));
}

TEST_F(MonitorTest, TwoBucketsSecondMatching) {
addBucket1();
addBucket2();

bool callback_called = false;
monitor_->setCallback(
monitor_->setExtMonitorCallback(
[&callback_called](uint32_t enforcing, std::string name, absl::optional<std::string>) {
callback_called = true;
ASSERT_EQ(name, monitor_name_);
Expand All @@ -231,7 +233,7 @@ TEST_F(MonitorTest, TwoBucketsSecondMatching) {
// After tripping, the monitor is reset
EXPECT_CALL(*monitor_, onReset);

monitor_->reportResult(HttpCode(200));
monitor_->putResult(HttpCode(200));

// Callback has been called, because onError returned true,
// meaning that monitor has tripped.
Expand Down

0 comments on commit 440628c

Please sign in to comment.