Add support for minion id in log formats #68410
Open
+161
−38
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d178823 to
eb33d35
Compare
|
This is adding functionality. Shouldn't it go into |
24dbd09 to
fb1d491
Compare
Yes, you're right - I've rebased and re-targeted this to master. |
This should help disambiguate errors on busy masters when multiple minions are running jobs at once, especially in the case of errors whilst rendering pillars or bugs in custom code. Add JID/minion_id to console/file logs by default. Signed-off-by: Joe Groocock <jgroocock@cloudflare.com>
Updates the Cloudflare patch to add minion ID to master logs: * Reverts changes to the default log formats - as with the existing support for JID it should probably be up to user to set that rather than us changing the defaults and potentially causing breakage. * Updates code to get id and log_fmt_minion_id from context as data is no longer available in _log
Adds test for jid and minion_id in log format strings to ensure they appear correctly in the master log. Updates documentation to cover minion_id field and to recommend adding minion_id and jid to the log format config.
Fixes review issues - Unecessary None default in .get() - Defining new fmt strings as optional
fb1d491 to
7fd0357
Compare
|
Thanks for submitting this 😍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds the ability to have
%(minion_id)sfield in log lines when it available a request's context. This is similar to the existing%(jid)sfield.While some log messages already include minion id in the message portion, this isn't consistent or comprehensive - this should give people the option of including that information in log messages when available and in a more structured way.
Replaces the non-functional test for
%(jid)swith a test that covers that and%(minion_id)s.Updates docs to cover
%(minion_id)sand to recommend using both%(jid)sand%(minion_id)s- whilst it would be good to make that the default, as it was in the original patch, changing that would likely break people's log parsing/processing setups.Previous Behavior
It wasn't possible to include minion id as a field in log formats.
New Behavior
You can get the minion id in logs lines that wouldn't otherwise have that info:
2025-10-17 19:48:30,657 [salt.template :23 ][PROFILE ][11085][salt-minion-1] Time (in seconds) to render '/srv/pillar/common.sls' using 'jinja' renderer: 0.00047898292541503906Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes/No