Skip to content

Release v2.2.0-rc#222

Merged
bobsira merged 10 commits into
mainfrom
version2.2.0-rc
Apr 20, 2026
Merged

Release v2.2.0-rc#222
bobsira merged 10 commits into
mainfrom
version2.2.0-rc

Conversation

@bobsira
Copy link
Copy Markdown
Contributor

@bobsira bobsira commented Apr 9, 2026

Merging version2.2.0-rc into main for the v2.2.0 release.

CharityKathure and others added 4 commits April 15, 2025 17:50
* boost json parser

Signed-off-by: Charity Kathure <ckathure@microsoft.com>

Co-authored-by: Charity Kathure <ckathure@microsoft.com>
Signed-off-by: Charity Kathure <ckathure@microsoft.com>

* review: fix memory safety, error handling, and build improvements

- Use unique_ptr for channels vector in handleEventLog to eliminate manual
  delete and prevent memory leak on early return paths
- SanitizeJson no longer replaces log content with placeholder on failure;
  leaves str unchanged and logs via logWriter instead
- getJsonStringCaseInsensitive adds required param (default false) so optional
  fields don't emit spurious errors; required fields (name, level, directory,
  providerName, providerGuid) now pass true
- Fix inconsistent nlohmann::json vs json alias in handleETWLog/handleProcessLog
- build.cmd: detect cmake from VS or vcpkg when not on PATH; use goto to avoid
  delayed expansion issues with nested if blocks
- docs: remove leading space from "Information" level in README example

---------

Signed-off-by: Charity Kathure <ckathure@microsoft.com>
Co-authored-by: Charity Kathure <ckathure@microsoft.com>
Co-authored-by: Bob Sira <Bob.Sira@microsoft.com>
The SDL pipeline was failing because nlohmann/json.hpp was not available
when MSBuild compiled the project. Added vcpkg setup and nlohmann-json
installation steps before the build step.
Comment thread LogMonitor/src/LogMonitor/JsonProcessor.cpp Fixed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR merges the v2.2.0-rc branch into main for the v2.2.0 release, including a configuration parsing refactor (moving from the custom JSON parser to nlohmann/json) and updates to the build/CI tooling.

Changes:

  • Replace the legacy config parsing implementation with a new JsonProcessor based on nlohmann::json, and update the entry point to use it.
  • Update JSON sanitization and add UTF-8 string/wstring conversion helpers.
  • Add CMake + vcpkg build support and update Azure Pipelines / compliance workflow to install dependencies and build via CMake.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 23 comments.

Show a summary per file
File Description
LogMonitor/src/LogMonitor/Utility.h Adds string conversion helpers used by JSON sanitization.
LogMonitor/src/LogMonitor/Utility.cpp Replaces manual JSON escaping with nlohmann::json-based escaping + conversions.
LogMonitor/src/LogMonitor/pch.h Adds nlohmann/json include and wires in JsonProcessor.
LogMonitor/src/LogMonitor/Parser/LoggerSettings.h Adds EventLogChannel constructors used by new parsing path.
LogMonitor/src/LogMonitor/Main.cpp Switches from OpenConfigFile to the new ReadConfigFile.
LogMonitor/src/LogMonitor/JsonProcessor.h/.cpp New config parsing implementation built on nlohmann::json.
LogMonitor/src/CMakeLists.txt / LogMonitor/CMakeLists.txt / LogMonitor/LogMonitorTests/CMakeLists.txt Introduces CMake build definitions for app + tests.
azure-pipelines.yml Reworks CI to install vcpkg deps and build/test via CMake.
build.cmd Replaces msbuild-only build with vcpkg + CMake build script.
LogMonitor/docs/README.md Fixes a JSON sample typo (" Information""Information").
Test project files (LogMonitorTests*, JsonProcessorTests.cpp) Removes legacy config parser tests and adds a (currently empty) placeholder for new tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread LogMonitor/src/LogMonitor/Utility.cpp Outdated
Comment thread LogMonitor/src/LogMonitor/Utility.cpp
Comment thread LogMonitor/src/LogMonitor/Utility.h Outdated
Comment thread LogMonitor/src/LogMonitor/EtwMonitor.cpp Outdated
Comment thread LogMonitor/src/LogMonitor/JsonProcessor.cpp Outdated
Comment thread LogMonitor/LogMonitorTests/CMakeLists.txt Outdated
Comment thread azure-pipelines.yml Outdated
Comment thread LogMonitor/LogMonitorTests/JsonProcessorTests.cpp Outdated
Comment thread LogMonitor/src/LogMonitor/JsonProcessor.cpp
Comment thread LogMonitor/CMakeLists.txt Outdated
Bob Sira added 2 commits April 13, 2026 13:37
- Fix boolean attributes stored as wstring* instead of bool* in
  JsonProcessor, causing incorrect EventFormatMultiLine/StartAtOldestRecord
  values after parsing
- Fix eventFormatMultiLine default from false to true (documented default)
- Make channel level optional in EventLog source (defaults to Error)
- Fix processSources to skip invalid sources and continue (resilient mode)
- Add missing waitInSeconds parsing in handleFileLog
- Fix path injection in Main.cpp: validate config filename and anchor to
  exe directory using GetModuleFileNameW/PathCombineW
- Fix %s -> %hs format specifier for narrow string in EtwMonitor.cpp
- Rename Utility string conversion functions to PascalCase
- Fix CMake toolchain file setup to run before project() call
- Fix output directory to use generator expression for multi-config layout
- Add JsonProcessorTests covering case-insensitive keys, optional field
  defaults, waitInSeconds parsing, and invalid source resilience
The VS .sln build compiles sources by #including them into
LogMonitorTests.cpp. JsonProcessor.cpp was missing from that list,
causing an unresolved external symbol for ReadConfigFile(PWCHAR,...).

- Add #include of JsonProcessor.cpp to LogMonitorTests.cpp
- Add #include "JsonProcessor.h" in JsonProcessor.cpp so all function
  declarations (with correct defaults) are visible when the file is
  #included as a translation unit, fixing C3861/C2660 errors
#include "../src/LogMonitor/EtwMonitor.cpp"
#include "../src/LogMonitor/EventMonitor.cpp"
#include "../src/LogMonitor/JsonFileParser.cpp"
#include "../src/LogMonitor/JsonProcessor.cpp"
Bob Sira added 3 commits April 13, 2026 17:30
Both x64 and x86 jobs now install nlohmann-json via vcpkg before
building. --no-binarycaching and VCPKG_BINARY_SOURCES=clear prevent
vcpkg from downloading PowerShell Core (causing 504 errors in CI).
- Enable C++17 in CMakeLists.txt (fixes STL4038 for <variant>)
- Remove unused hr, bytesWritten, hSubscription, eventsCount, szBuf
  local variables (C4189)
- Replace E_FAIL (HRESULT) returns from DWORD functions with
  ERROR_UNHANDLED_EXCEPTION (C4245)
- Mark unused Buffer and Event parameters as commented-out (C4100)
- Rename shadowed 'status' variable in EventMonitor to 'retryStatus' (C4456)
- Replace wchar_t-to-char narrowing in FileMonitorUtilities with
  Utility::WStringToString (C4244)
- Remove dead functions BufferCopy, BufferCopyAndSanitize, ClearBuffer
  from ProcessMonitor.cpp (C4505)
- Replace deprecated std::wstring_convert with Win32 APIs
  (WideCharToMultiByte / MultiByteToWideChar)
- Add missing #include directives and fix cpplint style issues in
  JsonProcessor.cpp/.h and Utility.cpp/.h
- Minor test cleanup and build script fixes (bootstrap call, vcpkg flags)
…nings

Move JsonProcessor.h before standard library headers in JsonProcessor.cpp
and add Utility.h before standard headers in Utility.cpp to satisfy
cpplint's required include ordering: own header, c system, c++ system, other.
…ibility

- Case-insensitive lookup for LogConfig, logFormat, and sources keys to
  match previous parser behavior (configs using logconfig/LOGCONFIG etc. now work)
- Case-insensitive source type matching (eventlog, FILE, etw all accepted)
- Normalize directory path via ParseDirectoryValue before storing (/ to \,
  strip trailing slashes) and validate via IsValidSourceFile
- Validate ETW providers array presence before processing; fix incorrect
  error message referencing 'channels' to reference 'providers'
- Accept providerName OR providerGuid for ETW providers (either is sufficient);
  skip and warn on entries where both are empty
- Downgrade missing logFormat from TraceError to TraceWarning (it is optional)
- Fix cleanupAttributes to delete void* through correct types to avoid UB
- Add findJsonKeyCaseInsensitive helper for object key lookup
@bobsira bobsira merged commit 5658e32 into main Apr 20, 2026
7 checks passed
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.

4 participants