Verbose undistinguished --stats-log=true counters (as found in CI configs) output can hide nearby messages

[huonw]

Describe the bug

We're onboarding new people to use our pants tooling, and one rough edge is that CI runs with [stats].log = true configured, which is helpful for experienced users to reference occasionally, but is usually just noise, and especially so for new users.

This logging output can end up jammed up against other real/relevant output of the tool, and thus someone scanning for messages can easily miss it. For example, the first thing CI usually does is ./pants tailor --check, and the output when there's something missing looks like:

Would update BUILD:
  - Add python_sources target root

To fix `tailor` failures, run `./pants tailor`.
10:05:26.27 [INFO] Counters:
  backtrack_attempts: 0
  local_cache_read_errors: 0
  local_cache_requests: 0
  local_cache_requests_cached: 0
...

Colored:

It's easy to miss the prompt to run ./pants tailor. Someone new to using pants wouldn't be aware of ./pants tailor as a thing, and someone more experienced can still benefit from the clear reminder.

This issue is a bit squishy/not-really-concrete bug, but I think there's some potential ways to improve this:

• tweak the styling of debug-style logging like this counters output (automatically applies across all such messages in CI):
  • more spacing around debug-style logging
  • use colours to deemphasise it, e.g. 'fade' it by setting the colour to grey
• use colours more in messages (requires updating individual messages, of which an intro command like ./pants tailor is potentially a single high-value example and similarly for update-build-files), e.g.:
  • add a help heading: help: to fix `tailor` failures, run ... prompt similar to rustc, and the heading can be emphasised
  • add colours within the "To fix ..." line itself
• another way to get the benefit of the [stats].log = true when required, without pessimising the common case: is it possible to write only to pants.log?

For instance, as a sketch:

Pants version

• 2.12.0
• 2.14.0a0

OS
macOS, linux

Additional info
Add any other information about the problem here, such as attachments or links to gists, if relevant.

[Eric-Arellano]

This is an awesome idea! For the tailor recommendation, would you want to implement that? You simply use console.blue() for example:

pants/src/python/pants/core/goals/tailor.py

Lines 621 to 633 in 107d56b

	for build_file_path, ptgts in ptgts_by_build_file.items():
	formatted_changes = "\n".join(
	f" - Add {console.green(ptgt.type_alias)} target {console.cyan(ptgt.name)}"
	for ptgt in ptgts
	)
	if build_file_path in updated_build_files:
	verb = "Would update" if tailor_subsystem.check else "Updated"
	else:
	verb = "Would create" if tailor_subsystem.check else "Created"
	console.print_stdout(f"{verb} {console.blue(build_file_path)}:\n{formatted_changes}")
	if tailor_subsystem.check:
	console.print_stdout(f"\nTo fix `tailor` failures, run `{bin_name()} tailor`.")

Making the counter log message gray doesn't have something like that helper function. We would need to change the string inside logger.info() by calling functions from the third-party library colors, e.g. colors.yellow():

pants/src/python/pants/goal/stats_aggregator.py

Line 96 in 107d56b

logger.info(f"Counters:\n{counter_lines}")

The tricky thing is if [GLOBAL].colors = false, we must not use colors. Hence, Console has this:

pants/src/python/pants/engine/console.py

Lines 95 to 119 in 107d56b

	@property
	def use_colors(self):
	return self._use_colors
	def _safe_color(self, text: str, color: Callable[[str], str]) -> str:
	"""We should only output color when the global flag --colors is enabled."""
	return color(text) if self._use_colors else text
	def blue(self, text: str) -> str:
	return self._safe_color(text, blue)
	def cyan(self, text: str) -> str:
	return self._safe_color(text, cyan)
	def green(self, text: str) -> str:
	return self._safe_color(text, green)
	def magenta(self, text: str) -> str:
	return self._safe_color(text, magenta)
	def red(self, text: str) -> str:
	return self._safe_color(text, red)
	def yellow(self, text: str) -> str:
	return self._safe_color(text, yellow)

We need something like safe_yellow(s: str, global_options: GlobalOptions) defined in maybe strutil.py. Then logs can start to use that :D That would be really cool! Contribution welcomed :)

[huonw]

This has been much improved by @AlexTereshenkov's #20512: we have this in our pants.ci.toml file:

[stats]
log = true
output_file = ".pants.d/stats.txt"

and just record that as build metadata/artifacts (along with the other ./.pants.d logs), for analysis/reference in the rare case it is required. As in, the stats output is no longer directly visible. I find this solution better for day-to-day usage that just having more colours would be.

I'm going to close this.

If we ever do a broader diagnostics project (e.g. #18423), I'm assuming we'd improve the behaviour here "coincidentally" too.