Summary
This issue proposes consolidation of convenience methods in the logger class based on Kent Beck's "Fewest Elements" and "No Duplication" principles.
Background
Kent Beck's Simple Design principles:
- No duplication (DRY) - Each piece of knowledge should have a single representation
- Fewest elements - Remove unnecessary abstractions
Current Assessment
| Principle |
Rating |
Notes |
| Reveals intention |
⭐⭐⭐⭐⭐ |
structured_log_builder, log_sampler, realtime_log_analyzer are clear |
| DRY |
⭐⭐⭐ |
Multiple convenience methods that could be consolidated |
| Fewest elements |
⭐⭐⭐ |
Excessive convenience methods |
Identified Issues
1. Excessive Structured Logging Convenience Methods (High Priority)
Current state in logger.h:
class logger {
// Generic method exists
[[nodiscard]] structured_log_builder log_structured(log_level level);
// But also 6 level-specific convenience methods
[[nodiscard]] structured_log_builder trace_structured();
[[nodiscard]] structured_log_builder debug_structured();
[[nodiscard]] structured_log_builder info_structured();
[[nodiscard]] structured_log_builder warn_structured();
[[nodiscard]] structured_log_builder error_structured();
[[nodiscard]] structured_log_builder fatal_structured();
};Problem: These 6 methods are trivial wrappers around log_structured(level).
Recommendation: Keep only the generic method:
class logger {
// Single method handles all cases
[[nodiscard]] structured_log_builder structured(log_level level);
};
// Usage remains ergonomic
logger->structured(log_level::info)
.message("User login")
.field("user_id", 123)
.emit();If level-specific methods are desired for discoverability, consider making them inline free functions or keeping them but documenting that structured(level) is the canonical API.
2. Context ID Method Proliferation (High Priority)
Current state:
class logger {
// Correlation ID
void set_correlation_id(const std::string& correlation_id);
std::string get_correlation_id() const;
void clear_correlation_id();
bool has_correlation_id() const;
// Request ID (similar to correlation ID)
void set_request_id(const std::string& request_id);
std::string get_request_id() const;
void clear_request_id();
bool has_request_id() const;
// Trace ID
void set_trace_id(const std::string& trace_id);
std::string get_trace_id() const;
void clear_trace_id();
bool has_trace_id() const;
// Span ID
void set_span_id(const std::string& span_id);
std::string get_span_id() const;
void clear_span_id();
bool has_span_id() const;
// Parent Span ID
void set_parent_span_id(const std::string& parent_span_id);
std::string get_parent_span_id() const;
void clear_parent_span_id();
bool has_parent_span_id() const;
};Problem: 20 methods for essentially the same pattern (5 IDs × 4 operations each).
Recommendation Option A - Generic context method:
class logger {
// Single generic method for all context IDs
void set_context_id(std::string_view key, std::string_view value);
std::string get_context_id(std::string_view key) const;
void clear_context_id(std::string_view key);
bool has_context_id(std::string_view key) const;
// Clear all context
void clear_all_context_ids();
};
// Usage
logger->set_context_id("correlation_id", "abc-123");
logger->set_context_id("trace_id", "trace-456");Recommendation Option B - Trace context structure:
struct trace_context {
std::optional<std::string> correlation_id;
std::optional<std::string> request_id;
std::optional<std::string> trace_id;
std::optional<std::string> span_id;
std::optional<std::string> parent_span_id;
};
class logger {
void set_trace_context(const trace_context& ctx);
trace_context get_trace_context() const;
void clear_trace_context();
};3. Dual API for Log Levels (Medium Priority)
Current state:
class logger : public common::interfaces::ILogger {
// ILogger interface (common::interfaces::log_level)
common::VoidResult log(common::interfaces::log_level level, const std::string& message) override;
bool is_enabled(common::interfaces::log_level level) const override;
// Native API (logger_system::log_level)
void log(log_level level, const std::string& message);
bool is_enabled(log_level level) const;
};Recommendation:
- Complete migration to
common::interfaces::log_level
- Mark native API as
[[deprecated]] with clear migration timeline
- Remove in next major version
4. Type Alias Location Clarity (Low Priority)
Current state:
// In logger.h
using log_level = logger_system::log_level;
using overflow_policy = logger_system::overflow_policy;
Problem: Definitions are in logger_types.h but using declarations in logger.h may cause confusion about authoritative location.
Recommendation: Document the canonical location clearly or consolidate definitions.
Acceptance Criteria
Migration Guide Template
### Structured Logging
// Before
logger->info_structured().message("Hello").emit();
// After
logger->structured(log_level::info).message("Hello").emit();
### Context IDs
// Before
logger->set_correlation_id("abc-123");
logger->set_trace_id("trace-456");
// After (Option A)
logger->set_context_id("correlation_id", "abc-123");
logger->set_context_id("trace_id", "trace-456");
// After (Option B)
logger->set_trace_context({
.correlation_id = "abc-123",
.trace_id = "trace-456"
});Labels
Summary
This issue proposes consolidation of convenience methods in the
loggerclass based on Kent Beck's "Fewest Elements" and "No Duplication" principles.Background
Kent Beck's Simple Design principles:
Current Assessment
structured_log_builder,log_sampler,realtime_log_analyzerare clearIdentified Issues
1. Excessive Structured Logging Convenience Methods (High Priority)
Current state in
logger.h:Problem: These 6 methods are trivial wrappers around
log_structured(level).Recommendation: Keep only the generic method:
If level-specific methods are desired for discoverability, consider making them
inlinefree functions or keeping them but documenting thatstructured(level)is the canonical API.2. Context ID Method Proliferation (High Priority)
Current state:
Problem: 20 methods for essentially the same pattern (5 IDs × 4 operations each).
Recommendation Option A - Generic context method:
Recommendation Option B - Trace context structure:
3. Dual API for Log Levels (Medium Priority)
Current state:
Recommendation:
common::interfaces::log_level[[deprecated]]with clear migration timeline4. Type Alias Location Clarity (Low Priority)
Current state:
Problem: Definitions are in
logger_types.hbutusingdeclarations inlogger.hmay cause confusion about authoritative location.Recommendation: Document the canonical location clearly or consolidate definitions.
Acceptance Criteria
Migration Guide Template
Labels
refactorapi-designDRY