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.

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

tools: add logfmt option for frr-reload.py #16796

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gtataranni
Copy link
Contributor

@gtataranni gtataranni commented Sep 11, 2024

Add the option of printing logs in logfmt format.
Adopt f-string for printing log message, so that appropriate formatting can be applied.

Additional machine readable information can be printed via the extra argument.
Example:

log.debug("exit context"), extra={"line": line, "ctx_keys": ctx_keys})
# ts=2024-09-10T16:10:54+02:00 level=DEBUG msg="exit context" line=exit ctx_keys=['interface t4_15867']
log.error(f"Failed to execute command {' '.join(cmd)}", extra={"cmd": cmd})
# ts=2024-09-10T16:17:07+02:00 level=ERROR msg="Failed to execute command foo bar" cmd=['foo', 'bar']

@ton31337
Copy link
Member

What is the real case it would be helpful?

@gtataranni
Copy link
Contributor Author

What is the real case it would be helpful?

This is useful for integrating with observability stacks, as logfmt is a common format for machine-readable logs. It enhances log parsing and visualization in tools like Prometheus and Grafana. We might consider using the logfmter library for a more standardized implementation. If we prefer using the library, I'll be happy to replace my implementation with that.

@ton31337
Copy link
Member

IMO it's better without 3rd party libraries in this place as it might complicate things (installing).

Two bullets to solve still regarding this PR:

  • Please separate completely this PR into separate commits (logfmt-related, and string formatting);
  • And make sure to apply frrbot (black) formatting to your code (each commit separately).

@ton31337
Copy link
Member

ci:rerun CI stuck

Adopt f-string for printing log message, so that appropriate formatting can be
applied.

Signed-off-by: Giovanni Tataranni <[email protected]>
Add the option of printing logs in logfmt format.

Additional machine readable information can be printed via the `extra`
argument.
Example:
```python
log.debug("exit context"), extra={"line": line, "ctx_keys": ctx_keys})

log.error(f"Failed to execute command {' '.join(cmd)}", extra={"cmd": cmd})
```

Signed-off-by: Giovanni Tataranni <[email protected]>
@gtataranni
Copy link
Contributor Author

gtataranni commented Jan 13, 2025

@ton31337 Changes done, please review

@ton31337 ton31337 self-requested a review January 14, 2025 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants