Skip to content

Commit 516dd89

Browse files
committed
feat: enhance logging and documentation
- Add HelmLogger class diagram to low-level design - Add logging system documentation - Create ADR index page for better navigation - Fix PathData and Value test cases - Update tasks.md to mark logging implementation complete
1 parent 7879f61 commit 516dd89

File tree

14 files changed

+793
-49
lines changed

14 files changed

+793
-49
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,6 @@ bin/
184184
.DS_Store
185185
.AppleDouble
186186
.LSOverride
187+
188+
# windsurf rules
189+
.windsurfrules
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# ADR-006: Helm-Style Logging System
2+
3+
## Status
4+
Accepted
5+
6+
## Context
7+
As a Helm plugin, our application needs to follow Helm's logging conventions for consistency and user experience. Key considerations include:
8+
9+
1. Debug output control via environment variables
10+
2. Error message formatting
11+
3. Performance in logging operations
12+
4. Integration with Helm's output conventions
13+
5. Testing and verification of log output
14+
15+
## Decision
16+
17+
### 1. Implement HelmLogger Class
18+
Create a dedicated logger class that follows Helm plugin conventions:
19+
20+
```python
21+
class HelmLogger:
22+
@staticmethod
23+
def debug(msg: str, *args: Any) -> None:
24+
if os.environ.get('HELM_DEBUG'):
25+
if args:
26+
msg = msg % args
27+
print("[debug] %s" % msg, file=sys.stderr)
28+
29+
@staticmethod
30+
def error(msg: str, *args: Any) -> None:
31+
if args:
32+
msg = msg % args
33+
print("Error: %s" % msg, file=sys.stderr)
34+
```
35+
36+
### 2. Key Design Decisions
37+
38+
1. **Helm Convention Compliance**
39+
- Use stderr for all output
40+
- Prefix debug messages with "[debug]"
41+
- Prefix error messages with "Error:"
42+
- Control debug output via HELM_DEBUG environment variable
43+
44+
2. **Performance Optimization**
45+
- Use string formatting instead of f-strings for consistent behavior
46+
- Lazy evaluation of debug messages
47+
- No string concatenation in critical paths
48+
49+
3. **Global Instance Pattern**
50+
- Provide a global logger instance
51+
- Enable consistent logging across the codebase
52+
- Avoid passing logger instances
53+
54+
4. **Testing Support**
55+
- Mockable stderr for testing
56+
- Environment variable control in tests
57+
- String format verification
58+
59+
### 3. Usage Pattern
60+
```python
61+
from helm_values_manager.utils.logger import logger
62+
63+
def some_function():
64+
logger.debug("Processing %s", value)
65+
try:
66+
# do something
67+
logger.debug("Success!")
68+
except Exception as e:
69+
logger.error("Failed: %s", str(e))
70+
```
71+
72+
## Consequences
73+
74+
### Positive
75+
1. **Consistent User Experience**
76+
- Matches Helm's output format
77+
- Familiar debug control mechanism
78+
- Clear error messages
79+
80+
2. **Performance**
81+
- Efficient string formatting
82+
- Debug messages skipped when disabled
83+
- Minimal memory overhead
84+
85+
3. **Maintainability**
86+
- Centralized logging logic
87+
- Easy to modify output format
88+
- Simple testing approach
89+
90+
4. **Debugging**
91+
- Clear debug output control
92+
- Consistent message format
93+
- Environment-based control
94+
95+
### Negative
96+
1. **Limited Flexibility**
97+
- Fixed output format
98+
- No log levels beyond debug/error
99+
- No log file support
100+
101+
2. **Global State**
102+
- Global logger instance
103+
- Potential for test interference
104+
- Need careful test isolation
105+
106+
## Implementation Notes
107+
108+
1. **Testing**
109+
```python
110+
def test_debug_output():
111+
stderr = StringIO()
112+
with mock.patch.dict(os.environ, {'HELM_DEBUG': '1'}), \
113+
mock.patch('helm_values_manager.utils.logger.sys.stderr', stderr):
114+
logger.debug("Test message")
115+
assert stderr.getvalue() == "[debug] Test message\n"
116+
```
117+
118+
2. **Integration**
119+
```python
120+
# In PathData class
121+
def validate(self):
122+
logger.debug("Validating PathData for path: %s", self.path)
123+
if not self.is_valid():
124+
logger.error("Invalid PathData: %s", self.path)
125+
```

docs/ADRs/README.md

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Architecture Decision Records (ADRs)
2+
3+
This directory contains Architecture Decision Records (ADRs) for the Helm Values Manager project.
4+
5+
## What is an ADR?
6+
An Architecture Decision Record (ADR) is a document that captures an important architectural decision made along with its context and consequences.
7+
8+
## ADR Index
9+
10+
### [ADR-001: Helm Values Manager as a Helm Plugin](001-helm-values-manager.md)
11+
- **Status**: Accepted
12+
- **Context**: Need for managing configurations and secrets across multiple Kubernetes deployments
13+
- **Decision**: Implement as a Helm plugin in Python with key-value storage model
14+
- **Impact**: Defines core architecture and integration approach
15+
16+
### [ADR-002: Config Path and Storage Design](002-config-path-and-storage-design.md)
17+
- **Status**: Accepted
18+
- **Context**: Need for separate config definition from value storage
19+
- **Decision**: Implement path map and standardized command pattern
20+
- **Impact**: Establishes core data structures and file operations
21+
- **Dependencies**: ADR-001
22+
23+
### [ADR-003: Unified Path and Value Storage](003-unified-path-value-storage.md)
24+
- **Status**: Accepted
25+
- **Context**: Complexity from separate path and value storage
26+
- **Decision**: Unified dictionary structure for paths and values
27+
- **Impact**: Simplifies storage and improves consistency
28+
- **Dependencies**: ADR-002
29+
30+
### [ADR-004: Value Resolution Strategy](004-value-resolution-strategy.md)
31+
- **Status**: Accepted
32+
- **Context**: Need for clear value resolution in unified storage
33+
- **Decision**: Introduce Value class for encapsulation
34+
- **Impact**: Clarifies value handling and storage strategy
35+
- **Dependencies**: ADR-003
36+
37+
### [ADR-005: Unified Backend Approach](005-unified-backend-approach.md)
38+
- **Status**: Proposed
39+
- **Context**: Split logic in Value class for different storage types
40+
- **Decision**: Remove storage type distinction, use SimpleValueBackend
41+
- **Impact**: Simplifies Value class and unifies storage interface
42+
- **Dependencies**: ADR-004
43+
44+
### [ADR-006: Helm-Style Logging System](006-helm-style-logging.md)
45+
- **Status**: Accepted
46+
- **Context**: Need for consistent logging following Helm conventions
47+
- **Decision**: Implement HelmLogger with Helm-style output
48+
- **Impact**: Ensures consistent user experience and debugging
49+
- **Dependencies**: ADR-001
50+
51+
## ADR Template
52+
For new ADRs, use this template:
53+
```markdown
54+
# ADR-NNN: Title
55+
56+
## Status
57+
[Proposed | Accepted | Deprecated | Superseded]
58+
59+
## Context
60+
What is the issue that we're seeing that is motivating this decision or change?
61+
62+
## Decision
63+
What is the change that we're proposing and/or doing?
64+
65+
## Consequences
66+
What becomes easier or more difficult to do because of this change?
67+
```

docs/Design/low-level-design.md

Lines changed: 59 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@ classDiagram
2121
}
2222
2323
class PathData {
24+
+str path
2425
+Dict metadata
2526
+Dict~str,Value~ values
27+
+validate() None
28+
+add_value(environment: str, value: Value) None
29+
+get_value(environment: str) Optional[Value]
30+
+to_dict() Dict
31+
+from_dict(data: Dict, create_value_fn) PathData
2632
}
2733
2834
class Value {
@@ -85,6 +91,11 @@ classDiagram
8591
+validate_auth_config(auth_config: dict) None
8692
}
8793
94+
class HelmLogger {
95+
+debug(msg: str, *args: Any) None
96+
+error(msg: str, *args: Any) None
97+
}
98+
8899
HelmValuesConfig "1" *-- "*" ConfigValue
89100
HelmValuesConfig "1" *-- "*" Deployment
90101
HelmValuesConfig "1" *-- "*" PathData
@@ -99,32 +110,32 @@ classDiagram
99110

100111
### 2. Value Management
101112

102-
The system uses a unified approach to value storage and resolution through the `Value` class:
113+
The system manages configuration values through a hierarchy of classes:
103114

104-
```python
105-
class Value:
106-
def __init__(self, path: str, environment: str,
107-
backend: ValueBackend):
108-
self.path = path
109-
self.environment = environment
110-
self._backend = backend
111-
112-
def get(self) -> str:
113-
"""Get the resolved value"""
114-
return self._backend.get_value(self.path, self.environment)
115-
116-
def set(self, value: str) -> None:
117-
"""Set the value"""
118-
if not isinstance(value, str):
119-
raise ValueError("Value must be a string")
120-
self._backend.set_value(self.path, self.environment, value)
121-
```
115+
1. **HelmValuesConfig**
116+
- Top-level configuration manager
117+
- Maintains mapping of paths to PathData instances
118+
- Handles backend selection based on value sensitivity
119+
- Manages deployments and their configurations
120+
121+
2. **PathData**
122+
- Represents a single configuration path and its properties
123+
- Owns the configuration path and ensures consistency
124+
- Stores metadata (description, required status, sensitivity)
125+
- Manages environment-specific values through Value instances
126+
- Validates path consistency between itself and its Values
127+
- Delegates actual value storage to Value instances
122128

123-
Key features:
124-
- Encapsulated value resolution logic
125-
- Unified interface for all storage backends
129+
3. **Value**
130+
- Handles actual value storage and retrieval
131+
- Uses appropriate backend for storage operations
132+
- Maintains reference to its path and environment
133+
134+
This hierarchy ensures:
126135
- Clear separation of concerns
127-
- Type-safe value handling
136+
- Consistent path handling across the system
137+
- Proper validation at each level
138+
- Flexible backend selection based on value sensitivity
128139

129140
### 3. Command Pattern
130141

@@ -249,6 +260,31 @@ The configuration follows the v1 schema:
249260
- Backup before writes
250261
- Atomic updates
251262

263+
## Logging System
264+
265+
The logging system follows Helm plugin conventions and provides consistent output formatting:
266+
267+
1. **HelmLogger Class**
268+
- Provides debug and error logging methods
269+
- Follows Helm output conventions
270+
- Uses stderr for all output
271+
- Controls debug output via HELM_DEBUG environment variable
272+
273+
2. **Global Logger Instance**
274+
- Available via `from helm_values_manager.utils.logger import logger`
275+
- Ensures consistent logging across all components
276+
- Simplifies testing with mock support
277+
278+
3. **Performance Features**
279+
- Uses string formatting for efficiency
280+
- Lazy evaluation of debug messages
281+
- Minimal memory overhead
282+
283+
4. **Testing Support**
284+
- Mockable stderr output
285+
- Environment variable control
286+
- String format verification
287+
252288
## Benefits of This Design
253289

254290
1. **Separation of Concerns**

docs/Development/tasks.md

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@
2222
- [x] Add support for sensitive field handling
2323

2424
### PathData Class Implementation
25-
- [ ] Create PathData class
26-
- [ ] Add metadata dictionary support
27-
- [ ] Add values dictionary for Value objects
28-
- [ ] Implement validation methods
29-
- [ ] Add serialization support
30-
- [ ] Implement to_dict() method
31-
- [ ] Implement from_dict() method
32-
- [ ] Add tests for serialization
25+
- [x] Create PathData class
26+
- [x] Add metadata dictionary support
27+
- [x] Add values dictionary for Value objects
28+
- [x] Implement validation methods
29+
- [x] Add serialization support
30+
- [x] Implement to_dict() method
31+
- [x] Implement from_dict() method
32+
- [x] Add tests for serialization
3333

3434
### HelmValuesConfig Refactoring
3535
- [ ] Remove PlainTextBackend references
@@ -75,13 +75,13 @@
7575
- [ ] Add command-specific validation
7676

7777
### Testing Infrastructure
78-
- [ ] Set up test infrastructure
79-
- [ ] Add pytest configuration
80-
- [ ] Set up test fixtures
81-
- [ ] Add mock backends for testing
82-
- [ ] Add unit tests
83-
- [ ] Value class tests
84-
- [ ] PathData class tests
78+
- [x] Set up test infrastructure
79+
- [x] Add pytest configuration
80+
- [x] Set up test fixtures
81+
- [x] Add mock backends for testing
82+
- [x] Add unit tests
83+
- [x] Value class tests
84+
- [x] PathData class tests
8585
- [ ] HelmValuesConfig tests
8686
- [ ] Backend tests
8787
- [ ] Command tests
@@ -90,6 +90,22 @@
9090
- [ ] Backend integration tests
9191
- [ ] File operation tests
9292

93+
### Logging System
94+
- [x] Implement Helm-style logger
95+
- [x] Create HelmLogger class
96+
- [x] Add debug and error methods
97+
- [x] Follow Helm output conventions
98+
- [x] Add HELM_DEBUG support
99+
- [x] Add comprehensive tests
100+
- [x] Test debug output control
101+
- [x] Test error formatting
102+
- [x] Test string formatting
103+
- [x] Test environment handling
104+
- [x] Update components to use logger
105+
- [x] Update PathData class
106+
- [x] Update Value class
107+
- [x] Add logging documentation
108+
93109
### Documentation
94110
- [ ] Update API documentation
95111
- [ ] Document Value class

0 commit comments

Comments
 (0)