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

Add logging to help diagnose ongoing issue with virus scanning #1241

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

jkempster34
Copy link
Contributor

@jkempster34 jkempster34 commented Nov 22, 2023

https://trello.com/c/9YZ5VItp

We have an ongoing issue where we are seeing the following errors related to missing files coming from the VirusScanWorker:

  • Errno::ENOENT: No such file or directory
  • StateMachines::InvalidTransition: Cannot transition state via :scanned_clean from :unscanned (Reason(s): File can't be blank)

Assets should go through the following stages:

  1. unscanned => saved to the file system and scanned by ClamAV in a background job.
  2. scanned => Saved to AWS.
  3. uploaded => removed from the file system.

However, it seems like the file has already been removed during the virus scanning stage.

This adds logging so that we can trace the history of assets which raise the above errors to try to diagnose the issue.


This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

We have an ongoing issue where we are seeing the following errors related to
missing files coming from the VirusScanWorker:
- Errno::ENOENT: No such file or directory
- StateMachines::InvalidTransition: Cannot transition state via :scanned_clean
  from :unscanned (Reason(s): File can't be blank)

Assets should go through the following stages:
1. unscanned => saved to the file system and scanned by ClamAV in a background
     job.
2. scanned  => Saved to AWS.
3. uploaded => removed from the file system.

However, it seems like the file has already been removed during the virus
scanning stage.

This adds logging so that we can trace the history of assets which raise the
above errors to try to diagnose the issue.
@jkempster34 jkempster34 changed the title Add logging to help diagnose ongoing issue Add logging to help diagnose ongoing issue with virus scanning Nov 22, 2023
Copy link
Contributor

@mike29736 mike29736 left a comment

Choose a reason for hiding this comment

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

As long as it's true about Asset#id, it looks good to me 👍

@@ -85,6 +85,11 @@ class Asset
scope :undeleted, -> { where(deleted_at: nil) }

state_machine :state, initial: :unscanned do
around_transition do |asset, transition, block|
Rails.logger.info("#{asset.id} - Asset#state_machine - event: #{transition.event}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with this state machine, so I'll assume that all Assets have an #id for all transitions (i.e. there's no point in using safe navigation here)

Copy link
Contributor

Choose a reason for hiding this comment

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

(And I see that Transition#event comes from https://github.com/state-machines/state_machines/blob/v0.6.0/lib/state_machines/transition.rb#L65, so that's nothing to worry about)

@jkempster34 jkempster34 merged commit a966c24 into main Nov 23, 2023
5 checks passed
@jkempster34 jkempster34 deleted the add-logging-for-debug branch November 23, 2023 09:09
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