Skip to content

Commit

Permalink
access_log: use AccessLogType::NotSet instead of default value (envoy…
Browse files Browse the repository at this point in the history
…proxy#27058)

* use AccessLogType::NotSet instead of default parameter value

* pass access log type parameter in wasm extension

Signed-off-by: ohadvano <ohadvano@gmail.com>
Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
  • Loading branch information
ohadvano authored and reskin89 committed Jul 11, 2023
1 parent e12cc80 commit 1dcd469
Show file tree
Hide file tree
Showing 27 changed files with 275 additions and 165 deletions.
3 changes: 1 addition & 2 deletions contrib/golang/filters/http/source/golang_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@ class Filter : public Http::StreamFilter,
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info,
Envoy::AccessLog::AccessLogType access_log_type =
Envoy::AccessLog::AccessLogType::NotSet) override;
Envoy::AccessLog::AccessLogType access_log_type) override;

void onStreamComplete() override {}

Expand Down
3 changes: 1 addition & 2 deletions envoy/access_log/access_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ class Instance {
virtual void log(const Http::RequestHeaderMap* request_headers,
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLogType access_log_type = AccessLogType::NotSet) PURE;
const StreamInfo::StreamInfo& stream_info, AccessLogType access_log_type) PURE;
};

using InstanceSharedPtr = std::shared_ptr<Instance>;
Expand Down
3 changes: 2 additions & 1 deletion source/common/quic/quic_stats_gatherer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ void QuicStatsGatherer::maybeDoDeferredLog(bool record_ack_timing) {
const Http::ResponseHeaderMap* response_headers = response_header_map_.get();
const Http::ResponseTrailerMap* response_trailers = response_trailer_map_.get();
for (const AccessLog::InstanceSharedPtr& log_handler : access_log_handlers_) {
log_handler->log(request_headers, response_headers, response_trailers, *stream_info_);
log_handler->log(request_headers, response_headers, response_trailers, *stream_info_,
AccessLog::AccessLogType::NotSet);
}
}

Expand Down
13 changes: 6 additions & 7 deletions source/extensions/access_loggers/common/access_log_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ImplBase : public AccessLog::Instance {
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override;
AccessLog::AccessLogType access_log_type) override;

private:
/**
Expand All @@ -41,12 +41,11 @@ class ImplBase : public AccessLog::Instance {
* @param stream_info supplies additional information about the request not
* contained in the request headers.
*/
virtual void
emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) PURE;
virtual void emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type) PURE;

AccessLog::FilterPtr filter_;
};
Expand Down
11 changes: 5 additions & 6 deletions source/extensions/access_loggers/common/file_access_log_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ class FileAccessLog : public Common::ImplBase {

private:
// Common::ImplBase
void
emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override;
void emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type) override;

AccessLog::AccessLogFileSharedPtr log_file_;
Formatter::FormatterPtr formatter_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ class HttpGrpcAccessLog : public Common::ImplBase {
};

// Common::ImplBase
void
emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override;
void emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type) override;

const HttpGrpcAccessLogConfigConstSharedPtr config_;
const ThreadLocal::SlotPtr tls_slot_;
Expand Down
11 changes: 5 additions & 6 deletions source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,11 @@ class TcpGrpcAccessLog : public Common::ImplBase {
};

// Common::ImplBase
void
emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override;
void emitLog(const Http::RequestHeaderMap& request_headers,
const Http::ResponseHeaderMap& response_headers,
const Http::ResponseTrailerMap& response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type) override;

const TcpGrpcAccessLogConfigConstSharedPtr config_;
const ThreadLocal::SlotPtr tls_slot_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class WasmAccessLog : public AccessLog::Instance {
}
if (handle->wasmHandle()) {
handle->wasmHandle()->wasm()->log(plugin_, request_headers, response_headers,
response_trailers, stream_info);
response_trailers, stream_info, access_log_type);
}
}

Expand Down
2 changes: 1 addition & 1 deletion source/extensions/common/wasm/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class Context : public proxy_wasm::ContextBase,
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override;
AccessLog::AccessLogType access_log_type) override;

uint32_t getLogLevel() override;

Expand Down
5 changes: 3 additions & 2 deletions source/extensions/common/wasm/wasm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,10 @@ ContextBase* Wasm::createVmContext() { return new Context(this); }
void Wasm::log(const PluginSharedPtr& plugin, const Http::RequestHeaderMap* request_headers,
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info) {
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type) {
auto context = getRootContext(plugin, true);
context->log(request_headers, response_headers, response_trailers, stream_info);
context->log(request_headers, response_headers, response_trailers, stream_info, access_log_type);
}

void Wasm::onStatsUpdate(const PluginSharedPtr& plugin, Envoy::Stats::MetricSnapshot& snapshot) {
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/common/wasm/wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Wasm : public WasmBase, Logger::Loggable<Logger::Id::wasm> {
void log(const PluginSharedPtr& plugin, const Http::RequestHeaderMap* request_headers,
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info);
const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType access_log_type);

void onStatsUpdate(const PluginSharedPtr& plugin, Envoy::Stats::MetricSnapshot& snapshot);

Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/http/composite/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Filter : public Http::StreamFilter,
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override {
AccessLog::AccessLogType access_log_type) override {
for (const auto& log : access_loggers_) {
log->log(request_headers, response_headers, response_trailers, stream_info, access_log_type);
}
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/http/tap/tap_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance {
const Http::ResponseHeaderMap* response_headers,
const Http::ResponseTrailerMap* response_trailers,
const StreamInfo::StreamInfo& stream_info,
AccessLog::AccessLogType access_log_type = AccessLog::AccessLogType::NotSet) override;
AccessLog::AccessLogType access_log_type) override;

private:
FilterConfigSharedPtr config_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ void ConnectionManager::emitLogEntry(const Http::RequestHeaderMap* request_heade
const Http::ResponseHeaderMap* response_headers,
const StreamInfo::StreamInfo& stream_info) {
for (const auto& access_log : config_.accessLogs()) {
access_log->log(request_headers, response_headers, nullptr, stream_info);
access_log->log(request_headers, response_headers, nullptr, stream_info,
AccessLog::AccessLogType::NotSet);
}
}

Expand Down
6 changes: 4 additions & 2 deletions source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ UdpProxyFilter::~UdpProxyFilter() {
if (!config_->proxyAccessLogs().empty()) {
fillProxyStreamInfo();
for (const auto& access_log : config_->proxyAccessLogs()) {
access_log->log(nullptr, nullptr, nullptr, udp_proxy_stats_.value());
access_log->log(nullptr, nullptr, nullptr, udp_proxy_stats_.value(),
AccessLog::AccessLogType::NotSet);
}
}
}
Expand Down Expand Up @@ -297,7 +298,8 @@ UdpProxyFilter::ActiveSession::~ActiveSession() {
if (!cluster_.filter_.config_->sessionAccessLogs().empty()) {
fillSessionStreamInfo();
for (const auto& access_log : cluster_.filter_.config_->sessionAccessLogs()) {
access_log->log(nullptr, nullptr, nullptr, udp_session_stats_.value());
access_log->log(nullptr, nullptr, nullptr, udp_session_stats_.value(),
AccessLog::AccessLogType::NotSet);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void ActiveStreamListenerBase::emitLogs(Network::ListenerConfig& config,
StreamInfo::StreamInfo& stream_info) {
stream_info.onRequestComplete();
for (const auto& access_log : config.accessLogs()) {
access_log->log(nullptr, nullptr, nullptr, stream_info);
access_log->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet);
}
}

Expand Down
Loading

0 comments on commit 1dcd469

Please sign in to comment.