Skip to content

feat: Add structured logging API with level filtering and component tags#170

Merged
shaia merged 8 commits into
masterfrom
feat/structured-logging
Mar 6, 2026
Merged

feat: Add structured logging API with level filtering and component tags#170
shaia merged 8 commits into
masterfrom
feat/structured-logging

Conversation

@shaia
Copy link
Copy Markdown
Owner

@shaia shaia commented Mar 5, 2026

Replace all raw fprintf/printf diagnostic calls (21 sites across 14 source files) with cfd_log() structured logging. Adds DEBUG log level, global atomic log level filtering, extended callback with component info, and convenience macros (CFD_LOG_DEBUG/INFO/WARNING/ERROR).

Backward-compatible: existing cfd_error/cfd_warning/cfd_info unchanged.

ROADMAP 0.6 (Structured Logging & Diagnostics) complete.

Replace all raw fprintf/printf diagnostic calls (21 sites across 14
source files) with cfd_log() structured logging. Adds DEBUG log level,
global atomic log level filtering, extended callback with component
info, and convenience macros (CFD_LOG_DEBUG/INFO/WARNING/ERROR).

Backward-compatible: existing cfd_error/cfd_warning/cfd_info unchanged.

ROADMAP 0.6 (Structured Logging & Diagnostics) complete.
@shaia shaia requested a review from Copilot March 5, 2026 19:36
The volatile pointer for s_global_callback_ex caused a TSan
SEGV/DEADLYSIGNAL on Linux/Clang. Replace with cfd_atomic_ptr
using _Atomic(void*) on POSIX and InterlockedExchangePointer
on Windows to satisfy ThreadSanitizer's memory access checks.
Copy link
Copy Markdown
Contributor

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

_Atomic(void*) caused ThreadSanitizer SEGV (DEADLYSIGNAL) on
Linux/Clang. Switch to _Atomic(uintptr_t) which is well-supported
by TSan. Also fix double atomic load (TOCTOU) by loading callback
pointer once into a local variable before use.
@shaia shaia requested a review from Copilot March 5, 2026 20:15
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.


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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread tests/core/test_logging.c
Comment thread tests/core/test_logging.c
Comment thread lib/src/core/cfd_threading_internal.h
Comment thread lib/src/core/cfd_threading_internal.h
Comment thread lib/src/core/logging.c Outdated
Comment thread lib/include/cfd/core/logging.h Outdated
shaia added 4 commits March 6, 2026 07:22
…ogging

- Add <stdio.h> for snprintf declaration (needed under strict C99+ flags)
- Clear s_ex_last_message when message is NULL to prevent stale state between tests
Plain volatile read doesn't guarantee atomicity on all Windows targets.
Use ICXP(ptr, NULL, NULL) which returns the current value atomically.
Resilient to enum renumbering and self-documenting.
GCC/Clang will now warn on mismatched format strings at all CFD_LOG_* call sites.
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.


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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread lib/src/core/logging.c Outdated
Comment thread lib/src/core/logging.c
…casts

Out-of-range values (e.g., from int casts) could suppress ERROR logging
or index out of bounds in the level name table.
@shaia shaia merged commit 015e067 into master Mar 6, 2026
18 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.

2 participants