Skip to content

Commit ea21d99

Browse files
entry: use map for metadata in ctor
Transition the entry constructor to use the `map<string,string>` rather than the `vector<string>`. Tested: UTs pass and daemon still creates logs. Signed-off-by: Patrick Williams <[email protected]> Change-Id: Icf3ead163cc99a1498734d33335fb6da52e6c98c
1 parent 5402fa6 commit ea21d99

8 files changed

+31
-39
lines changed

elog_entry.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class Entry : public EntryIfaces
6565
*/
6666
Entry(sdbusplus::bus_t& bus, const std::string& objectPath, uint32_t idErr,
6767
uint64_t timestampErr, Level severityErr, std::string&& msgErr,
68-
std::vector<std::string>&& additionalDataErr,
68+
std::map<std::string, std::string>&& additionalDataErr,
6969
AssociationList&& objects, const std::string& fwVersion,
7070
const std::string& filePath, internal::Manager& parent) :
7171
EntryIfaces(bus, objectPath.c_str(), EntryIfaces::action::defer_emit),
@@ -76,8 +76,8 @@ class Entry : public EntryIfaces
7676
timestamp(timestampErr, true);
7777
updateTimestamp(timestampErr, true);
7878
message(std::move(msgErr), true);
79-
additionalData(std::move(additionalDataErr), true);
80-
additionalData2(util::additional_data::parse(additionalData()), true);
79+
additionalData2(std::move(additionalDataErr), true);
80+
additionalData(util::additional_data::combine(additionalData2()), true);
8181
associations(std::move(objects), true);
8282
// Store a copy of associations in case we need to recreate
8383
assocs = associations();

lib/lg2_commit.cpp

+2-11
Original file line numberDiff line numberDiff line change
@@ -107,18 +107,9 @@ static auto data_from_json(sdbusplus::exception::generated_event_base& t)
107107
}
108108

109109
auto extractEvent(sdbusplus::exception::generated_event_base&& t)
110-
-> std::tuple<std::string, Entry::Level, std::vector<std::string>>
110+
-> std::tuple<std::string, Entry::Level, std::map<std::string, std::string>>
111111
{
112-
auto data = data_from_json(t);
113-
std::vector<std::string> additional_data = {};
114-
115-
for (auto& [key, data] : data)
116-
{
117-
additional_data.emplace_back(key + "=" + data);
118-
}
119-
120-
return {t.name(), severity_from_syslog(t.severity()),
121-
std::move(additional_data)};
112+
return {t.name(), severity_from_syslog(t.severity()), data_from_json(t)};
122113
}
123114

124115
} // namespace details

lib/lg2_commit.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ using Entry = sdbusplus::client::xyz::openbmc_project::logging::Entry<>;
1717
* @return A tuple containing the message, level, and additional data.
1818
*/
1919
auto extractEvent(sdbusplus::exception::generated_event_base&&)
20-
-> std::tuple<std::string, Entry::Level, std::vector<std::string>>;
20+
-> std::tuple<std::string, Entry::Level,
21+
std::map<std::string, std::string>>;
2122

2223
} // namespace lg2::details

log_manager.cpp

+5-7
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,12 @@ void Manager::_commit(uint64_t transactionId [[maybe_unused]],
193193

194194
sd_journal_close(j);
195195
}
196-
createEntry(errMsg, errLvl, additionalData);
196+
createEntry(errMsg, errLvl, util::additional_data::parse(additionalData));
197197
}
198198

199199
auto Manager::createEntry(
200200
std::string errMsg, Entry::Level errLvl,
201-
std::vector<std::string> additionalData,
201+
std::map<std::string, std::string> additionalData,
202202
const FFDCEntries& ffdc) -> sdbusplus::message::object_path
203203
{
204204
if (!Extensions::disableDefaultLogCaps())
@@ -234,7 +234,8 @@ auto Manager::createEntry(
234234
auto objPath = std::string(OBJ_ENTRY) + '/' + std::to_string(entryId);
235235

236236
AssociationList objects{};
237-
processMetadata(errMsg, additionalData, objects);
237+
auto additionalDataVec = util::additional_data::combine(additionalData);
238+
processMetadata(errMsg, additionalDataVec, objects);
238239

239240
auto e = std::make_unique<Entry>(
240241
busLog, objPath, entryId,
@@ -706,10 +707,7 @@ auto Manager::create(const std::string& message, Entry::Level severity,
706707
const std::map<std::string, std::string>& additionalData,
707708
const FFDCEntries& ffdc) -> sdbusplus::message::object_path
708709
{
709-
// Convert the map into a vector of "key=value" strings
710-
auto ad = util::additional_data::combine(additionalData);
711-
712-
return createEntry(message, severity, ad, ffdc);
710+
return createEntry(message, severity, additionalData, ffdc);
713711
}
714712

715713
} // namespace internal

log_manager.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ class Manager : public details::ServerObject<details::ManagerIface>
270270
* vector.
271271
*/
272272
auto createEntry(std::string errMsg, Entry::Level errLvl,
273-
std::vector<std::string> additionalData,
273+
std::map<std::string, std::string> additionalData,
274274
const FFDCEntries& ffdc = FFDCEntries{})
275275
-> sdbusplus::message::object_path;
276276

test/elog_quiesce_test.cpp

+14-13
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ TEST_F(TestQuiesceOnError, testNoCallout)
4141
std::string message{"test error"};
4242
std::string fwLevel{"level42"};
4343
std::string path{"/tmp/99"};
44-
std::vector<std::string> testData{"no", "callout"};
44+
std::map<std::string, std::string> testData{{"no", "no"},
45+
{"callout", "callout"}};
4546
phosphor::logging::AssociationList associations{};
4647

4748
Entry elog{mockedBus,
@@ -67,9 +68,9 @@ TEST_F(TestQuiesceOnError, testCallout)
6768
std::string message{"test error"};
6869
std::string fwLevel{"level42"};
6970
std::string path{"/tmp/99"};
70-
std::vector<std::string> testData{
71-
"CALLOUT_INVENTORY_PATH=/xyz/openbmc_project/inventory/system/chassis/"
72-
"motherboard/powersupply0/"};
71+
std::map<std::string, std::string> testData{
72+
{"CALLOUT_INVENTORY_PATH",
73+
"/xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0/"}};
7374
phosphor::logging::AssociationList associations{};
7475

7576
Entry elog{mockedBus,
@@ -95,9 +96,9 @@ TEST_F(TestQuiesceOnError, testBlockingErrorsCreated)
9596
std::string message{"test error"};
9697
std::string fwLevel{"level42"};
9798
std::string path{"/tmp/99"};
98-
std::vector<std::string> testData{
99-
"CALLOUT_INVENTORY_PATH=/xyz/openbmc_project/inventory/system/chassis/"
100-
"motherboard/powersupply0/"};
99+
std::map<std::string, std::string> testData{
100+
{"CALLOUT_INVENTORY_PATH",
101+
"/xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0/"}};
101102
phosphor::logging::AssociationList associations{};
102103

103104
// Ensure D-Bus object created for this blocking error
@@ -152,9 +153,9 @@ TEST_F(TestQuiesceOnError, testBlockingErrorsResolved)
152153
std::string message{"test error"};
153154
std::string fwLevel{"level42"};
154155
std::string path{"/tmp/99"};
155-
std::vector<std::string> testData{
156-
"CALLOUT_INVENTORY_PATH=/xyz/openbmc_project/inventory/system/chassis/"
157-
"motherboard/powersupply0/"};
156+
std::map<std::string, std::string> testData{
157+
{"CALLOUT_INVENTORY_PATH",
158+
"/xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0/"}};
158159
phosphor::logging::AssociationList associations{};
159160

160161
// Ensure D-Bus object created for this blocking error
@@ -211,9 +212,9 @@ TEST_F(TestQuiesceOnError, testBlockingErrorTwice)
211212
std::string message{"test error"};
212213
std::string fwLevel{"level42"};
213214
std::string path{"/tmp/99"};
214-
std::vector<std::string> testData{
215-
"CALLOUT_INVENTORY_PATH=/xyz/openbmc_project/inventory/system/chassis/"
216-
"motherboard/powersupply0/"};
215+
std::map<std::string, std::string> testData{
216+
{"CALLOUT_INVENTORY_PATH",
217+
"/xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0/"}};
217218
phosphor::logging::AssociationList associations{};
218219

219220
// Ensure D-Bus object created for this blocking error

test/elog_update_ts_test.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ TEST(TestUpdateTS, testChangeResolved)
6161
std::string message{"test error"};
6262
std::string fwLevel{"level42"};
6363
std::string path{"/tmp/99"};
64-
std::vector<std::string> testData{"additional", "data"};
64+
std::map<std::string, std::string> testData{{"additional", "data"}};
6565
phosphor::logging::AssociationList associations{};
6666

6767
Entry elog{bus,
@@ -132,7 +132,7 @@ TEST(TestResolveProhibited, testResolveFlagChange)
132132
std::string message{"test error"};
133133
std::string fwLevel{"level42"};
134134
std::string path{"/tmp/99"};
135-
std::vector<std::string> testData{"additional", "data"};
135+
std::map<std::string, std::string> testData{{"additional", "data"}};
136136
phosphor::logging::AssociationList associations{};
137137

138138
Entry elog{bus,

test/serialization_test_properties.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ TEST_F(TestSerialization, testProperties)
1313
{
1414
auto id = 99;
1515
phosphor::logging::AssociationList assocations{};
16-
std::vector<std::string> testData{"additional=1", "data=yes"};
16+
std::map<std::string, std::string> testData = {{"additional", "1"},
17+
{"data", "yes"}};
1718
uint64_t timestamp{100};
1819
std::string message{"test error"};
1920
std::string fwLevel{"level42"};

0 commit comments

Comments
 (0)