Skip to content

Cleanup: add const for access_log_filter::evaluate function #11002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
htuch merged 3 commits into from
May 1, 2020
Merged

Cleanup: add const for access_log_filter::evaluate function #11002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f38883
add const for access_log_filter::evaluate function
qiwzhang Apr 29, 2020
06db47c
fix tests
qiwzhang Apr 30, 2020
c3bdf19
add mutable
qiwzhang Apr 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/envoy/access_log/access_log.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Filter {
virtual bool evaluate(const StreamInfo::StreamInfo& info,
const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) PURE;
const Http::ResponseTrailerMap& response_trailers) const PURE;
};

using FilterPtr = std::unique_ptr<Filter>;
Expand Down
26 changes: 15 additions & 11 deletions source/common/access_log/access_log_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ ComparisonFilter::ComparisonFilter(const envoy::config::accesslog::v3::Compariso
Runtime::Loader& runtime)
: config_(config), runtime_(runtime) {}

bool ComparisonFilter::compareAgainstValue(uint64_t lhs) {
bool ComparisonFilter::compareAgainstValue(uint64_t lhs) const {
uint64_t value = config_.value().default_value();

if (!config_.value().runtime_key().empty()) {
Expand Down Expand Up @@ -92,14 +92,15 @@ FilterPtr FilterFactory::fromProto(const envoy::config::accesslog::v3::AccessLog
bool TraceableRequestFilter::evaluate(const StreamInfo::StreamInfo& info,
const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap&,
const Http::ResponseTrailerMap&) {
const Http::ResponseTrailerMap&) const {
Tracing::Decision decision = Tracing::HttpTracerUtility::isTracing(info, request_headers);

return decision.traced && decision.reason == Tracing::Reason::ServiceForced;
}

bool StatusCodeFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap&,
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) {
const Http::ResponseHeaderMap&,
const Http::ResponseTrailerMap&) const {
if (!info.responseCode()) {
return compareAgainstValue(0ULL);
}
Expand All @@ -108,7 +109,8 @@ bool StatusCodeFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::
}

bool DurationFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap&,
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) {
const Http::ResponseHeaderMap&,
const Http::ResponseTrailerMap&) const {
absl::optional<std::chrono::nanoseconds> final = info.requestComplete();
ASSERT(final);

Expand All @@ -124,7 +126,8 @@ RuntimeFilter::RuntimeFilter(const envoy::config::accesslog::v3::RuntimeFilter&

bool RuntimeFilter::evaluate(const StreamInfo::StreamInfo& stream_info,
const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) {
const Http::ResponseHeaderMap&,
const Http::ResponseTrailerMap&) const {
auto rid_extension = stream_info.getRequestIDExtension();
uint64_t random_value;
if (use_independent_randomness_ ||
Expand Down Expand Up @@ -161,7 +164,7 @@ AndFilter::AndFilter(const envoy::config::accesslog::v3::AndFilter& config,
bool OrFilter::evaluate(const StreamInfo::StreamInfo& info,
const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) {
const Http::ResponseTrailerMap& response_trailers) const {
bool result = false;
for (auto& filter : filters_) {
result |= filter->evaluate(info, request_headers, response_headers, response_trailers);
Expand All @@ -177,7 +180,7 @@ bool OrFilter::evaluate(const StreamInfo::StreamInfo& info,
bool AndFilter::evaluate(const StreamInfo::StreamInfo& info,
const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) {
const Http::ResponseTrailerMap& response_trailers) const {
bool result = true;
for (auto& filter : filters_) {
result &= filter->evaluate(info, request_headers, response_headers, response_trailers);
Expand All @@ -192,7 +195,7 @@ bool AndFilter::evaluate(const StreamInfo::StreamInfo& info,

bool NotHealthCheckFilter::evaluate(const StreamInfo::StreamInfo& info,
const Http::RequestHeaderMap&, const Http::ResponseHeaderMap&,
const Http::ResponseTrailerMap&) {
const Http::ResponseTrailerMap&) const {
return !info.healthCheck();
}

Expand All @@ -201,7 +204,7 @@ HeaderFilter::HeaderFilter(const envoy::config::accesslog::v3::HeaderFilter& con

bool HeaderFilter::evaluate(const StreamInfo::StreamInfo&,
const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) {
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) const {
return Http::HeaderUtility::matchHeaders(request_headers, *header_data_);
}

Expand All @@ -217,7 +220,8 @@ ResponseFlagFilter::ResponseFlagFilter(
}

bool ResponseFlagFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap&,
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) {
const Http::ResponseHeaderMap&,
const Http::ResponseTrailerMap&) const {
if (configured_flags_ != 0) {
return info.intersectResponseFlags(configured_flags_);
}
Expand All @@ -234,7 +238,7 @@ GrpcStatusFilter::GrpcStatusFilter(const envoy::config::accesslog::v3::GrpcStatu

bool GrpcStatusFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap&,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) {
const Http::ResponseTrailerMap& response_trailers) const {

Grpc::Status::GrpcStatus status = Grpc::Status::WellKnownGrpcStatus::Unknown;
const auto& optional_status =
Expand Down
22 changes: 11 additions & 11 deletions source/common/access_log/access_log_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class ComparisonFilter : public Filter {
ComparisonFilter(const envoy::config::accesslog::v3::ComparisonFilter& config,
Runtime::Loader& runtime);

bool compareAgainstValue(uint64_t lhs);
bool compareAgainstValue(uint64_t lhs) const;

envoy::config::accesslog::v3::ComparisonFilter config_;
Runtime::Loader& runtime_;
Expand All @@ -60,7 +60,7 @@ class StatusCodeFilter : public ComparisonFilter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;
};

/**
Expand All @@ -75,7 +75,7 @@ class DurationFilter : public ComparisonFilter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;
};

/**
Expand Down Expand Up @@ -104,7 +104,7 @@ class AndFilter : public OperatorFilter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;
};

/**
Expand All @@ -119,7 +119,7 @@ class OrFilter : public OperatorFilter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;
};

/**
Expand All @@ -132,7 +132,7 @@ class NotHealthCheckFilter : public Filter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;
};

/**
Expand All @@ -143,7 +143,7 @@ class TraceableRequestFilter : public Filter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;
};

/**
Expand All @@ -157,7 +157,7 @@ class RuntimeFilter : public Filter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;

private:
Runtime::Loader& runtime_;
Expand All @@ -177,7 +177,7 @@ class HeaderFilter : public Filter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;

private:
const Http::HeaderUtility::HeaderDataPtr header_data_;
Expand All @@ -193,7 +193,7 @@ class ResponseFlagFilter : public Filter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;

private:
uint64_t configured_flags_{};
Expand All @@ -214,7 +214,7 @@ class GrpcStatusFilter : public Filter {
// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers) override;
const Http::ResponseTrailerMap& response_trailers) const override;

private:
GrpcStatusHashSet statuses_;
Expand Down
4 changes: 2 additions & 2 deletions test/common/access_log/access_log_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1317,7 +1317,7 @@ class SampleExtensionFilter : public Filter {

// AccessLog::Filter
bool evaluate(const StreamInfo::StreamInfo&, const Http::RequestHeaderMap&,
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) override {
const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&) const override {
if (current_++ == 0) {
return true;
}
Expand All @@ -1328,7 +1328,7 @@ class SampleExtensionFilter : public Filter {
}

private:
uint32_t current_ = 0;
mutable uint32_t current_ = 0;
uint32_t sample_rate_;
};

Expand Down
3 changes: 2 additions & 1 deletion test/mocks/access_log/mocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class MockFilter : public Filter {
MOCK_METHOD(bool, evaluate,
(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers));
const Http::ResponseTrailerMap& response_trailers),
(const));
};

class MockAccessLogManager : public AccessLogManager {
Expand Down