Skip to content

Commit fb19ca5

Browse files
committed
fix components: clean up startup failure logs
* On startup failure, only the first component's exception is now logged with `level=ERROR` (and others are logged with `level=WARNING`), making it easy to distinguish the root cause. * On startup failure, `OnLoadingCancelled` hook debug logs are no longer attached to an unrelated span. commit_hash:0b7349183a8acf832b5909957fb90190f1688eea
1 parent 74a037d commit fb19ca5

7 files changed

Lines changed: 253 additions & 32 deletions

File tree

.mapping.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,6 +1514,7 @@
15141514
"core/src/components/component_list_test.hpp":"taxi/uservices/userver/core/src/components/component_list_test.hpp",
15151515
"core/src/components/component_sample_test.cpp":"taxi/uservices/userver/core/src/components/component_sample_test.cpp",
15161516
"core/src/components/component_sample_test.hpp":"taxi/uservices/userver/core/src/components/component_sample_test.hpp",
1517+
"core/src/components/component_start_failure_log_test.cpp":"taxi/uservices/userver/core/src/components/component_start_failure_log_test.cpp",
15171518
"core/src/components/config_not_required_test.cpp":"taxi/uservices/userver/core/src/components/config_not_required_test.cpp",
15181519
"core/src/components/config_schema_validation_test.cpp":"taxi/uservices/userver/core/src/components/config_schema_validation_test.cpp",
15191520
"core/src/components/container_test.cpp":"taxi/uservices/userver/core/src/components/container_test.cpp",
@@ -6444,6 +6445,7 @@
64446445
"universal/utest/src/utest/current_process_open_files.cpp":"taxi/uservices/userver/universal/utest/src/utest/current_process_open_files.cpp",
64456446
"universal/utest/src/utest/current_process_open_files_test.cpp":"taxi/uservices/userver/universal/utest/src/utest/current_process_open_files_test.cpp",
64466447
"universal/utest/src/utest/log_capture_fixture.cpp":"taxi/uservices/userver/universal/utest/src/utest/log_capture_fixture.cpp",
6448+
"universal/utest/src/utest/log_capture_fixture_test.cpp":"taxi/uservices/userver/universal/utest/src/utest/log_capture_fixture_test.cpp",
64476449
"universal/utest/src/utest/parameter_names_test.cpp":"taxi/uservices/userver/universal/utest/src/utest/parameter_names_test.cpp",
64486450
"universal/version.hpp.in":"taxi/uservices/userver/universal/version.hpp.in",
64496451
"version.txt":"taxi/uservices/userver/version.txt",
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
#include <algorithm>
2+
#include <optional>
3+
#include <ranges>
4+
#include <string>
5+
#include <vector>
6+
7+
#include <fmt/format.h>
8+
#include <gtest/gtest.h>
9+
10+
#include <userver/components/component_base.hpp>
11+
#include <userver/components/component_context.hpp>
12+
#include <userver/components/component_list.hpp>
13+
#include <userver/components/run.hpp>
14+
#include <userver/components/statistics_storage.hpp>
15+
#include <userver/engine/exception.hpp>
16+
#include <userver/engine/sleep.hpp>
17+
#include <userver/engine/task/cancel.hpp>
18+
#include <userver/fs/blocking/read.hpp>
19+
#include <userver/fs/blocking/temp_directory.hpp>
20+
#include <userver/logging/component.hpp>
21+
#include <userver/logging/log.hpp>
22+
#include <userver/os_signals/component.hpp>
23+
#include <userver/utest/log_capture_fixture.hpp>
24+
#include <userver/utest/utest.hpp>
25+
26+
#include <components/component_list_test.hpp>
27+
28+
USERVER_NAMESPACE_BEGIN
29+
30+
namespace {
31+
32+
// Sleeps in its constructor until the sibling component fails and cancels component loading.
33+
class SlowStartingComponent final : public components::ComponentBase {
34+
public:
35+
static constexpr std::string_view kName = "slow-component";
36+
37+
SlowStartingComponent(const components::ComponentConfig& config, const components::ComponentContext& context)
38+
: ComponentBase(config, context)
39+
{
40+
engine::InterruptibleSleepFor(utest::kMaxTestWaitTime);
41+
if (engine::current_task::ShouldCancel()) {
42+
throw engine::WaitInterruptedException(engine::current_task::CancellationReason());
43+
}
44+
}
45+
};
46+
47+
// Yields several times so the slow component starts waiting, then throws.
48+
class FailingComponent final : public components::ComponentBase {
49+
public:
50+
static constexpr std::string_view kName = "failing-component";
51+
52+
FailingComponent(const components::ComponentConfig& config, const components::ComponentContext& context)
53+
: ComponentBase(config, context)
54+
{
55+
for (int i = 0; i < 10; ++i) {
56+
engine::Yield();
57+
}
58+
throw std::runtime_error("FailingComponent constructor intentionally throws");
59+
}
60+
};
61+
62+
constexpr std::string_view kCannotStartComponentLogTextPrefix = "Cannot start component";
63+
constexpr std::string_view kOnLoadingCancelledLogTextPrefix = "Call OnLoadingCancelled() for component";
64+
65+
std::size_t CountCannotStartComponentLogs(
66+
const std::vector<utest::LogRecord>& records,
67+
std::optional<std::string_view> level = std::nullopt,
68+
std::optional<std::string_view> component_name = std::nullopt
69+
) {
70+
return std::ranges::count_if(records, [&](const utest::LogRecord& record) {
71+
return record.GetText().starts_with(kCannotStartComponentLogTextPrefix) &&
72+
(!level || record.GetTagOptional("level") == *level) &&
73+
(!component_name || record.GetTagOptional("component_name") == *component_name);
74+
});
75+
}
76+
77+
std::size_t CountOnLoadingCancelledLogs(
78+
const std::vector<utest::LogRecord>& records,
79+
std::optional<std::string_view> level = std::nullopt,
80+
std::optional<std::string_view> component_name = std::nullopt
81+
) {
82+
return std::ranges::count_if(records, [&](const utest::LogRecord& record) {
83+
return record.GetText().starts_with(kOnLoadingCancelledLogTextPrefix) &&
84+
!record.GetTagOptional("component_name") && (!level || record.GetTagOptional("level") == *level) &&
85+
(!component_name ||
86+
record.GetText() == fmt::format("Call OnLoadingCancelled() for component '{}'", *component_name));
87+
});
88+
}
89+
90+
} // namespace
91+
92+
using ComponentStartFailureLog = ComponentList;
93+
94+
TEST_F(ComponentStartFailureLog, RootCauseAndCancelledComponentLogs) {
95+
const auto temp_root = fs::blocking::TempDirectory::Create();
96+
const std::string logs_path = temp_root.GetPath() + "/log.txt";
97+
98+
const std::string config = tests::MergeYaml(
99+
tests::kMinimalStaticConfig,
100+
fmt::format(
101+
R"(
102+
components_manager:
103+
components:
104+
slow-component: {{}}
105+
failing-component: {{}}
106+
statistics-storage: {{}}
107+
logging:
108+
loggers:
109+
default:
110+
file_path: {}
111+
format: tskv
112+
level: debug
113+
)",
114+
logs_path
115+
)
116+
);
117+
118+
auto component_list =
119+
components::ComponentList()
120+
.Append<os_signals::ProcessorComponent>()
121+
.Append<components::StatisticsStorage>()
122+
.Append<components::Logging>()
123+
.Append<SlowStartingComponent>()
124+
.Append<FailingComponent>();
125+
126+
UEXPECT_THROW_MSG(
127+
components::RunOnce(components::InMemoryConfig{config}, component_list),
128+
std::runtime_error,
129+
"FailingComponent constructor intentionally throws"
130+
);
131+
132+
logging::LogFlush();
133+
134+
const auto records = utest::ParseTskvLogRecords(fs::blocking::ReadFileContents(logs_path));
135+
136+
ASSERT_EQ(CountCannotStartComponentLogs(records, "ERROR", FailingComponent::kName), 1) << records;
137+
ASSERT_EQ(CountCannotStartComponentLogs(records, "WARNING", SlowStartingComponent::kName), 1) << records;
138+
ASSERT_EQ(CountCannotStartComponentLogs(records), 2) << records;
139+
140+
ASSERT_EQ(CountOnLoadingCancelledLogs(records, "DEBUG", SlowStartingComponent::kName), 1) << records;
141+
ASSERT_EQ(CountOnLoadingCancelledLogs(records, "DEBUG", FailingComponent::kName), 1) << records;
142+
ASSERT_EQ(CountOnLoadingCancelledLogs(records, "DEBUG"), std::ranges::distance(component_list)) << records;
143+
}
144+
145+
USERVER_NAMESPACE_END

core/src/components/manager.cpp

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
#include <userver/components/component_list.hpp>
1919
#include <userver/engine/async.hpp>
2020
#include <userver/engine/task/current_task.hpp>
21-
#include <userver/engine/wait_any.hpp>
21+
#include <userver/engine/wait_all_checked.hpp>
2222
#include <userver/hostinfo/cpu_limit.hpp>
2323
#include <userver/logging/component.hpp>
2424
#include <userver/logging/log.hpp>
@@ -366,51 +366,45 @@ components::ComponentConfigMap Manager::MakeComponentConfigMap(const ComponentLi
366366
return component_config_map;
367367
}
368368

369+
struct ComponentAddResult {
370+
bool has_components_load_cancelled_exception{false};
371+
};
372+
369373
void Manager::AddComponents(const ComponentList& component_list) {
374+
auto start_time = std::chrono::steady_clock::now();
375+
370376
const auto component_config_map = MakeComponentConfigMap(component_list);
377+
ValidateConfigs(component_list, component_config_map, config_->validate_components_configs);
378+
std::vector<engine::TaskWithResult<ComponentAddResult>> tasks;
371379

372-
auto start_time = std::chrono::steady_clock::now();
373-
std::vector<engine::TaskWithResult<void>> tasks;
374-
bool is_load_cancelled = false;
375380
try {
376-
ValidateConfigs(component_list, component_config_map, config_->validate_components_configs);
377-
378381
for (const auto& adder : component_list) {
379382
const auto& component_name = adder->GetComponentName();
380383
auto task_name = "boot/" + component_name;
381-
tasks.push_back(utils::CriticalAsync(std::move(task_name), [&]() {
384+
385+
tasks.push_back(utils::CriticalAsync(std::move(task_name), [&]() -> ComponentAddResult {
382386
tracing::Span::CurrentSpan().AddTag("component_name", component_name);
383387
tracing::Span::CurrentSpan().SetLogLevel(logging::Level::kDebug);
384388
try {
385389
AddComponentImpl(component_config_map, component_name, *adder);
390+
return {};
386391
} catch (const ComponentsLoadCancelledException& ex) {
387392
LOG_WARNING() << "Cannot start component " << component_name << ": " << ex;
388-
component_context_->CancelComponentsLoad();
389-
throw;
393+
// Keep the parent task waiting for the root cause of the startup failure.
394+
return {.has_components_load_cancelled_exception = true};
390395
} catch (const std::exception& ex) {
391-
LOG_ERROR() << "Cannot start component " << component_name << ": " << ex;
392-
component_context_->CancelComponentsLoad();
396+
// The exception is likely the root cause of the startup failure
397+
// if it happened not because of a task cancellation.
398+
const auto log_level =
399+
engine::current_task::ShouldCancel() ? logging::Level::kWarning : logging::Level::kError;
400+
LOG(log_level) << "Cannot start component " << component_name << ": " << ex;
393401
throw std::runtime_error(fmt::format("Cannot start component {}: {}", component_name, ex.what()));
394-
} catch (...) {
395-
component_context_->CancelComponentsLoad();
396-
throw;
397402
}
398403
}));
399404
}
400405

401406
// Wait for tasks in completion order and rethrow exceptions as soon as possible.
402-
auto wait_any = engine::MakeWaitAny(tasks);
403-
for (std::size_t completed = 0; completed < tasks.size(); ++completed) {
404-
const auto idx = wait_any.Wait();
405-
if (!idx.has_value()) {
406-
throw engine::WaitInterruptedException(engine::current_task::CancellationReason());
407-
}
408-
try {
409-
tasks[*idx].Get();
410-
} catch (const ComponentsLoadCancelledException&) {
411-
is_load_cancelled = true;
412-
}
413-
}
407+
engine::WaitAllChecked(tasks);
414408
} catch (const std::exception& ex) {
415409
component_context_->CancelComponentsLoad();
416410

@@ -431,12 +425,12 @@ void Manager::AddComponents(const ComponentList& component_list) {
431425
throw;
432426
}
433427

428+
const bool is_load_cancelled = std::ranges::any_of(tasks, [](auto& task) {
429+
return task.Get().has_components_load_cancelled_exception;
430+
});
434431
if (is_load_cancelled) {
435432
ClearComponents();
436-
throw std::logic_error(
437-
"Components load cancelled, but only ComponentsLoadCancelledExceptions "
438-
"were caught"
439-
);
433+
throw std::logic_error("Components load cancelled, but only ComponentsLoadCancelledExceptions were caught");
440434
}
441435

442436
LOG_INFO()

universal/src/utils/text_light_test.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <gmock/gmock.h>
12
#include <gtest/gtest.h>
23

34
#include <climits>

universal/utest/include/userver/utest/log_capture_fixture.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ LogRecord GetSingleLog(
8787
const utils::impl::SourceLocation& source_location = utils::impl::SourceLocation::Current()
8888
);
8989

90+
/// @returns log records parsed from TSKV file contents.
91+
/// Each line must be a complete TSKV record ending with `\n`.
92+
std::vector<LogRecord> ParseTskvLogRecords(std::string_view tskv_log_contents);
93+
9094
/// @ingroup userver_utest
9195
///
9296
/// @brief A mocked logger that stores the log records in memory.

universal/utest/src/utest/log_capture_fixture.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@
66
#include <fmt/format.h>
77
#include <boost/algorithm/cxx11/all_of.hpp>
88

9-
#include <userver/utils/algo.hpp>
109
#include <userver/utils/encoding/tskv_parser.hpp>
1110
#include <userver/utils/encoding/tskv_parser_read.hpp>
12-
#include <userver/utils/text_light.hpp>
1311

1412
USERVER_NAMESPACE_BEGIN
1513

@@ -150,6 +148,28 @@ LogRecord GetSingleLog(utils::span<const LogRecord> log, const utils::impl::Sour
150148
return single_record;
151149
}
152150

151+
std::vector<LogRecord> ParseTskvLogRecords(std::string_view tskv_log_contents) {
152+
std::vector<LogRecord> result;
153+
while (true) {
154+
const auto newline_pos = tskv_log_contents.find('\n');
155+
if (newline_pos == std::string_view::npos) {
156+
break;
157+
}
158+
result.emplace_back(
159+
utils::impl::InternalTag{},
160+
logging::Level::kInfo,
161+
std::string(tskv_log_contents.substr(0, newline_pos + 1))
162+
);
163+
tskv_log_contents.remove_prefix(newline_pos + 1);
164+
}
165+
if (!tskv_log_contents.empty()) {
166+
throw std::runtime_error(
167+
fmt::format(R"(TSKV log contents must end with '\n', leftover: "{}")", tskv_log_contents)
168+
);
169+
}
170+
return result;
171+
}
172+
153173
LogCaptureLogger::LogCaptureLogger(logging::Format format)
154174
: logger_(utils::MakeSharedRef<impl::ToStringLogger>(format))
155175
{}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#include <userver/utest/log_capture_fixture.hpp>
2+
3+
#include <string>
4+
5+
#include <gmock/gmock.h>
6+
#include <gtest/gtest.h>
7+
8+
#include <userver/utest/assert_macros.hpp>
9+
10+
USERVER_NAMESPACE_BEGIN
11+
12+
namespace {
13+
14+
constexpr std::string_view kLine1 = "tskv\ttext=hello\tlevel=INFO\n";
15+
constexpr std::string_view kLine2 = "tskv\ttext=world\tlevel=DEBUG\n";
16+
17+
} // namespace
18+
19+
TEST(ParseTskvLogRecords, Empty) { EXPECT_THAT(utest::ParseTskvLogRecords(""), testing::IsEmpty()); }
20+
21+
TEST(ParseTskvLogRecords, SingleRecord) {
22+
const auto records = utest::ParseTskvLogRecords(kLine1);
23+
ASSERT_EQ(records.size(), 1);
24+
EXPECT_EQ(records[0].GetText(), "hello");
25+
EXPECT_EQ(records[0].GetTagOptional("level"), "INFO");
26+
EXPECT_EQ(records[0].GetLogRaw(), kLine1);
27+
}
28+
29+
TEST(ParseTskvLogRecords, MultipleRecords) {
30+
const std::string content = std::string(kLine1) + std::string(kLine2);
31+
const auto records = utest::ParseTskvLogRecords(content);
32+
ASSERT_EQ(records.size(), 2);
33+
EXPECT_EQ(records[0].GetText(), "hello");
34+
EXPECT_EQ(records[1].GetText(), "world");
35+
EXPECT_EQ(records[0].GetLogRaw(), kLine1);
36+
EXPECT_EQ(records[1].GetLogRaw(), kLine2);
37+
}
38+
39+
TEST(ParseTskvLogRecords, NoTrailingNewline) {
40+
UEXPECT_THROW_MSG(
41+
utest::ParseTskvLogRecords("tskv\ttext=hello"),
42+
std::runtime_error,
43+
R"(TSKV log contents must end with '\n')"
44+
);
45+
}
46+
47+
TEST(ParseTskvLogRecords, LeftoverAfterLastNewline) {
48+
UEXPECT_THROW_MSG(utest::ParseTskvLogRecords("tskv\ttext=hello\nleftover"), std::runtime_error, "leftover");
49+
}
50+
51+
TEST(ParseTskvLogRecords, InvalidRecord) {
52+
UEXPECT_THROW_MSG(utest::ParseTskvLogRecords("garbage\n"), std::runtime_error, "Invalid log record");
53+
}
54+
55+
USERVER_NAMESPACE_END

0 commit comments

Comments
 (0)