Skip to content

Commit 9945a0d

Browse files
deepblinkov
authored andcommitted
YTORM-1292 Error enrichment via dependency injection
Идея такая: хочу подкладывать атрибуты в ошибки, не протаскивая их через стек (как в orm/server/objects) и не прогоняя все ошибки через специальные методы (как в orm/library/attributes). Для этого завожу fiber-local структурку с ленивым выведением строчек. Поскольку TError и TFls живут в разных мирах, пришлось сделать отдельный трамплин для совсем генеричной доработки ошибок в момент создания. Игнат посоветовал глянуть на Codicil. Там очень похоже, но нет key/value, поэтому похитил только название. Вообще, можно унифицировать, если есть запрос. commit_hash:203ec7abe5e8c8484e66d55f16192485db776806
1 parent 142d060 commit 9945a0d

File tree

8 files changed

+213
-3
lines changed

8 files changed

+213
-3
lines changed

library/cpp/yt/error/error.cpp

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ void FormatValue(TStringBuilderBase* builder, TErrorCode code, TStringBuf spec)
2929

3030
constexpr TStringBuf ErrorMessageTruncatedSuffix = "...<message truncated>";
3131

32+
TError::TEnricher TError::Enricher_;
33+
3234
////////////////////////////////////////////////////////////////////////////////
3335

3436
class TError::TImpl
@@ -275,15 +277,20 @@ TError::TErrorOr(const std::exception& ex)
275277
*this = TError(NYT::EErrorCode::Generic, TRuntimeFormat{ex.what()});
276278
}
277279
YT_VERIFY(!IsOK());
280+
Enrich();
278281
}
279282

280283
TError::TErrorOr(std::string message, TDisableFormat)
281284
: Impl_(std::make_unique<TImpl>(std::move(message)))
282-
{ }
285+
{
286+
Enrich();
287+
}
283288

284289
TError::TErrorOr(TErrorCode code, std::string message, TDisableFormat)
285290
: Impl_(std::make_unique<TImpl>(code, std::move(message)))
286-
{ }
291+
{
292+
Enrich();
293+
}
287294

288295
TError& TError::operator = (const TError& other)
289296
{
@@ -632,6 +639,20 @@ std::optional<TError> TError::FindMatching(const THashSet<TErrorCode>& codes) co
632639
});
633640
}
634641

642+
void TError::RegisterEnricher(TEnricher enricher)
643+
{
644+
// NB: This daisy-chaining strategy is optimal when there's O(1) callbacks. Convert to a vector
645+
// if the number grows.
646+
if (Enricher_) {
647+
Enricher_ = [first = std::move(Enricher_), second = std::move(enricher)] (TError& error) {
648+
first(error);
649+
second(error);
650+
};
651+
} else {
652+
Enricher_ = std::move(enricher);
653+
}
654+
}
655+
635656
TError::TErrorOr(std::unique_ptr<TImpl> impl)
636657
: Impl_(std::move(impl))
637658
{ }
@@ -643,6 +664,13 @@ void TError::MakeMutable()
643664
}
644665
}
645666

667+
void TError::Enrich()
668+
{
669+
if (Enricher_) {
670+
Enricher_(*this);
671+
}
672+
}
673+
646674
////////////////////////////////////////////////////////////////////////////////
647675

648676
TError& TError::operator <<= (const TErrorAttribute& attribute) &

library/cpp/yt/error/error.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,15 +219,25 @@ class [[nodiscard]] TErrorOr<void>
219219
template <CErrorNestable TValue>
220220
TError operator << (const std::optional<TValue>& rhs) const &;
221221

222+
// The |enricher| is called during TError construction and before TErrorOr<> construction. Meant
223+
// to enrich the error, e.g. by setting generic attributes. The |RegisterEnricher| method is not
224+
// threadsafe and is meant to be called from single-threaded bootstrapping code. Multiple
225+
// enrichers are supported and will be called in order of registration.
226+
using TEnricher = std::function<void(TError&)>;
227+
static void RegisterEnricher(TEnricher enricher);
228+
222229
private:
223230
class TImpl;
224231
std::unique_ptr<TImpl> Impl_;
225232

226233
explicit TErrorOr(std::unique_ptr<TImpl> impl);
227234

228235
void MakeMutable();
236+
void Enrich();
229237

230238
friend class TErrorAttributes;
239+
240+
static TEnricher Enricher_;
231241
};
232242

233243
////////////////////////////////////////////////////////////////////////////////

yt/yt/core/concurrency/fls-inl.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ Y_FORCE_INLINE T* TFlsSlot<T>::GetOrCreate() const
9292
return static_cast<T*>(cookie);
9393
}
9494

95+
template <class T>
96+
Y_FORCE_INLINE T* TFlsSlot<T>::MaybeGet() const
97+
{
98+
return static_cast<T*>(GetCurrentFls()->Get(Index_));
99+
}
100+
95101
template <class T>
96102
T* TFlsSlot<T>::Create() const
97103
{

yt/yt/core/concurrency/fls.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class TFlsSlot
4242
T* operator->();
4343

4444
T* GetOrCreate() const;
45+
T* MaybeGet() const;
4546
const T* Get(const TFls& fls) const;
4647

4748
bool IsInitialized() const;

yt/yt/core/misc/error.cpp

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
#include "error.h"
22
#include "serialize.h"
33

4-
#include <yt/yt/core/concurrency/public.h>
4+
#include <yt/yt/core/concurrency/fls.h>
55

66
#include <yt/yt/core/net/local_address.h>
77

8+
#include <yt/yt/core/misc/collection_helpers.h>
89
#include <yt/yt/core/misc/protobuf_helpers.h>
910

1011
#include <yt/yt/core/tracing/trace_context.h>
@@ -18,6 +19,8 @@
1819

1920
#include <library/cpp/yt/global/variable.h>
2021

22+
#include <library/cpp/yt/misc/global.h>
23+
2124
namespace NYT {
2225

2326
using namespace NTracing;
@@ -31,6 +34,8 @@ using NYT::ToProto;
3134

3235
constexpr TStringBuf OriginalErrorDepthAttribute = "original_error_depth";
3336

37+
bool TErrorCodicils::Initialized_ = false;
38+
3439
////////////////////////////////////////////////////////////////////////////////
3540

3641
namespace NDetail {
@@ -656,4 +661,96 @@ void TErrorSerializer::Load(TStreamLoadContext& context, TError& error)
656661

657662
////////////////////////////////////////////////////////////////////////////////
658663

664+
static YT_DEFINE_GLOBAL(NConcurrency::TFlsSlot<TErrorCodicils>, ErrorCodicilsSlot);
665+
666+
TErrorCodicils::TGuard::~TGuard()
667+
{
668+
TErrorCodicils::GetOrCreate().Set(std::move(Key_), std::move(OldGetter_));
669+
}
670+
671+
TErrorCodicils::TGuard::TGuard(
672+
std::string key,
673+
TGetter oldGetter)
674+
: Key_(std::move(key))
675+
, OldGetter_(std::move(oldGetter))
676+
{ }
677+
678+
void TErrorCodicils::Initialize()
679+
{
680+
if (Initialized_) {
681+
// Multiple calls are OK.
682+
return;
683+
}
684+
Initialized_ = true;
685+
686+
ErrorCodicilsSlot(); // Warm up the slot.
687+
TError::RegisterEnricher([] (TError& error) {
688+
if (auto* codicils = TErrorCodicils::MaybeGet()) {
689+
codicils->Apply(error);
690+
}
691+
});
692+
}
693+
694+
TErrorCodicils& TErrorCodicils::GetOrCreate()
695+
{
696+
return *ErrorCodicilsSlot().GetOrCreate();
697+
}
698+
699+
TErrorCodicils* TErrorCodicils::MaybeGet()
700+
{
701+
return ErrorCodicilsSlot().MaybeGet();
702+
}
703+
704+
std::optional<std::string> TErrorCodicils::MaybeEvaluate(const std::string& key)
705+
{
706+
auto* instance = MaybeGet();
707+
if (!instance) {
708+
return {};
709+
}
710+
711+
auto getter = instance->Get(key);
712+
if (!getter) {
713+
return {};
714+
}
715+
716+
return getter();
717+
}
718+
719+
auto TErrorCodicils::Guard(std::string key, TGetter getter) -> TGuard
720+
{
721+
auto& instance = GetOrCreate();
722+
auto [it, added] = instance.Getters_.try_emplace(key, getter);
723+
TGetter oldGetter;
724+
if (!added) {
725+
oldGetter = std::move(it->second);
726+
it->second = std::move(getter);
727+
}
728+
return TGuard(std::move(key), std::move(oldGetter));
729+
}
730+
731+
void TErrorCodicils::Apply(TError& error) const
732+
{
733+
for (const auto& [key, getter] : Getters_) {
734+
error <<= TErrorAttribute(key, getter());
735+
}
736+
}
737+
738+
void TErrorCodicils::Set(std::string key, TGetter getter)
739+
{
740+
// We could enforce Initialized_, but that could make an error condition worse at runtime.
741+
// Instead, let's keep enrichment optional.
742+
if (getter) {
743+
Getters_.insert_or_assign(std::move(key), std::move(getter));
744+
} else {
745+
Getters_.erase(std::move(key));
746+
}
747+
}
748+
749+
auto TErrorCodicils::Get(const std::string& key) const -> TGetter
750+
{
751+
return GetOrDefault(Getters_, std::move(key));
752+
}
753+
754+
////////////////////////////////////////////////////////////////////////////////
755+
659756
} // namespace NYT

yt/yt/core/misc/error.h

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include <library/cpp/yt/error/error.h>
1414
#include <library/cpp/yt/error/origin_attributes.h>
1515

16+
#include <util/generic/noncopyable.h>
17+
1618
namespace NYT {
1719

1820
////////////////////////////////////////////////////////////////////////////////
@@ -90,6 +92,56 @@ struct TSerializerTraits<
9092

9193
////////////////////////////////////////////////////////////////////////////////
9294

95+
// A fiber-local set of attributes to enrich all errors with.
96+
class TErrorCodicils
97+
{
98+
public:
99+
using TGetter = std::function<std::string()>;
100+
101+
class TGuard
102+
: public TNonCopyable
103+
{
104+
public:
105+
~TGuard();
106+
107+
private:
108+
friend class TErrorCodicils;
109+
TGuard(std::string key, TGetter oldGetter);
110+
std::string Key_;
111+
TGetter OldGetter_;
112+
};
113+
114+
// Call from single-threaded bootstrapping code. Errors will not be enriched otherwise.
115+
static void Initialize();
116+
117+
// Gets or creates an instance for this fiber.
118+
static TErrorCodicils& GetOrCreate();
119+
120+
// Gets the instance for this fiber if one was created previously.
121+
static TErrorCodicils* MaybeGet();
122+
123+
// Evaluates the codicil for the key if one was set.
124+
static std::optional<std::string> MaybeEvaluate(const std::string& key);
125+
126+
// Sets the getter and returns an RAII object to restore the previous value on desctruction.
127+
static TGuard Guard(std::string key, TGetter getter);
128+
129+
// Adds error attributes.
130+
void Apply(TError& error) const;
131+
132+
// Sets the getter (or erases if the getter is empty).
133+
void Set(std::string key, TGetter getter);
134+
135+
// Gets the getter.
136+
TGetter Get(const std::string& key) const;
137+
138+
private:
139+
THashMap<std::string, TGetter> Getters_;
140+
static bool Initialized_;
141+
};
142+
143+
////////////////////////////////////////////////////////////////////////////////
144+
93145
} // namespace NYT
94146

95147
#define ERROR_INL_H_

yt/yt/core/misc/unittests/error_ut.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,20 @@ TEST(TErrorTest, NativeFiberId)
228228
.ThrowOnError();
229229
}
230230

231+
TEST(TErrorTest, ErrorCodicils)
232+
{
233+
EXPECT_FALSE(TError("ErrorCodicils").Attributes().Contains("test_attribute"));
234+
{
235+
auto guard = TErrorCodicils::Guard("test_attribute", [] () -> std::string {
236+
return "test_value";
237+
});
238+
EXPECT_EQ("test_value",
239+
TError("ErrorCodicils").Attributes().Get<std::string>("test_attribute"));
240+
EXPECT_FALSE(TError().Attributes().Contains("test_attribute"));
241+
}
242+
EXPECT_FALSE(TError("ErrorCodicils").Attributes().Contains("test_attribute"));
243+
}
244+
231245
////////////////////////////////////////////////////////////////////////////////
232246

233247
} // namespace

yt/yt/core/test_framework/framework.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ Y_TEST_HOOK_BEFORE_RUN(GTEST_YT_SETUP)
156156
NYT::TSignalRegistry::Get()->PushDefaultSignalHandler(NYT::AllCrashSignals);
157157
#endif
158158

159+
NYT::TErrorCodicils::Initialize();
160+
159161
auto config = NYT::NLogging::TLogManagerConfig::CreateYTServer("unittester", GetOutputPath().GetPath());
160162
NYT::NLogging::TLogManager::Get()->Configure(config);
161163
NYT::NLogging::TLogManager::Get()->EnableReopenOnSighup();

0 commit comments

Comments
 (0)