Skip to content

fix(integration): use log_entry API for common_system v3.0.0 compatibility#337

Merged
kcenon merged 16 commits into
mainfrom
feature/335-switch-integration-flags-to-kcenon
Dec 23, 2025
Merged

fix(integration): use log_entry API for common_system v3.0.0 compatibility#337
kcenon merged 16 commits into
mainfrom
feature/335-switch-integration-flags-to-kcenon

Conversation

@kcenon

@kcenon kcenon commented Dec 23, 2025

Copy link
Copy Markdown
Owner

Summary

Changes

include/kcenon/network/integration/common_system_adapter.h

  • Changed from deprecated 5-argument log() call to log(log_entry) API
  • Creates log_entry struct with proper field assignment

Test plan

  • Local build with common_system succeeds
  • Local build with pacs_system succeeds

- Include feature_flags.h header in all integration entry points
- Replace BUILD_WITH_THREAD_SYSTEM with KCENON_WITH_THREAD_SYSTEM
- Replace BUILD_WITH_CONTAINER_SYSTEM with KCENON_WITH_CONTAINER_SYSTEM
- Replace BUILD_WITH_MONITORING_SYSTEM with KCENON_WITH_MONITORING_SYSTEM
- Replace BUILD_WITH_COMMON_SYSTEM with KCENON_WITH_COMMON_SYSTEM
- Update comments to reference new macro names

Part of #335: Align integration gating with unified flag contract
- Include feature_flags.h in all public headers
- Replace BUILD_WITH_THREAD_SYSTEM with KCENON_WITH_THREAD_SYSTEM
- Replace BUILD_WITH_CONTAINER_SYSTEM with KCENON_WITH_CONTAINER_SYSTEM
- Replace BUILD_WITH_MONITORING_SYSTEM with KCENON_WITH_MONITORING_SYSTEM
- Replace BUILD_WITH_COMMON_SYSTEM with KCENON_WITH_COMMON_SYSTEM
- Update documentation comments to reference new macro names

Part of #335: Align integration gating with unified flag contract
Update all target_compile_definitions() to use WITH_*_SYSTEM
instead of BUILD_WITH_*. This allows feature_system_deps.h to
properly detect and define KCENON_WITH_* macros.

Changes:
- BUILD_WITH_CONTAINER_SYSTEM -> WITH_CONTAINER_SYSTEM
- BUILD_WITH_THREAD_SYSTEM -> WITH_THREAD_SYSTEM
- BUILD_WITH_LOGGER_SYSTEM -> WITH_LOGGER_SYSTEM
- BUILD_WITH_COMMON_SYSTEM -> WITH_COMMON_SYSTEM
- BUILD_WITH_MONITORING_SYSTEM -> WITH_MONITORING_SYSTEM

BUILD_WITH_* option() declarations are preserved for backward
compatibility with existing build scripts.

Part of #335: Align integration gating with unified flag contract
Update test files and verify_build.cpp to use feature_flags.h header
and KCENON_WITH_* macros instead of BUILD_WITH_* macros for
consistent feature detection across the codebase.

- Add kcenon/common/config/feature_flags.h include
- Replace BUILD_WITH_THREAD_SYSTEM with KCENON_WITH_THREAD_SYSTEM
- Replace BUILD_WITH_CONTAINER_SYSTEM with KCENON_WITH_CONTAINER_SYSTEM
- Replace BUILD_WITH_LOGGER_SYSTEM with KCENON_WITH_LOGGER_SYSTEM
- Replace BUILD_MESSAGING_BRIDGE with KCENON_WITH_MESSAGING_BRIDGE

Part of #335
Update documentation code examples to use KCENON_WITH_* macros
instead of BUILD_WITH_* for feature detection in source code.

CMake command-line options (-DBUILD_WITH_*) remain unchanged as
they are user-facing configuration options.

Updated files:
- README.md, README_KO.md: Integration example
- docs/INTEGRATION.md, docs/INTEGRATION_KO.md: Feature check examples
- docs/FEATURES.md: Dual API support example
- docs/API_REFERENCE_KO.md: Thread system adapter examples
- docs/integration/with-common-system.md: Concepts integration
- docs/integration/with-monitoring.md: Monitoring adapter example
- docs/implementation/02-dependency-and-testing.md: Test examples

Part of #335
Create network_system's own feature_flags.h that works both with
and without common_system dependency:

- When common_system is available (WITH_COMMON_SYSTEM defined),
  include common_system's feature_flags.h
- When common_system is not available, define KCENON_WITH_* macros
  locally based on CMake definitions (WITH_*_SYSTEM)

Update all source and header files to use the local feature_flags.h
at <kcenon/network/config/feature_flags.h> instead of requiring
common_system's header.

This fixes standalone builds (Minimal Build) that were failing due
to missing common_system headers.

Part of #335
When thread_system is built with common_system, it uses
KCENON_HAS_COMMON_EXECUTOR=1 which affects the class layout of
thread_pool (conditional inheritance from IExecutor interface).

network_system must define the same macro when including thread_system
headers to ensure consistent class layout across compilation units.
Without this, Windows MSVC fails with LNK2019 unresolved external
symbol errors for thread_pool::is_running().
Update remaining code examples in documentation to use the new
KCENON_WITH_* macro style instead of the old BUILD_WITH_* or
#ifdef patterns, consistent with the codebase changes.
Fix ODR violation that caused ServerStartupOnUsedPort test to abort.
The integration tests were defining BUILD_WITH_* macros but
feature_flags.h only detects WITH_*_SYSTEM macros. This caused
KCENON_WITH_COMMON_SYSTEM to be 0 in tests but 1 in the library,
resulting in class layout mismatch (messaging_server has conditional
members based on KCENON_WITH_COMMON_SYSTEM).

Changes:
- BUILD_WITH_COMMON_SYSTEM -> WITH_COMMON_SYSTEM
- BUILD_WITH_LOGGER_SYSTEM -> WITH_LOGGER_SYSTEM
- BUILD_WITH_THREAD_SYSTEM -> WITH_THREAD_SYSTEM
- Add missing WITH_CONTAINER_SYSTEM

This aligns integration tests with all other test CMakeLists.txt
which already use WITH_*_SYSTEM macros.
Document the feature flag changes made in PR #336:
- Switch from BUILD_WITH_* to KCENON_WITH_* macros
- Add feature_flags.h for unified feature detection
- Fix ODR violation in integration tests

Closes #335
Fix ODR violation that caused "corrupted size vs. prev_size" error
on Ubuntu Debug and test failures on macOS Release.

The integration_tests were using BUILD_WITH_* CMake options to
determine compile definitions, but these don't always match the
actual system availability detected by NetworkSystemIntegration.cmake.

Changes:
- Use *_SYSTEM_INCLUDE_DIR variables instead of BUILD_WITH_* options
  to detect actual system availability
- Add KCENON_HAS_COMMON_EXECUTOR=1 when both thread_system and
  common_system are available (required for consistent thread_pool
  class layout)

This aligns integration_tests compile definitions with the pattern
used in tests/CMakeLists.txt and tests/integration/CMakeLists.txt.

Part of #335
On macOS, localhost resolves to IPv6 (::1) first, then falls back
to IPv4 (127.0.0.1) when the connection fails. This fallback process
can take 5-10 seconds, causing test timeouts in CI environments.

Changes:
- Replace "localhost" with "127.0.0.1" in ConnectClient()
- Replace "localhost" with "127.0.0.1" in ConnectAllClients()
- Increase CI timeout from 2-3 seconds to 5 seconds for consistency

This fixes ServerShutdownWithActiveConnections test failures on
macOS Release builds.

Part of #335
Add details about the ODR violation fix and macOS IPv6 delay fix
to the KCENON_WITH_* feature flag unification section:
- *_SYSTEM_INCLUDE_DIR pattern for consistent macro definitions
- KCENON_HAS_COMMON_EXECUTOR for thread_pool class layout
- 127.0.0.1 instead of localhost to avoid IPv6 fallback delays

Part of #335
When start_server() fails due to port binding errors (e.g., address
already in use), previously created resources (io_context_, work_guard_)
were not cleaned up. This caused heap corruption during object destruction.

Changes:
- Add resource cleanup in start_server() catch blocks to release
  partially created resources before returning error
- Add explicit resource cleanup in destructor to handle cases where
  stop_server() returns early due to is_running_ being false

This fixes "corrupted size vs. prev_size" errors in
ConnectionLifecycleTest.ServerStartupOnUsedPort on Linux Debug builds.
Document the heap corruption fix in both English and Korean changelogs.
…unction overload

common_system v3.0.0 (Issue #217) removed the deprecated
log(level, message, file, line, function) API from ILogger.
Update common_logger_adapter to use log_entry-based API.
@github-actions

Copy link
Copy Markdown
Contributor

Performance Comparison

Base Branch Results

No base results

PR Branch Results

No PR results

@codecov

codecov Bot commented Dec 23, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.36364% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.74%. Comparing base (3a2b7e1) to head (d069d38).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
src/core/messaging_server.cpp 36.36% 14 Missing ⚠️

❌ Your patch status has failed because the patch coverage (36.36%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (25.74%) is below the target coverage (55.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
- Coverage   35.30%   25.74%   -9.57%     
==========================================
  Files          39       27      -12     
  Lines        4039     2649    -1390     
==========================================
- Hits         1426      682     -744     
+ Misses       2613     1967     -646     
Flag Coverage Δ
unittests 25.74% <36.36%> (-9.57%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...de/kcenon/network/integration/logger_integration.h 0.00% <ø> (ø)
include/kcenon/network/utils/result_types.h 81.81% <ø> (ø)
src/core/network_context.cpp 20.63% <ø> (-6.79%) ⬇️
src/integration/logger_integration.cpp 46.95% <ø> (ø)
src/integration/thread_integration.cpp 26.11% <ø> (-11.60%) ⬇️
src/metrics/network_metrics.cpp 22.22% <ø> (ø)
src/core/messaging_server.cpp 62.50% <36.36%> (-2.72%) ⬇️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kcenon kcenon merged commit ada2bb3 into main Dec 23, 2025
44 of 46 checks passed
@kcenon kcenon deleted the feature/335-switch-integration-flags-to-kcenon branch December 27, 2025 18:15
kcenon added a commit that referenced this pull request Apr 13, 2026
…ility (#337)

Fix API compatibility with common_system v3.0.0 by using log_entry-based API instead of deprecated file/line/function overload
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.

1 participant