refactor!: remove unused deprecated context API methods by kcenon · Pull Request #536 · kcenon/logger_system

kcenon · 2026-03-20T06:12:54Z

What

Removes 8 deprecated methods from the logger API that have been superseded by the unified context() API. Migrates internal callers and tests to the new API.

Why

Fixes #534 -- These methods were deprecated in favor of the unified context() API (introduced in 3.4.0). All 8 removed methods had zero external callers (verified via grep across the ecosystem). Keeping them increases maintenance burden and API surface confusion.

Removed Methods

Method	Replacement
`clear_context()`	`context().clear()`
`has_context()`	`!context().empty()`
`get_context()`	`context().to_fields()`
`set_context_id(key, value)`	`context().set(key, value, context_category::trace)`
`get_context_id(key)`	`context().get_string(key)`
`clear_context_id(key)`	`context().remove(key)`
`has_context_id(key)`	`context().has(key)`
`clear_all_context_ids()`	`context().clear(context_category::trace)`

How

logger.h: Removed 8 method declarations (~107 lines)
logger.cpp: Removed implementations (~63 lines), added removal comment
log_context_scope.cpp: Migrated from set_context()/remove_context() to context().set()/context().remove()
structured_logging_test.cpp: Migrated all test cases to unified context API
structured_logging_example.cpp: Updated examples to use new API

Breaking Changes

This is a breaking change for any code calling the 8 removed methods. Note: set_context() and remove_context() are still preserved as they have active callers.

BREAKING CHANGE: Remove deprecated methods that had no external consumers: - clear_context(), has_context(), get_context() - set_context_id(), get_context_id(), clear_context_id(), has_context_id(), clear_all_context_ids() Migrate internal callers (log_context_scope, tests, examples) to the unified context() API. Preserved deprecated methods still used by file_trans_system: - set_context() (4 overloads), remove_context() - set_otel_context(), get_otel_context(), clear_otel_context(), has_otel_context() Note: result<T> (lowercase, logger-specific wrapper) vs Result<T> (common alias) inconsistency is intentional but worth unifying later. Refs: #534

Add breaking change entry for deprecated context method removal.

github-actions · 2026-03-20T06:37:05Z

Performance Regression Check

Benchmark	Base (ns)	PR (ns)	Delta
BM_AsyncDecorator	—	336.3	—
BM_BufferedAsyncDecorator	—	116.7	—
BM_BufferedDecorator	—	810.9	—
BM_ConsoleAsyncDecorator	—	242.2	—
BM_DirectConsoleWriter	—	6013.0	—
BM_DirectFileWriter	—	699.7	—
BM_ManualNesting_Async	—	295.3	—
BM_ManualNesting_BufferedAsync	—	118.7	—
BM_ObjectPool_HighContention/real_time/threads:4	—	1094.9	—
BM_ObjectPool_HighContention/real_time/threads:8	—	1296.0	—
BM_ObjectPool_MultiThread/real_time/threads:1	—	18.4	—
BM_ObjectPool_MultiThread/real_time/threads:2	—	80.4	—
BM_ObjectPool_MultiThread/real_time/threads:4	—	111.8	—
BM_ObjectPool_MultiThread/real_time/threads:8	—	122.9	—
BM_ObjectPool_SingleThread	—	17.7	—
BM_ThreadLocalObjectPool_CacheEfficiency/16/real_time/threads:4	—	1.5	—
BM_ThreadLocalObjectPool_CacheEfficiency/32/real_time/threads:4	—	1.5	—
BM_ThreadLocalObjectPool_CacheEfficiency/4/real_time/threads:4	—	1.5	—
BM_ThreadLocalObjectPool_CacheEfficiency/64/real_time/threads:4	—	1.5	—
BM_ThreadLocalObjectPool_CacheEfficiency/8/real_time/threads:4	—	1.5	—
BM_ThreadLocalObjectPool_HighContention/real_time/threads:4	—	151.4	—
BM_ThreadLocalObjectPool_HighContention/real_time/threads:8	—	142.9	—
BM_ThreadLocalObjectPool_MultiThread/real_time/threads:1	—	4.1	—
BM_ThreadLocalObjectPool_MultiThread/real_time/threads:2	—	6.9	—
BM_ThreadLocalObjectPool_MultiThread/real_time/threads:4	—	15.9	—
BM_ThreadLocalObjectPool_MultiThread/real_time/threads:8	—	15.2	—
BM_ThreadLocalObjectPool_SingleThread	—	3.4	—
BM_Throughput_LargeMessages	—	143.7	—
BM_Throughput_SmallMessages	—	147.4	—

Threshold: >5% regression triggers warning
Result: ✅ No performance regression detected

kcenon · 2026-03-20T08:47:24Z

CI/CD Failure Analysis

Analysis Time: 2026-03-20 17:10 KST
Attempt: #1

Failed Workflows

Workflow	Job	Error	Status
Code Coverage	Coverage Analysis	`has no member named 'has_context'`	Failed
Integration Tests	Coverage, Performance, FetchContent, Windows/macOS/Ubuntu	Same build error	Failed/Cancelled

Root Cause Analysis

Primary Error:

tests/structured_logging_test.cpp:219: error: 'class kcenon::logger::logger' has no member named 'has_context'; did you mean 'set_context'?
tests/structured_logging_test.cpp:421: error: same

Analysis:
The PR removed 8 deprecated context API methods including has_context(), but the test file structured_logging_test.cpp still calls has_context() at lines 219 and 421. These calls were not migrated to the new context() API.

All 7 CI failures stem from this single compilation error — Integration Tests, Coverage, Performance, and FetchContent all fail because the test binary cannot be built.

Identified Issues:

structured_logging_test.cpp:219 — has_context call not migrated
structured_logging_test.cpp:421 — has_context call not migrated

Proposed Fix

Issue	Proposed Solution	Files Affected
`has_context()` not migrated	Replace with `!context().empty()` or `context().has(key)`	`tests/structured_logging_test.cpp`

Next Steps

Read test file to understand exact context of has_context() calls
Apply appropriate migration
Push and monitor CI

Automated failure analysis - Attempt #1

Replace has_context(), get_context(), clear_context(), and remove_context() calls in tests and examples with the new unified context() API methods. Fixes CI build failure caused by removed deprecated methods.

kcenon · 2026-03-20T09:58:33Z

CI/CD Failure Analysis

Analysis Time: 2026-03-20 09:15 UTC
Attempt: #1

Failed Workflows

Workflow	Job	Step	Status
Clang-Tidy Analysis	Clang-Tidy Analysis	CMake Configure	Failed
Coverage Analysis	Coverage Analysis	Configure CMake with coverage	Failed
Integration Test Coverage	Integration Test Coverage	Configure CMake with Coverage	Failed

Root Cause Analysis

Primary Error:

fatal: invalid reference: 93b1d0f
CMake Error: Failed to checkout tag: '93b1d0f'

Analysis:
All three failures share the same root cause. cmake/UnifiedDependencies.cmake specifies a short commit hash (93b1d0f) for the common_system dependency. FetchContent uses shallow clone (--depth=1) by default, and Git servers cannot resolve abbreviated/short SHA references during shallow clone operations. The commit exists in the common_system repository (full hash: 93b1d0f64c6fb69529f0ff84bed2601de07ebba5), but the short form fails to resolve.

Why other CI jobs pass: Workflows like Integration Tests and Core-Only Build use an explicit actions/checkout step to clone common_system before CMake runs, so find_package finds the local copy and FetchContent fallback is never triggered.

Identified Issues:

Short commit hash 93b1d0f in cmake/UnifiedDependencies.cmake:113 incompatible with FetchContent shallow clone

Proposed Fix

Issue	Proposed Solution	Files Affected
Short hash incompatible with shallow clone	Replace with full 40-char hash `93b1d0f64c6fb69529f0ff84bed2601de07ebba5`	`cmake/UnifiedDependencies.cmake`

Next Steps

Apply proposed fix
Push and monitor CI

Automated failure analysis - Attempt #1

Short commit hash (93b1d0f) fails to resolve during FetchContent shallow clone operations. Replace with the full 40-character hash to ensure reliable dependency resolution across all CI workflows.

FetchContent GIT_SHALLOW only works with tags and branch names. When GIT_TAG is a commit hash (hex-only string), shallow clone fails because git --depth=1 only fetches the branch tip. Detect hash-style tags and disable shallow clone for them.

CMake macro parameters are text replacements, not variables. In if(GIT_TAG MATCHES ...), CMake treats 'GIT_TAG' as a literal string, not the macro argument value. Use "${GIT_TAG}" to force parameter expansion before the if() evaluation.

kcenon · 2026-03-20T10:07:46Z

CI/CD Failure Analysis - Attempt #3

Previous Attempt Result: Failed (same error persisted)
Previous Fixes Applied:

Short hash → full 40-char hash
Added conditional GIT_SHALLOW FALSE for commit hashes

What Changed

Previous Fix:

Added _git_shallow logic to detect commit hashes and disable shallow clone
Used if(GIT_TAG MATCHES ...) which doesn't work in CMake macros

Why It Still Failed:

CMake macro parameters are text replacements, not variables
if(GIT_TAG MATCHES "^[0-9a-f]+$") treats GIT_TAG as the literal string "GIT_TAG"
The regex never matched, so GIT_SHALLOW stayed TRUE

New Root Cause

Error:

fatal: unable to read tree (93b1d0f64c6fb69529f0ff84bed2601de07ebba5)

Analysis:
CMake macro parameter expansion requires "${GIT_TAG}" (with ${} and quotes) to properly expand the argument before if() evaluation. Without the expansion, the shallow clone detection was never triggered.

Fix Applied

Issue	Solution	File
Macro parameter not expanded	`if("${GIT_TAG}" MATCHES ...)` instead of `if(GIT_TAG MATCHES ...)`	`cmake/UnifiedDependencies.cmake`

Automated failure analysis - Attempt #3 of 3

* refactor\!: remove unused deprecated context API methods BREAKING CHANGE: Remove deprecated methods that had no external consumers: - clear_context(), has_context(), get_context() - set_context_id(), get_context_id(), clear_context_id(), has_context_id(), clear_all_context_ids() Migrate internal callers (log_context_scope, tests, examples) to the unified context() API. Preserved deprecated methods still used by file_trans_system: - set_context() (4 overloads), remove_context() - set_otel_context(), get_otel_context(), clear_otel_context(), has_otel_context() Note: result<T> (lowercase, logger-specific wrapper) vs Result<T> (common alias) inconsistency is intentional but worth unifying later. Refs: #534 * docs: add Phase 2 changelog entry for #534 Add breaking change entry for deprecated context method removal. * fix(test): migrate remaining has_context() calls to unified context API Replace has_context(), get_context(), clear_context(), and remove_context() calls in tests and examples with the new unified context() API methods. Fixes CI build failure caused by removed deprecated methods. * fix(cmake): use full commit hash for common_system FetchContent Short commit hash (93b1d0f) fails to resolve during FetchContent shallow clone operations. Replace with the full 40-character hash to ensure reliable dependency resolution across all CI workflows. * fix(cmake): disable shallow clone for commit hash references FetchContent GIT_SHALLOW only works with tags and branch names. When GIT_TAG is a commit hash (hex-only string), shallow clone fails because git --depth=1 only fetches the branch tip. Detect hash-style tags and disable shallow clone for them. * fix(cmake): use quoted expansion for macro parameter in if() CMake macro parameters are text replacements, not variables. In if(GIT_TAG MATCHES ...), CMake treats 'GIT_TAG' as a literal string, not the macro argument value. Use "${GIT_TAG}" to force parameter expansion before the if() evaluation.

kcenon added 2 commits March 20, 2026 15:04

docs: add Phase 2 changelog entry for #534

9ab778f

Add breaking change entry for deprecated context method removal.

kcenon added refactor Code refactoring without changing functionality priority:medium Medium priority issue core Core functionality labels Mar 20, 2026

fix(test): migrate remaining has_context() calls to unified context API

a44e5da

Replace has_context(), get_context(), clear_context(), and remove_context() calls in tests and examples with the new unified context() API methods. Fixes CI build failure caused by removed deprecated methods.

kcenon added 3 commits March 20, 2026 18:58

fix(cmake): use full commit hash for common_system FetchContent

d25807c

Short commit hash (93b1d0f) fails to resolve during FetchContent shallow clone operations. Replace with the full 40-character hash to ensure reliable dependency resolution across all CI workflows.

kcenon merged commit f78e6da into main Mar 20, 2026
33 checks passed

kcenon deleted the refactor/534-remove-deprecated-api branch March 20, 2026 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: remove unused deprecated context API methods#536

refactor!: remove unused deprecated context API methods#536
kcenon merged 6 commits into
mainfrom
refactor/534-remove-deprecated-api

kcenon commented Mar 20, 2026

Uh oh!

github-actions Bot commented Mar 20, 2026 •

edited

Loading

Uh oh!

kcenon commented Mar 20, 2026

Uh oh!

kcenon commented Mar 20, 2026

Uh oh!

kcenon commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kcenon commented Mar 20, 2026

What

Why

Removed Methods

How

Breaking Changes

Uh oh!

github-actions Bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance Regression Check

Uh oh!

kcenon commented Mar 20, 2026

CI/CD Failure Analysis

Failed Workflows

Root Cause Analysis

Proposed Fix

Next Steps

Uh oh!

kcenon commented Mar 20, 2026

CI/CD Failure Analysis

Failed Workflows

Root Cause Analysis

Proposed Fix

Next Steps

Uh oh!

kcenon commented Mar 20, 2026

CI/CD Failure Analysis - Attempt #3

What Changed

New Root Cause

Fix Applied

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Mar 20, 2026 •

edited

Loading