Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix log fl_ctx parsing #3179

Merged
merged 4 commits into from
Jan 28, 2025
Merged

Fix log fl_ctx parsing #3179

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
704cb65
fix fl_ctx parsing bug
SYangster Jan 24, 2025
04e46f8
add clarifying comment
SYangster Jan 24, 2025
65afcf9
Merge branch 'main' into fix_log_parse
SYangster Jan 27, 2025
6539e53
Merge branch 'main' into fix_log_parse
SYangster Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions nvflare/fuel/utils/log_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,24 +94,32 @@ def format(self, record):
record.fl_ctx = ""
record.identity = ""
message = record.getMessage()
# attempting to parse fl ctx key value pairs "[key0=value0, key1=value1,... ]: " from message
fl_ctx_match = re.search(r"\[(.*?)\]: ", message)
if fl_ctx_match:
fl_ctx_pairs = {
pair.split("=", 1)[0]: pair.split("=", 1)[1] for pair in fl_ctx_match.group(1).split(", ")
}
record.fl_ctx = fl_ctx_match[0][:-2]
record.identity = fl_ctx_pairs["identity"] # TODO add more values as attributes?
record.msg = message.replace(fl_ctx_match[0], "")
self._style._fmt = self.fmt
try:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you are trying to turn fl_ctx info in brackets into some JSON representation? Can you give some examples ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I added a clarifying comment.

We want to parse out the fl ctx part of the message into a separate log record field "[key0=value0, key1=value1,... ]: ". This PR adds case when bracket match is not in fl ctx format.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please give some examples. For instance, if the original log msg is like:

[identity=blue, peer=red, time=12345] something happened

It will become what?

Copy link
Collaborator Author

@SYangster SYangster Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

original message = "[identity=blue, peer=red, time=12345]: something happened"
after parsing -->
record.msg = "something happened"
record.fl_ctx = "[identity=blue, peer=red, time=12345]"
record.identity = "blue" (for now identity is the only key extracted into additional field based on feedback, but others can be added as needed)

If original message does not contain the bracketed pattern, then record.fl_ctx and record.identity are "".

These attributes can now be configured as desired in the log_config.json format string:
"fmt": "%(asctime)s - %(identity)s - %(name)s - %(levelname)s - %(fl_ctx)s - %(message)s"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, but what are the reasons to extract identity out and put it in the record?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for convenience, as suggested by Chester. Identity is useful for cases when the log output is combined, such as in the simulator case. But it can be removed if we think otherwise.

fl_ctx_pairs = {
pair.split("=", 1)[0]: pair.split("=", 1)[1] for pair in fl_ctx_match.group(1).split(", ")
}
record.fl_ctx = fl_ctx_match[0][:-2]
record.identity = fl_ctx_pairs.get("identity", "") # TODO add more values as attributes?
record.msg = message.replace(fl_ctx_match[0], "")
self._style._fmt = self.fmt
except:
# found brackets pattern, but was not fl_ctx format
self.remove_empty_placeholders()
else:
for placeholder in [
" %(fl_ctx)s -",
" %(identity)s -",
]: # TODO generalize this or add default values?
self._style._fmt = self._style._fmt.replace(placeholder, "")
self.remove_empty_placeholders()

return super().format(record)

def remove_empty_placeholders(self):
for placeholder in [
" %(fl_ctx)s -",
" %(identity)s -",
]: # TODO generalize this or add default values?
self._style._fmt = self._style._fmt.replace(placeholder, "")


class ColorFormatter(BaseFormatter):
def __init__(
Expand Down
Loading