Skip to content

Commit a19e364

Browse files
garethsbZayanKhan-12claude
authored
Fix/negative logging categories (#519)
* Add support for negative logging categories in the log gateway Categories prefixed with '!' in the logging_categories setting specify messages that should not be written to the error log. A negative match takes precedence over any positive match, and when only negative categories are specified, all other messages remain pertinent, so e.g. ["!access"] logs everything except access messages. Behaviour is unchanged when logging_categories is omitted (everything is logged), empty (nothing is logged), or contains only positive categories. Closes #338 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Treat "!" as negated empty logging category Complete the negative-prefix rule for uncategorized messages, align example config comments with settings.h, and add coverage for same- category positive/negative conflict. Signed-off-by: Gareth Sylvester-Bradley <garethsb@nvidia.com> --------- Signed-off-by: Gareth Sylvester-Bradley <garethsb@nvidia.com> Co-authored-by: ZayanKhan-12 <108294002+ZayanKhan-12@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent dac770c commit a19e364

6 files changed

Lines changed: 133 additions & 3 deletions

File tree

‎Development/cmake/NmosCppTest.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ set(NMOS_CPP_TEST_NMOS_TEST_SOURCES
5757
nmos/test/json_validator_test.cpp
5858
nmos/test/jwt_generator_test.cpp
5959
nmos/test/jwt_validation_test.cpp
60+
nmos/test/log_gate_test.cpp
6061
nmos/test/mdns_test.cpp
6162
nmos/test/node_interfaces_test.cpp
6263
nmos/test/paging_utils_test.cpp

‎Development/nmos-cpp-node/config.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@
7979
//"logging_level": 0,
8080

8181
// logging_categories [registry, node]: array of logging categories to be included in the error log
82+
// categories prefixed with '!' are excluded, even if another category matches positively;
83+
// "!" excludes messages with no category; when only excluded categories are specified, all other log messages are included
8284
//"logging_categories": ["node_implementation"],
8385

8486
// Configuration settings and defaults for the NMOS APIs

‎Development/nmos-cpp-registry/config.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
//"logging_level": 0,
1515

1616
// logging_categories [registry, node]: array of logging categories to be included in the error log
17+
// categories prefixed with '!' are excluded, even if another category matches positively;
18+
// "!" excludes messages with no category; when only excluded categories are specified, all other log messages are included
1719
//"logging_categories": ["send_query_ws_events"],
1820

1921
// Configuration settings and defaults for the NMOS APIs

‎Development/nmos/log_gate.h‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <boost/algorithm/string/find_format.hpp>
88
#include <boost/algorithm/string/finder.hpp>
99
#include <boost/algorithm/string/formatter.hpp>
10+
#include <boost/algorithm/string/predicate.hpp>
1011
#include <boost/range/algorithm/find.hpp>
1112
#include <boost/range/algorithm/find_if.hpp>
1213
#include "nmos/log_model.h"
@@ -56,23 +57,58 @@ namespace nmos
5657
protected:
5758
virtual bool pertinent(const std::list<nmos::category>& categories) const
5859
{
60+
// logging_categories setting:
61+
// - omitted: log everything
62+
// - empty list: log nothing
63+
// - only positives: allowlist (log just those; "" includes uncategorized)
64+
// - only negatives ('!' prefixes): blocklist (log everything except those; "!" excludes uncategorized)
65+
// - mix: allowlist of the positives, but a negative match still wins
66+
5967
if (!model.settings.has_field(nmos::fields::logging_categories))
6068
{
6169
return true;
6270
}
6371

6472
const auto& pertinent_categories = nmos::fields::logging_categories(model.settings);
6573

74+
const auto is_negative = [](const web::json::value& category)
75+
{
76+
return boost::starts_with(category.as_string(), U("!"));
77+
};
78+
// true when the list is non-empty and every entry is negative (blocklist mode)
79+
const bool default_pertinent = 0 != pertinent_categories.size()
80+
&& pertinent_categories.end() == boost::range::find_if(pertinent_categories, [&](const web::json::value& category)
81+
{
82+
return !is_negative(category);
83+
});
84+
6685
if (categories.empty())
6786
{
87+
// "!" is the negative of "", i.e. excludes messages with no category
88+
static const auto no_category_negative = web::json::value::string(U("!"));
89+
if (pertinent_categories.end() != boost::range::find(pertinent_categories, no_category_negative))
90+
{
91+
return false;
92+
}
93+
6894
static const auto no_category = web::json::value::string(utility::string_t());
69-
return pertinent_categories.end() != boost::range::find(pertinent_categories, no_category);
95+
return default_pertinent || pertinent_categories.end() != boost::range::find(pertinent_categories, no_category);
7096
}
7197

7298
// this could be made more efficient if there may be many pertinent categories
73-
return categories.end() != boost::range::find_if(categories, [&](const nmos::category& c)
99+
if (categories.end() != boost::range::find_if(categories, [&](const nmos::category& c)
100+
{
101+
const auto category_negative = web::json::value::string(utility::s2us("!" + c));
102+
return pertinent_categories.end() != boost::range::find(pertinent_categories, category_negative);
103+
}))
104+
{
105+
return false;
106+
}
107+
108+
return default_pertinent || categories.end() != boost::range::find_if(categories, [&](const nmos::category& c)
74109
{
75-
return pertinent_categories.end() != boost::range::find(pertinent_categories, web::json::value::string(utility::s2us(c)));
110+
const auto category = web::json::value::string(utility::s2us(c));
111+
return pertinent_categories.end() != boost::range::find(pertinent_categories, category);
76112
});
77113
}
78114

‎Development/nmos/settings.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ namespace nmos
100100
const web::json::field_as_integer_or logging_level{ U("logging_level"), 0 }; // 0, rather than slog::severities::info or slog::nil_severity, just to avoid a #include
101101

102102
// logging_categories [registry, node]: array of logging categories to be included in the error log
103+
// categories prefixed with '!' are excluded, even if another category matches positively;
104+
// "!" excludes messages with no category; when only excluded categories are specified, all other log messages are included
103105
const web::json::field_as_array logging_categories{ U("logging_categories") }; // when omitted, all log messages are included
104106

105107
// Configuration settings and defaults for the NMOS APIs

‎Development/nmos/test/log_gate_test.cpp‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// The first "test" is of course whether the header compiles standalone
2+
#include "nmos/log_gate.h"
3+
4+
#include <sstream>
5+
#include "bst/test/test.h"
6+
7+
namespace
8+
{
9+
struct test_gate : nmos::experimental::log_gate
10+
{
11+
test_gate(std::ostream& error_log, std::ostream& access_log, nmos::experimental::log_model& model)
12+
: nmos::experimental::log_gate(error_log, access_log, model) {}
13+
using nmos::experimental::log_gate::pertinent;
14+
};
15+
}
16+
17+
////////////////////////////////////////////////////////////////////////////////////////////
18+
BST_TEST_CASE(testLogGatePertinentCategories)
19+
{
20+
using web::json::value_of;
21+
22+
std::ostringstream error_log;
23+
std::ostringstream access_log;
24+
nmos::experimental::log_model model;
25+
test_gate gate(error_log, access_log, model);
26+
27+
const std::list<nmos::category> no_categories;
28+
const std::list<nmos::category> access{ "access" };
29+
const std::list<nmos::category> send_query_ws_events{ "send_query_ws_events" };
30+
const std::list<nmos::category> both{ "send_query_ws_events", "access" };
31+
32+
// when logging_categories is omitted, all messages are pertinent
33+
BST_REQUIRE(gate.pertinent(no_categories));
34+
BST_REQUIRE(gate.pertinent(access));
35+
BST_REQUIRE(gate.pertinent(both));
36+
37+
// when logging_categories is empty, no messages are pertinent
38+
model.settings[nmos::fields::logging_categories] = web::json::value::array();
39+
BST_REQUIRE(!gate.pertinent(no_categories));
40+
BST_REQUIRE(!gate.pertinent(access));
41+
BST_REQUIRE(!gate.pertinent(both));
42+
43+
// positive categories select the messages to be logged
44+
model.settings[nmos::fields::logging_categories] = value_of({ U("send_query_ws_events") });
45+
BST_REQUIRE(!gate.pertinent(no_categories));
46+
BST_REQUIRE(gate.pertinent(send_query_ws_events));
47+
BST_REQUIRE(!gate.pertinent(access));
48+
BST_REQUIRE(gate.pertinent(both));
49+
50+
// the empty string selects messages with no category
51+
model.settings[nmos::fields::logging_categories] = value_of({ U("") });
52+
BST_REQUIRE(gate.pertinent(no_categories));
53+
BST_REQUIRE(!gate.pertinent(access));
54+
55+
// a category prefixed with '!' excludes matching messages, even if another
56+
// category matches positively
57+
model.settings[nmos::fields::logging_categories] = value_of({ U("send_query_ws_events"), U("!access") });
58+
BST_REQUIRE(gate.pertinent(send_query_ws_events));
59+
BST_REQUIRE(!gate.pertinent(access));
60+
BST_REQUIRE(!gate.pertinent(both));
61+
BST_REQUIRE(!gate.pertinent(no_categories));
62+
63+
// when only excluded categories are specified, all other messages are pertinent
64+
model.settings[nmos::fields::logging_categories] = value_of({ U("!access") });
65+
BST_REQUIRE(gate.pertinent(no_categories));
66+
BST_REQUIRE(gate.pertinent(send_query_ws_events));
67+
BST_REQUIRE(!gate.pertinent(access));
68+
BST_REQUIRE(!gate.pertinent(both));
69+
70+
// a negative match takes precedence over the same category listed positively
71+
model.settings[nmos::fields::logging_categories] = value_of({ U("access"), U("!access") });
72+
BST_REQUIRE(!gate.pertinent(access));
73+
BST_REQUIRE(!gate.pertinent(both));
74+
BST_REQUIRE(!gate.pertinent(no_categories));
75+
BST_REQUIRE(!gate.pertinent(send_query_ws_events));
76+
77+
// "!" excludes messages with no category (negation of "")
78+
model.settings[nmos::fields::logging_categories] = value_of({ U("!") });
79+
BST_REQUIRE(!gate.pertinent(no_categories));
80+
BST_REQUIRE(gate.pertinent(access));
81+
BST_REQUIRE(gate.pertinent(send_query_ws_events));
82+
83+
model.settings[nmos::fields::logging_categories] = value_of({ U("!"), U("!access") });
84+
BST_REQUIRE(!gate.pertinent(no_categories));
85+
BST_REQUIRE(!gate.pertinent(access));
86+
BST_REQUIRE(gate.pertinent(send_query_ws_events));
87+
}

0 commit comments

Comments
 (0)