Skip to content

Suppress unnecessary error log for ERROR_NOT_SUPPORTED cases #215

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Suppress unnecessary error log for ERROR_NOT_SUPPORTED cases #215

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ef1ae3
fix JSON parser to correctly handle numeric values
bobsira Jul 22, 2025
40bf213
moving the parser change to a different PR
bobsira Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions LogMonitor/src/LogMonitor/LogFileMonitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,13 +701,21 @@ LogFileMonitor::InitializeDirectoryChangeEventsQueue()
nullptr);
if (logFile == INVALID_HANDLE_VALUE)
{
logWriter.TraceError(
Utility::FormatString(
L"Error in log file monitor. Failed to open file %ws. Error = %d",
fileName.c_str(),
GetLastError()
).c_str()
);
//
// Added a conditional check to suppress logging for ERROR_NOT_SUPPORTED (benign error).
// This prevents unnecessary log pollution while still logging other actionable errors.
// https://github.com/microsoft/windows-container-tools/issues/125#issuecomment-3056545183
//
if (GetLastError() != ERROR_NOT_SUPPORTED)
{
logWriter.TraceError(
Utility::FormatString(
L"Error in log file monitor. Failed to open file %ws. Error = %d",
fileName.c_str(),
GetLastError()
).c_str()
);
}
Comment on lines +709 to +718
Copy link
Contributor

@mloskot mloskot Jul 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@bobsira I think similar workaround is needed here

windows-container-tools/LogMonitor/src/LogMonitor/LogFileMonitor.cpp

Lines 1082 to 1088 in 09983f6

logWriter.TraceError(
Utility::FormatString(
L"Error in log file monitor. Failed to query file ID. File: %ws. Error: %d",
fullLongPath.c_str(),
status
).c_str()
);

I'm testing your LogMonitor.exe build you linked in #214 (comment) and

image

The waitInSeconds: 10 does not seem to help delay the files query in this particular case.

My LogMonitorConfig.json is this

{
  "LogConfig": {
    "sources": [
      {
        "includeSubdirectories": false,
        "filter": "*.log",
        "directory": "C:\\Logs",
        "waitInSeconds": 10,
        "type": "File",
        "includeFileNames": true
      }
    ]
  }
}

The fix for ordering of the waitInSeconds seem to work.

Copy link
Contributor Author

@bobsira bobsira Jul 31, 2025

Choose a reason for hiding this comment

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

@mloskot please confirm if the issue still exists in this binary -> https://minikubevhdimagebuider.blob.core.windows.net/versions/LogMonitor.exe

It has the change you've suggested. if it's there we can move this fix to a different PR and let this handle the parser fix

Copy link
Contributor

@mloskot mloskot Aug 3, 2025

Choose a reason for hiding this comment

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

@bobsira Hmm, I've just tested LogMonitor.exe from that URL

Invoke-WebRequest -Uri 'https://minikubevhdimagebuider.blob.core.windows.net/versions/LogMonitor.exe' -OutFile C:/LogMonitor.exe -UseBasicParsing

and this version is reported inside the container - am I using the right custom ad-hoc build of yours?

image

but I can't see any difference

image

Here is my config inside the container

image

and logs location

image

Copy link
Contributor Author

@bobsira bobsira Aug 5, 2025

Choose a reason for hiding this comment

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

Hmmm, strange that should be the binary with the change. I'll go ahead and split the PR for now. I have a feeling this issue might be coming from another place in the code as well so let's have it addressed in a different PR.

Copy link
Contributor

@mloskot mloskot Aug 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good idea. Let's take baby steps. The overall direction seems fine though 😊


//
// Ignore failure and continue. In the worst case we will
Expand Down Expand Up @@ -1079,13 +1087,19 @@ LogFileMonitor::LogFileAddEventHandler(
if (status != ERROR_SUCCESS)
{
status = GetLastError();
logWriter.TraceError(
Utility::FormatString(
L"Error in log file monitor. Failed to query file ID. File: %ws. Error: %d",
fullLongPath.c_str(),
status
).c_str()
);
// Added a conditional check to suppress logging for ERROR_NOT_SUPPORTED (benign error).
// This prevents unnecessary log pollution while still logging other actionable errors.
// https://github.com/microsoft/windows-container-tools/issues/125#issuecomment-3056545183
if (status != ERROR_NOT_SUPPORTED)
{
logWriter.TraceError(
Utility::FormatString(
L"Error in log file monitor. Failed to query file ID for File: %ws. Error: %d",
fullLongPath.c_str(),
status
).c_str()
);
}
}

status = ReadLogFile(logFileInfo);
Expand Down
Loading