Skip to content

Commit 569d6f6

Browse files
committed
feat logging: add an alert for invalid dynamic debug lines
commit_hash:6d8808ae13f15abae6267f7871cdd5b96de06d9d
1 parent 3959ae0 commit 569d6f6

4 files changed

Lines changed: 50 additions & 36 deletions

File tree

core/functional_tests/metrics/tests/static/metrics_values.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
alerts.cache_update_error: GAUGE 0
22
alerts.config_parse_error: GAUGE 0
3+
alerts.dynamic_debug_invalid_location: GAUGE 0
34
alerts.log_reopening_error: GAUGE 0
45
cache.any.documents.parse_failures.v2: cache_name=dynamic-config-client-updater RATE 0
56
cache.any.documents.parse_failures.v2: cache_name=sample-cache RATE 0

core/include/userver/alerts/source.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ void DumpMetric(utils::statistics::Writer& writer, const SourceData& m);
2727
class Source final {
2828
public:
2929
static constexpr std::chrono::seconds kDefaultDuration{120};
30+
static constexpr std::chrono::hours kInfiniteDuration{24 * 365 * 10}; // In 10 years, someone should notice.
3031

3132
explicit Source(const std::string& name);
3233

core/include/userver/components/logging_configurator.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
#include <userver/components/component_fwd.hpp>
77
#include <userver/components/raw_component_base.hpp>
88
#include <userver/concurrent/async_event_source.hpp>
9-
#include <userver/dynamic_config/source.hpp>
10-
#include <userver/rcu/rcu.hpp>
9+
#include <userver/utils/statistics/fwd.hpp>
1110

1211
#include <dynamic_config/variables/USERVER_LOG_DYNAMIC_DEBUG.hpp>
1312

@@ -58,8 +57,9 @@ class LoggingConfigurator final : public RawComponentBase {
5857
private:
5958
void OnConfigUpdate(const dynamic_config::Snapshot& config);
6059

60+
utils::statistics::MetricsStoragePtr metrics_storage_;
61+
// config_subscription_ must be the last field!
6162
concurrent::AsyncEventSubscriberScope config_subscription_;
62-
rcu::Variable<logging::DynamicDebugConfig> dynamic_debug_;
6363
};
6464

6565
/// }@

core/src/components/logging_configurator.cpp

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
#include <logging/dynamic_debug.hpp>
44
#include <logging/dynamic_debug_config.hpp>
5+
#include <userver/alerts/source.hpp>
56
#include <userver/components/component.hpp>
7+
#include <userver/components/statistics_storage.hpp>
68
#include <userver/dynamic_config/storage/component.hpp>
79
#include <userver/dynamic_config/value.hpp>
810
#include <userver/tracing/tracer.hpp>
@@ -28,14 +30,17 @@ const dynamic_config::Key<logging::DynamicDebugConfig> kDynamicDebugConfig{
2830
}
2931
)"}};
3032

33+
alerts::Source kDynamicDebugInvalidLocation{"dynamic_debug_invalid_location"};
34+
3135
} // namespace
3236

33-
LoggingConfigurator::LoggingConfigurator(const ComponentConfig& config, const ComponentContext& context) {
37+
LoggingConfigurator::LoggingConfigurator(const ComponentConfig& config, const ComponentContext& context)
38+
: metrics_storage_(context.FindComponent<components::StatisticsStorage>().GetMetricsStorage()) {
3439
logging::impl::SetLogLimitedEnable(config["limited-logging-enable"].As<bool>());
3540
logging::impl::SetLogLimitedInterval(config["limited-logging-interval"].As<std::chrono::milliseconds>());
3641

3742
config_subscription_ = context.FindComponent<components::DynamicConfig>().GetSource().UpdateAndListen(
38-
this, kName, &LoggingConfigurator::OnConfigUpdate
43+
this, kName, &LoggingConfigurator::OnConfigUpdate, ::dynamic_config::USERVER_NO_LOG_SPANS, kDynamicDebugConfig
3944
);
4045
}
4146

@@ -45,38 +50,45 @@ void LoggingConfigurator::OnConfigUpdate(const dynamic_config::Snapshot& config)
4550
(void)this; // silence clang-tidy
4651
tracing::SetNoLogSpans(tracing::NoLogSpans{config[::dynamic_config::USERVER_NO_LOG_SPANS]});
4752

48-
try {
49-
const auto& dd = config[kDynamicDebugConfig];
50-
auto old_dd = dynamic_debug_.Read();
51-
if (!(*old_dd == dd)) {
52-
auto lock = dynamic_debug_.StartWrite();
53-
*lock = dd;
54-
55-
/* There is a race between multiple AddDynamicDebugLog(), thus some logs
56-
* may be logged or not logged by mistake. This is on purpose as logging
57-
* locking would be too slow and heavy.
58-
*/
59-
60-
// Flush
61-
logging::RemoveAllDynamicDebugLog();
62-
63-
for (const auto& [location, level] : dd.force_disabled) {
64-
const auto [path, line] = logging::SplitLocation(location);
65-
logging::EntryState state;
66-
state.force_disabled_level_plus_one = logging::GetForceDisabledLevelPlusOne(level);
67-
AddDynamicDebugLog(path, line, state);
68-
}
69-
for (const auto& [location, level] : dd.force_enabled) {
70-
const auto [path, line] = logging::SplitLocation(location);
71-
logging::EntryState state;
72-
state.force_enabled_level = level;
73-
AddDynamicDebugLog(path, line, state);
74-
}
75-
76-
lock.Commit();
53+
const auto& dd = config[kDynamicDebugConfig];
54+
55+
/* There is a race between multiple AddDynamicDebugLog(), thus some logs
56+
* may be logged or not logged by mistake. This is on purpose as logging
57+
* locking would be too slow and heavy.
58+
*/
59+
60+
// Flush
61+
logging::RemoveAllDynamicDebugLog();
62+
bool has_error = false;
63+
64+
for (const auto& [location, level] : dd.force_disabled) {
65+
try {
66+
const auto [path, line] = logging::SplitLocation(location);
67+
logging::EntryState state;
68+
state.force_disabled_level_plus_one = logging::GetForceDisabledLevelPlusOne(level);
69+
AddDynamicDebugLog(path, line, state);
70+
} catch (const std::exception& e) {
71+
LOG_ERROR() << "Failed to disable dynamic debug log at '" << location << "': " << e;
72+
has_error = true;
73+
}
74+
}
75+
76+
for (const auto& [location, level] : dd.force_enabled) {
77+
try {
78+
const auto [path, line] = logging::SplitLocation(location);
79+
logging::EntryState state;
80+
state.force_enabled_level = level;
81+
AddDynamicDebugLog(path, line, state);
82+
} catch (const std::exception& e) {
83+
LOG_ERROR() << "Failed to enable dynamic debug log at '" << location << "': " << e;
84+
has_error = true;
7785
}
78-
} catch (const std::exception& e) {
79-
LOG_ERROR() << "Failed to set dynamic debug logs from config: " << e;
86+
}
87+
88+
if (has_error) {
89+
kDynamicDebugInvalidLocation.FireAlert(*metrics_storage_, alerts::Source::kInfiniteDuration);
90+
} else {
91+
kDynamicDebugInvalidLocation.StopAlertNow(*metrics_storage_);
8092
}
8193
}
8294

0 commit comments

Comments
 (0)