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

Conditional log message evaluation #316

Closed
elementbound opened this issue Oct 28, 2024 · 0 comments · Fixed by #335
Closed

Conditional log message evaluation #316

elementbound opened this issue Oct 28, 2024 · 0 comments · Fixed by #335
Labels
refactor Same functionality with improved code quality

Comments

@elementbound
Copy link
Contributor

✨ Description

Currently most, if not all, log messages are using string interpolation in the message param:

	_logger.trace("Received sample: t1=%s; t2=%s; t3=%s; t4=%s; theta=%sms; delta=%sms" % [
		sample.ping_sent, sample.ping_received, sample.pong_sent, sample.pong_received,
		sample.get_offset() * 1000., sample.get_rtt() * 1000.
	])

This means that even if the message is not logged, the string will be evaluated. This can waste some processing time for messages that are never seen. To improve on this, the following is my proposal:

	_logger.trace("Received sample: t1=%s; t2=%s; t3=%s; t4=%s; theta=%sms; delta=%sms", [
		sample.ping_sent, sample.ping_received, sample.pong_sent, sample.pong_received,
		sample.get_offset() * 1000., sample.get_rtt() * 1000.
	])

Notice the change from "..." % [...] to "...", [...]. Instead of doing the string interpolation in the message param, the logger receives both the string and the values to substitute. The string interpolation is only performed when the message actually gets logged.

Use case

This would be used in netfox addons for logging messages, so would affect all projects using netfox.

Upcoming features could also include more toggleabble logging without being concerned about wasted performance. The above time sync snippet is a good example.

Distribution

This would be an update in netfox.internals.

Notes

Code parts to consider:

  • _NetfoxLogger in netfox.internals
  • Search results for _NetfoxLogger.for_ and _NetfoxLogger.new - these should find all usages of the class

The change could be updating all logging methods to this signature:

func info(message: String, values: Array = [])

Make sure that no interpolation is done if the array is empty! This should be a backwards-compatible refactor.

@elementbound elementbound added the feature New feature or request label Oct 28, 2024
@elementbound elementbound added refactor Same functionality with improved code quality and removed feature New feature or request labels Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Same functionality with improved code quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant