Skip to content

Commit 1b50084

Browse files
committed
feat core: remove unused statistics in HTTP handler
Tests: протестировано CI commit_hash:1973e5b43781cd26091faac201b88120ea768688
1 parent 38df0fd commit 1b50084

File tree

11 files changed

+2
-77
lines changed

11 files changed

+2
-77
lines changed

core/include/userver/server/handlers/http_handler_base.hpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ class Auth;
3333
namespace server::handlers {
3434

3535
class HttpHandlerStatistics;
36-
class HttpRequestStatistics;
3736
class HttpHandlerMethodStatistics;
3837
class HttpHandlerStatisticsScope;
3938

@@ -75,9 +74,6 @@ class HttpHandlerBase : public HandlerBase {
7574
/// @cond
7675
// For internal use only.
7776
HttpHandlerStatistics& GetHandlerStatistics() const;
78-
79-
// For internal use only.
80-
HttpRequestStatistics& GetRequestStatistics() const;
8177
/// @endcond
8278

8379
/// Override it if you need a custom logging level for messages about finish
@@ -217,7 +213,6 @@ class HttpHandlerBase : public HandlerBase {
217213
std::unordered_map<int, logging::Level> log_level_for_status_codes_;
218214

219215
std::unique_ptr<HttpHandlerStatistics> handler_statistics_;
220-
std::unique_ptr<HttpRequestStatistics> request_statistics_;
221216

222217
bool set_response_server_hostname_;
223218
bool is_body_streamed_;

core/include/userver/server/http/http_request.hpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
USERVER_NAMESPACE_BEGIN
2222

2323
namespace server::handlers {
24-
class HttpRequestStatistics;
2524
class HttpHandlerBase;
2625
} // namespace server::handlers
2726

@@ -268,8 +267,6 @@ class HttpRequest final {
268267
/// @endcond
269268

270269
private:
271-
void AccountResponseTime();
272-
273270
void SetPathArgs(std::vector<std::pair<std::string, std::string>> args);
274271

275272
void SetHttpHandler(const handlers::HttpHandlerBase& handler);
@@ -278,8 +275,6 @@ class HttpRequest final {
278275
void SetTaskProcessor(engine::TaskProcessor& task_processor);
279276
engine::TaskProcessor* GetTaskProcessor() const;
280277

281-
void SetHttpHandlerStatistics(handlers::HttpRequestStatistics&);
282-
283278
// HTTP/2.0 only
284279
void SetResponseStreamId(std::int32_t);
285280
void SetStreamProducer(impl::Http2StreamEventProducer&& producer);

core/include/userver/server/http/http_request_builder.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ class HttpRequestBuilder final {
5555

5656
HttpRequestBuilder& SetTaskProcessor(engine::TaskProcessor& task_processor);
5757

58-
HttpRequestBuilder& SetHttpHandlerStatistics(handlers::HttpRequestStatistics& stats);
59-
6058
// TODO: remove?
6159
HttpRequestBuilder& SetStreamProducer(impl::Http2StreamEventProducer&& producer);
6260

core/src/server/handlers/http_handler_base.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ namespace {
5050

5151
const std::string kHostname = hostinfo::blocking::GetRealHostName();
5252

53-
// "request" is redundant: https://st.yandex-team.ru/TAXICOMMON-1793
54-
// set to 1 if you need server metrics
55-
constexpr bool kIncludeServerHttpMetrics = false;
56-
5753
std::vector<http::HttpMethod> InitAllowedMethods(const HandlerConfig& config) {
5854
std::vector<http::HttpMethod> allowed_methods;
5955
std::vector<std::string> methods;
@@ -138,7 +134,6 @@ HttpHandlerBase::HttpHandlerBase(
138134
log_level_for_status_codes_(ParseStatusCodesLogLevel(config["status-codes-log-level"]
139135
.As<std::unordered_map<std::string, std::string>>({}))),
140136
handler_statistics_(std::make_unique<HttpHandlerStatistics>()),
141-
request_statistics_(std::make_unique<HttpRequestStatistics>()),
142137
is_body_streamed_(config["response-body-stream"].As<bool>(false))
143138
{
144139
if (allowed_methods_.empty()) {
@@ -187,12 +182,7 @@ HttpHandlerBase::HttpHandlerBase(
187182
RegisterWriterScope(
188183
context,
189184
std::move(prefix),
190-
[this](utils::statistics::Writer& result) {
191-
FormatStatistics(result["handler"], *handler_statistics_);
192-
if constexpr (kIncludeServerHttpMetrics) {
193-
FormatStatistics(result["request"], *request_statistics_);
194-
}
195-
},
185+
[this](utils::statistics::Writer& result) { FormatStatistics(result["handler"], *handler_statistics_); },
196186
std::move(labels)
197187
);
198188
}
@@ -299,8 +289,6 @@ const std::vector<http::HttpMethod>& HttpHandlerBase::GetAllowedMethods() const
299289

300290
HttpHandlerStatistics& HttpHandlerBase::GetHandlerStatistics() const { return *handler_statistics_; }
301291

302-
HttpRequestStatistics& HttpHandlerBase::GetRequestStatistics() const { return *request_statistics_; }
303-
304292
logging::Level HttpHandlerBase::GetLogLevelForResponseStatus(http::HttpStatus status) const {
305293
const auto status_code = static_cast<int>(status);
306294
const auto* const level = utils::FindOrNullptr(log_level_for_status_codes_, status_code);

core/src/server/handlers/http_handler_base_statistics.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ void DumpMetric(utils::statistics::Writer& writer, const HttpHandlerStatisticsSn
7575
writer.ValueWithLabels(HttpHandlerStatisticsHelper{stats}, {"version", "2"});
7676
}
7777

78-
void HttpRequestMethodStatistics::Account(const HttpRequestStatisticsEntry& stats) noexcept {
79-
timings_.GetCurrentCounter().Account(stats.timing.count());
80-
}
81-
8278
bool IsOkMethod(http::HttpMethod method) noexcept {
8379
return static_cast<std::size_t>(method) <= http::kHandlerMethodsMax;
8480
}

core/src/server/handlers/http_handler_base_statistics.hpp

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -82,26 +82,6 @@ struct HttpHandlerStatisticsSnapshot final {
8282

8383
void DumpMetric(utils::statistics::Writer& writer, const HttpHandlerStatisticsSnapshot& stats);
8484

85-
// Statistics for a single request from the overall server or the external
86-
// client perspective. Includes the time spent in queue.
87-
struct HttpRequestStatisticsEntry final {
88-
std::chrono::milliseconds timing;
89-
};
90-
91-
class HttpRequestMethodStatistics final {
92-
public:
93-
using Snapshot = void;
94-
95-
void Account(const HttpRequestStatisticsEntry& stats) noexcept;
96-
97-
using Percentile = utils::statistics::Percentile<2048, unsigned int, 120>;
98-
99-
Percentile GetTimings() const { return timings_.GetStatsForPeriod(); }
100-
101-
private:
102-
utils::statistics::RecentPeriod<Percentile, Percentile, utils::datetime::SteadyClock> timings_;
103-
};
104-
10585
bool IsOkMethod(http::HttpMethod method) noexcept;
10686

10787
std::size_t HttpMethodToIndex(http::HttpMethod method) noexcept;
@@ -123,8 +103,6 @@ class ByMethodStatistics {
123103

124104
class HttpHandlerStatistics final : public ByMethodStatistics<HttpHandlerMethodStatistics> {};
125105

126-
class HttpRequestStatistics final : public ByMethodStatistics<HttpRequestMethodStatistics> {};
127-
128106
class HttpHandlerStatisticsScope final {
129107
public:
130108
HttpHandlerStatisticsScope(

core/src/server/http/http_request.cpp

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -311,14 +311,6 @@ void HttpRequest::SetPathArgs(std::vector<std::pair<std::string, std::string>> a
311311
}
312312
}
313313

314-
void HttpRequest::AccountResponseTime() {
315-
if (pimpl_->request_statistics) {
316-
auto timing = std::chrono::duration_cast<
317-
std::chrono::milliseconds>(pimpl_->finish_send_response_time - pimpl_->start_time);
318-
pimpl_->request_statistics->ForMethod(GetMethod()).Account(handlers::HttpRequestStatisticsEntry{timing});
319-
}
320-
}
321-
322314
void HttpRequest::MarkAsInternalServerError() const {
323315
// TODO : refactor, this being here is a bit ridiculous
324316
pimpl_->response.SetStatus(http::HttpStatus::kInternalServerError);
@@ -334,10 +326,6 @@ void HttpRequest::SetTaskProcessor(engine::TaskProcessor& task_processor) { pimp
334326

335327
engine::TaskProcessor* HttpRequest::GetTaskProcessor() const { return pimpl_->task_processor; }
336328

337-
void HttpRequest::SetHttpHandlerStatistics(handlers::HttpRequestStatistics& stats) {
338-
pimpl_->request_statistics = &stats;
339-
}
340-
341329
void HttpRequest::SetResponseStreamId(std::int32_t stream_id) { pimpl_->response.SetStreamId(stream_id); }
342330

343331
void HttpRequest::SetStreamProducer(impl::Http2StreamEventProducer&& producer) {
@@ -358,10 +346,7 @@ void HttpRequest::SetStartSendResponseTime() noexcept {
358346
pimpl_->start_send_response_time = std::chrono::steady_clock::now();
359347
}
360348

361-
void HttpRequest::SetFinishSendResponseTime() {
362-
pimpl_->finish_send_response_time = std::chrono::steady_clock::now();
363-
AccountResponseTime();
364-
}
349+
void HttpRequest::SetFinishSendResponseTime() { pimpl_->finish_send_response_time = std::chrono::steady_clock::now(); }
365350

366351
void HttpRequest::WriteAccessLogs(
367352
const logging::TextLoggerPtr& logger_access,

core/src/server/http/http_request_builder.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,6 @@ HttpRequestBuilder& HttpRequestBuilder::SetTaskProcessor(engine::TaskProcessor&
103103
return *this;
104104
}
105105

106-
HttpRequestBuilder& HttpRequestBuilder::SetHttpHandlerStatistics(handlers::HttpRequestStatistics& stats) {
107-
request_->SetHttpHandlerStatistics(stats);
108-
return *this;
109-
}
110-
111106
HttpRequestBuilder& HttpRequestBuilder::SetStreamProducer(impl::Http2StreamEventProducer&& producer) {
112107
request_->SetStreamProducer(std::move(producer));
113108
return *this;

core/src/server/http/http_request_constructor.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ void HttpRequestConstructor::ParseUrl() {
126126

127127
builder_.SetTaskProcessor(handler_info->task_processor);
128128
builder_.SetHttpHandler(handler_info->handler);
129-
builder_.SetHttpHandlerStatistics(handler_info->handler.GetRequestStatistics());
130129
} else {
131130
if (match_result.status == MatchRequestResult::Status::kMethodNotAllowed) {
132131
SetStatus(Status::kMethodNotAllowed);

core/src/server/http/http_request_handler.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,6 @@ HttpRequestHandler::HttpRequestHandler(
5454
engine::TaskWithResult<void> HttpRequestHandler::StartFailsafeTask(std::shared_ptr<http::HttpRequest> http_request
5555
) const {
5656
const auto* handler = http_request->GetHttpHandler();
57-
static handlers::HttpRequestStatistics dummy_statistics;
58-
59-
http_request->SetHttpHandlerStatistics(dummy_statistics);
6057

6158
return engine::AsyncNoSpan([request = std::move(http_request), handler]() {
6259
request->SetTaskStartTime();

0 commit comments

Comments
 (0)