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

Reduce log spam if network monitor fails #1494

Merged
merged 2 commits into from
Oct 27, 2024

Conversation

crschardt
Copy link
Contributor

@crschardt crschardt commented Oct 27, 2024

This prevents spamming of the logs by the network interface device monitor by:

  • checking to make sure the device file exists before starting the monitoring task
  • only logging once if it throws an exception, but keep trying in case the exception is transient

This prevents spamming of the logs by the network interface device
monitor by:

- checking to make sure the device file exists before starting the
    monitoring task
- cancelling the task if it throws an exception
@crschardt crschardt requested a review from a team as a code owner October 27, 2024 19:29
logger.error("Could not check network status for " + devName, e);
// Cancel the task to avoid spamming the logs
logger.info("Cancelling task " + taskName);
TimedTaskManager.getInstance().cancelTask(taskName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just reviewing the diff - if this task is canceled, do we have a way to bring it back if the interface comes back up?

Copy link
Contributor Author

@crschardt crschardt Oct 27, 2024

Choose a reason for hiding this comment

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

The cancellation is only triggered when the this throws an error, which I think only happens if Files.readstring() fails. An alternative is to keep the task but only log an error once. That way, if it stops throwing, the monitor will continue working. I'll push a commit with this logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Which I think could only happen if the file doesn't exist? Maybe we need something like ROS_LOG_ONCE lol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's my assumption. But maybe there could be another filesystem problem that could cause it to transiently fail. The only downside of not cancelling is that it leaves the monitor running and triggering an exception every 5 seconds.

@crschardt crschardt merged commit acbae88 into master Oct 27, 2024
31 checks passed
@crschardt crschardt deleted the avoid-log-spam-with-device-monitor branch October 27, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants