Skip to content

Commit 322a2ed

Browse files
seriouscoder43robot-piglet
authored andcommitted
fix core: convert SystemStatisticsCollector to PIMPL
Issue: current SystemStatisticsCollector header includes an internal header and therefore isn't really usable in every case. I believe this class should match TaskProcessorsLoadMonitor. --- Pull Request resolved: #1275 commit_hash:bd4ad1ea9bd5fa6f0d792c1674aec6c04d8827db
1 parent bd2e459 commit 322a2ed

2 files changed

Lines changed: 47 additions & 25 deletions

File tree

core/include/userver/utils/statistics/system_statistics_collector.hpp

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,19 @@
33
/// @file userver/utils/statistics/system_statistics_collector.hpp
44
/// @brief @copybrief components::SystemStatisticsCollector
55

6+
#include <memory>
7+
68
#include <userver/components/component_base.hpp>
79
#include <userver/components/component_fwd.hpp>
8-
#include <userver/concurrent/variable.hpp>
9-
#include <userver/engine/task/task_processor_fwd.hpp>
10-
#include <userver/utils/periodic_task.hpp>
11-
#include <utils/statistics/system_statistics.hpp>
1210

1311
USERVER_NAMESPACE_BEGIN
1412

13+
namespace utils::statistics {
14+
15+
class Writer;
16+
17+
} // namespace utils::statistics
18+
1519
namespace components {
1620

1721
/// @ingroup userver_components
@@ -36,6 +40,7 @@ class SystemStatisticsCollector final : public ComponentBase {
3640
static constexpr std::string_view kName = "system-statistics-collector";
3741

3842
SystemStatisticsCollector(const ComponentConfig&, const ComponentContext&);
43+
~SystemStatisticsCollector() override;
3944

4045
static yaml_config::Schema GetStaticConfigSchema();
4146

@@ -44,15 +49,8 @@ class SystemStatisticsCollector final : public ComponentBase {
4449

4550
void ProcessTimer();
4651

47-
struct Data {
48-
utils::statistics::impl::SystemStats last_stats{};
49-
utils::statistics::impl::SystemStats last_nginx_stats{};
50-
};
51-
52-
const bool with_nginx_;
53-
engine::TaskProcessor& fs_task_processor_;
54-
concurrent::Variable<Data> data_;
55-
utils::PeriodicTask periodic_;
52+
struct Impl;
53+
std::unique_ptr<Impl> impl_;
5654
};
5755

5856
template <>

core/src/utils/statistics/system_statistics_collector.cpp

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
#include <userver/utils/statistics/system_statistics_collector.hpp>
22

3+
#include <memory>
4+
35
#include <userver/components/component.hpp>
46
#include <userver/components/statistics_storage.hpp>
7+
#include <userver/concurrent/variable.hpp>
58
#include <userver/engine/async.hpp>
9+
#include <userver/utils/periodic_task.hpp>
610
#include <userver/yaml_config/merge_schemas.hpp>
711

12+
#include <utils/statistics/system_statistics.hpp>
13+
814
#ifndef ARCADIA_ROOT
915
#include "generated/src/utils/statistics/system_statistics_collector.yaml.hpp" // Y_IGNORE
1016
#endif
@@ -13,40 +19,58 @@ USERVER_NAMESPACE_BEGIN
1319

1420
namespace components {
1521

22+
struct SystemStatisticsCollector::Impl {
23+
struct Data {
24+
utils::statistics::impl::SystemStats last_stats{};
25+
utils::statistics::impl::SystemStats last_nginx_stats{};
26+
};
27+
28+
Impl(SystemStatisticsCollector& owner, const ComponentConfig& config, const ComponentContext& context)
29+
: with_nginx(config["with-nginx"].As<bool>(false)),
30+
fs_task_processor(GetFsTaskProcessor(config, context)),
31+
periodic(
32+
"system_statistics_collector",
33+
{std::chrono::seconds(10), {utils::PeriodicTask::Flags::kNow}},
34+
[&owner] { owner.ProcessTimer(); }
35+
)
36+
{}
37+
38+
const bool with_nginx;
39+
engine::TaskProcessor& fs_task_processor;
40+
concurrent::Variable<Data> data;
41+
utils::PeriodicTask periodic;
42+
};
43+
1644
SystemStatisticsCollector::SystemStatisticsCollector(const ComponentConfig& config, const ComponentContext& context)
1745
: ComponentBase(config, context),
18-
with_nginx_(config["with-nginx"].As<bool>(false)),
19-
fs_task_processor_(GetFsTaskProcessor(config, context)),
20-
periodic_(
21-
"system_statistics_collector",
22-
{std::chrono::seconds(10), {utils::PeriodicTask::Flags::kNow}},
23-
[this] { ProcessTimer(); }
24-
)
46+
impl_(std::make_unique<Impl>(*this, config, context))
2547
{
2648
utils::statistics::RegisterWriterScope(context, "", [this](utils::statistics::Writer& writer) {
2749
ExtendStatistics(writer);
2850
});
2951
}
3052

53+
SystemStatisticsCollector::~SystemStatisticsCollector() = default;
54+
3155
void SystemStatisticsCollector::ProcessTimer() {
32-
engine::CriticalAsyncNoTracing(fs_task_processor_, [&] {
56+
engine::CriticalAsyncNoTracing(impl_->fs_task_processor, [&] {
3357
auto self = utils::statistics::impl::GetSelfSystemStatistics();
3458
utils::statistics::impl::SystemStats nginx;
35-
if (with_nginx_) {
59+
if (impl_->with_nginx) {
3660
nginx = utils::statistics::impl::GetSystemStatisticsByExeName("nginx");
3761
}
3862

39-
auto data = data_.UniqueLock();
63+
auto data = impl_->data.UniqueLock();
4064
data->last_stats = self;
4165
data->last_nginx_stats = nginx;
4266
}).Get();
4367
}
4468

4569
void SystemStatisticsCollector::ExtendStatistics(utils::statistics::Writer& writer) {
46-
auto data = data_.Lock();
70+
auto data = impl_->data.Lock();
4771

4872
DumpMetric(writer, data->last_stats);
49-
if (with_nginx_) {
73+
if (impl_->with_nginx) {
5074
writer.ValueWithLabels(data->last_nginx_stats, {"application", "nginx"});
5175
}
5276
}

0 commit comments

Comments
 (0)