Skip to content

Commit

Permalink
libstore/derivation-goal: avoid double-parsing of JSON messages
Browse files Browse the repository at this point in the history
To avoid that JSON messages are parsed twice in case of
remote builds with `ssh-ng://`, I split up the original
`handleJSONLogMessage` into three parts:

* `parseJSONMessage(const std::string&)` checks if it's a message in the
  form of `@nix {...}` and tries to parse it (and prints an error if the
  parsing fails).
* `handleJSONLogMessage(nlohmann::json&, ...)` reads the fields from the
  message and passes them to the logger.
* `handleJSONLogMessage(const std::string&, ...)` behaves as before, but
  uses the two functions mentioned above as implementation.

In case of `ssh-ng://`-logs the first two methods are invoked manually.
  • Loading branch information
Ma27 committed Feb 19, 2022
1 parent 3d525e9 commit 8a83e34
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 39 deletions.
14 changes: 8 additions & 6 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1207,12 +1207,14 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
if (hook && fd == hook->fromHook.readSide.get()) {
for (auto c : data)
if (c == '\n') {
auto s = handleJSONLogMessage(currentHookLine, worker.act, hook->activities, true);
if (s && !isWrittenToLog && logSink) {
auto json = nlohmann::json::parse(std::string(currentHookLine, 5));
if (json["type"] == resBuildLogLine) {
auto f = json["fields"];
(*logSink)((f.size() > 0 ? f.at(0).get<std::string>() : "") + "\n");
auto json = parseJSONMessage(currentHookLine);
if (json) {
auto s = handleJSONLogMessage(*json, worker.act, hook->activities, true);
if (s && !isWrittenToLog && logSink) {
if ((*json)["type"] == resBuildLogLine) {
auto f = (*json)["fields"];
(*logSink)((f.size() > 0 ? f.at(0).get<std::string>() : "") + "\n");
}
}
}
currentHookLine.clear();
Expand Down
78 changes: 45 additions & 33 deletions src/libutil/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,51 +266,63 @@ static Logger::Fields getFields(nlohmann::json & json)
return fields;
}

bool handleJSONLogMessage(const std::string & msg,
const Activity & act, std::map<ActivityId, Activity> & activities, bool trusted)
std::optional<nlohmann::json> parseJSONMessage(const std::string & msg)
{
if (!hasPrefix(msg, "@nix ")) return false;

if (!hasPrefix(msg, "@nix ")) return std::nullopt;
try {
auto json = nlohmann::json::parse(std::string(msg, 5));

std::string action = json["action"];

if (action == "start") {
auto type = (ActivityType) json["type"];
if (trusted || type == actFileTransfer)
activities.emplace(std::piecewise_construct,
std::forward_as_tuple(json["id"]),
std::forward_as_tuple(*logger, (Verbosity) json["level"], type,
json["text"], getFields(json["fields"]), act.id));
}
return nlohmann::json::parse(std::string(msg, 5));
} catch (std::exception & e) {
printError("bad JSON log message from builder: %s", e.what());
}
return std::nullopt;
}

else if (action == "stop")
activities.erase((ActivityId) json["id"]);
bool handleJSONLogMessage(nlohmann::json & json,
const Activity & act, std::map<ActivityId, Activity> & activities,
bool trusted)
{
std::string action = json["action"];

if (action == "start") {
auto type = (ActivityType) json["type"];
if (trusted || type == actFileTransfer)
activities.emplace(std::piecewise_construct,
std::forward_as_tuple(json["id"]),
std::forward_as_tuple(*logger, (Verbosity) json["level"], type,
json["text"], getFields(json["fields"]), act.id));
}

else if (action == "result") {
auto i = activities.find((ActivityId) json["id"]);
if (i != activities.end())
i->second.result((ResultType) json["type"], getFields(json["fields"]));
}
else if (action == "stop")
activities.erase((ActivityId) json["id"]);

else if (action == "setPhase") {
std::string phase = json["phase"];
act.result(resSetPhase, phase);
}
else if (action == "result") {
auto i = activities.find((ActivityId) json["id"]);
if (i != activities.end())
i->second.result((ResultType) json["type"], getFields(json["fields"]));
}

else if (action == "msg") {
std::string msg = json["msg"];
logger->log((Verbosity) json["level"], msg);
}
else if (action == "setPhase") {
std::string phase = json["phase"];
act.result(resSetPhase, phase);
}

} catch (std::exception & e) {
printError("bad JSON log message from builder: %s", e.what());
else if (action == "msg") {
std::string msg = json["msg"];
logger->log((Verbosity) json["level"], msg);
}

return true;
}

bool handleJSONLogMessage(const std::string & msg,
const Activity & act, std::map<ActivityId, Activity> & activities, bool trusted)
{
auto json = parseJSONMessage(msg);
if (!json) return false;

return handleJSONLogMessage(*json, act, activities, trusted);
}

Activity::~Activity()
{
try {
Expand Down
8 changes: 8 additions & 0 deletions src/libutil/logging.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include "error.hh"
#include "config.hh"

#include <nlohmann/json.hpp>

namespace nix {

typedef enum {
Expand Down Expand Up @@ -166,6 +168,12 @@ Logger * makeSimpleLogger(bool printBuildLogs = true);

Logger * makeJSONLogger(Logger & prevLogger);

std::optional<nlohmann::json> parseJSONMessage(const std::string & msg);

bool handleJSONLogMessage(nlohmann::json & json,
const Activity & act, std::map<ActivityId, Activity> & activities,
bool trusted);

bool handleJSONLogMessage(const std::string & msg,
const Activity & act, std::map<ActivityId, Activity> & activities,
bool trusted);
Expand Down

0 comments on commit 8a83e34

Please sign in to comment.