Browse Source

YT-19851: Less restrictions on logging config

nadya73 1 year ago
parent
commit
804d5f57f7

+ 3 - 39
yt/yt/core/logging/config.cpp

@@ -103,14 +103,9 @@ void TLogWriterConfig::Register(TRegistrar registrar)
     });
 }
 
-ELogFamily TLogWriterConfig::GetFamily() const
-{
-    return Format == ELogFormat::PlainText ? ELogFamily::PlainText : ELogFamily::Structured;
-}
-
 bool TLogWriterConfig::AreSystemMessagesEnabled() const
 {
-    return EnableSystemMessages.value_or(GetFamily() == ELogFamily::PlainText);
+    return EnableSystemMessages.value_or(Format == ELogFormat::PlainText);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -127,7 +122,7 @@ void TRuleConfig::Register(TRegistrar registrar)
         .Default(ELogLevel::Maximum);
     registrar.Parameter("family", &TThis::Family)
         .Alias("message_format")
-        .Default(ELogFamily::PlainText);
+        .Default();
     registrar.Parameter("writers", &TThis::Writers)
         .NonEmpty();
 }
@@ -135,7 +130,7 @@ void TRuleConfig::Register(TRegistrar registrar)
 bool TRuleConfig::IsApplicable(TStringBuf category, ELogFamily family) const
 {
     return
-        Family == family &&
+        (!Family || *Family == family) &&
         ExcludeCategories.find(category) == ExcludeCategories.end() &&
         (!IncludeCategories || IncludeCategories->find(category) != IncludeCategories->end());
 }
@@ -147,7 +142,6 @@ bool TRuleConfig::IsApplicable(TStringBuf category, ELogLevel level, ELogFamily
         MinLevel <= level && level <= MaxLevel;
 }
 
-
 ////////////////////////////////////////////////////////////////////////////////
 
 void TLogManagerConfig::Register(TRegistrar registrar)
@@ -198,36 +192,6 @@ void TLogManagerConfig::Register(TRegistrar registrar)
 
     registrar.Parameter("compression_thread_count", &TThis::CompressionThreadCount)
         .Default(1);
-
-    registrar.Postprocessor([] (TThis* config) {
-        THashMap<TString, ELogFamily> writerNameToFamily;
-        for (const auto& [writerName, writerConfig] : config->Writers) {
-            try {
-                auto typedWriterConfig = ConvertTo<TLogWriterConfigPtr>(writerConfig);
-                EmplaceOrCrash(writerNameToFamily, writerName, typedWriterConfig->GetFamily());
-            } catch (const std::exception& ex) {
-                THROW_ERROR_EXCEPTION("Malformed configuration of writer %Qv",
-                    writerName)
-                    << ex;
-            }
-        }
-
-        for (const auto& [ruleIndex, rule] : Enumerate(config->Rules)) {
-            for (const auto& writerName : rule->Writers) {
-                auto it = writerNameToFamily.find(writerName);
-                if (it == writerNameToFamily.end()) {
-                    THROW_ERROR_EXCEPTION("Unknown writer %Qv", writerName);
-                }
-                if (rule->Family != it->second) {
-                    THROW_ERROR_EXCEPTION("Writer %Qv has family %Qlv while rule %v has family %Qlv",
-                        writerName,
-                        it->second,
-                        ruleIndex,
-                        rule->Family);
-                }
-            }
-        }
-    });
 }
 
 TLogManagerConfigPtr TLogManagerConfig::ApplyDynamic(const TLogManagerDynamicConfigPtr& dynamicConfig) const

+ 1 - 2
yt/yt/core/logging/config.h

@@ -93,7 +93,6 @@ public:
     THashMap<TString, NYTree::INodePtr> CommonFields;
     NJson::TJsonFormatConfigPtr JsonFormat;
 
-    ELogFamily GetFamily() const;
     bool AreSystemMessagesEnabled() const;
 
     //! Constructs a full config by combining parameters from this one and #typedConfig.
@@ -119,7 +118,7 @@ public:
     ELogLevel MinLevel;
     ELogLevel MaxLevel;
 
-    ELogFamily Family;
+    std::optional<ELogFamily> Family;
 
     std::vector<TString> Writers;
 

+ 31 - 0
yt/yt/core/logging/unittests/logging_ut.cpp

@@ -387,11 +387,42 @@ TEST_F(TLoggingTest, Rule)
         })"))));
 
     EXPECT_TRUE(rule->IsApplicable("some_service", ELogFamily::PlainText));
+    EXPECT_TRUE(rule->IsApplicable("some_service", ELogFamily::Structured));
     EXPECT_FALSE(rule->IsApplicable("bus", ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("bus", ELogFamily::Structured));
     EXPECT_FALSE(rule->IsApplicable("bus", ELogLevel::Debug, ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("bus", ELogLevel::Debug, ELogFamily::Structured));
     EXPECT_FALSE(rule->IsApplicable("some_service", ELogLevel::Debug, ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("some_service", ELogLevel::Debug, ELogFamily::Structured));
     EXPECT_TRUE(rule->IsApplicable("some_service", ELogLevel::Warning, ELogFamily::PlainText));
+    EXPECT_TRUE(rule->IsApplicable("some_service", ELogLevel::Warning, ELogFamily::Structured));
     EXPECT_TRUE(rule->IsApplicable("some_service", ELogLevel::Info, ELogFamily::PlainText));
+    EXPECT_TRUE(rule->IsApplicable("some_service", ELogLevel::Info, ELogFamily::Structured));
+}
+
+TEST_F(TLoggingTest, RuleWithFamily)
+{
+    auto rule = New<TRuleConfig>();
+    rule->Load(ConvertToNode(TYsonString(TStringBuf(
+        R"({
+            exclude_categories = [ bus ];
+            min_level = info;
+            writers = [ some_writer ];
+            family = plain_text;
+        })"))));
+
+    EXPECT_TRUE(rule->IsApplicable("some_service", ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("some_service", ELogFamily::Structured));
+    EXPECT_FALSE(rule->IsApplicable("bus", ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("bus", ELogFamily::Structured));
+    EXPECT_FALSE(rule->IsApplicable("bus", ELogLevel::Debug, ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("bus", ELogLevel::Debug, ELogFamily::Structured));
+    EXPECT_FALSE(rule->IsApplicable("some_service", ELogLevel::Debug, ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("some_service", ELogLevel::Debug, ELogFamily::Structured));
+    EXPECT_TRUE(rule->IsApplicable("some_service", ELogLevel::Warning, ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("some_service", ELogLevel::Warning, ELogFamily::Structured));
+    EXPECT_TRUE(rule->IsApplicable("some_service", ELogLevel::Info, ELogFamily::PlainText));
+    EXPECT_FALSE(rule->IsApplicable("some_service", ELogLevel::Info, ELogFamily::Structured));
 }
 
 TEST_F(TLoggingTest, LogManager)