Skip to content

Commit f0c97a9

Browse files
ebenshebensh
committed
Adding design plan to remove Sentry logging related code using this prompt: There is unused code in this repository that was added to be able to send logs to Sentry. We do not use this behavior. Please create a design proposal in
 @.design-proposals/remove-dead-code-sentry.md to remove it. That will likely include an enumeration of locations in the code related to the Sentry logging feature, configuration for Sentry tokens/keys, out of date documentation about the feature, references in Makefiles, and go modules. Don't actually do the modification yet - just plan it out by enumerating the files and such you'd need to touch to make the change. We'll then hand the design to a more junior engineer to actually do the removal work.
1 parent 8ba6439 commit f0c97a9

File tree

1 file changed

+183
-0
lines changed

1 file changed

+183
-0
lines changed

‎.design-proposals/remove-dead-code-sentry.md‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
# Design Proposal: Remove Dead Sentry Code
2+
3+
## Summary
4+
5+
This proposal outlines the removal of unused Sentry logging functionality from the ACS Fleet Manager codebase. The Sentry error monitoring feature was implemented but is not being used in production, making it dead code that should be cleaned up.
6+
7+
## Background
8+
9+
Sentry is an error monitoring service that was integrated into Fleet Manager to capture and report errors in real-time. However, this functionality is currently disabled and not used in production environments. The code, configuration, and documentation related to Sentry should be removed to:
10+
11+
- Reduce codebase complexity
12+
- Remove unused dependencies
13+
- Simplify configuration
14+
- Eliminate dead code paths
15+
16+
## Current Sentry Implementation
17+
18+
### Core Package Files
19+
The following files constitute the main Sentry implementation:
20+
21+
1. **`pkg/services/sentry/sentry.go`** - Main Sentry initialization logic
22+
2. **`pkg/services/sentry/config.go`** - Sentry configuration structure and flags
23+
3. **`pkg/services/sentry/providers.go`** - Dependency injection setup for Sentry
24+
25+
### Integration Points
26+
27+
1. **Logger Integration** (`pkg/logger/logger.go`):
28+
- Lines 10, 99, 108, 215, 223, 230-233, 240, 243-253: Sentry hub usage and event capture
29+
- Sentry integration in error, warning, and fatal logging methods
30+
31+
2. **Core Providers** (`pkg/providers/core.go`):
32+
- Line 23: Import of sentry services
33+
- Line 49: Sentry config providers registration
34+
35+
3. **Environment Configurations**:
36+
- `internal/central/pkg/environments/development.go` - Line 17: Sentry disabled
37+
- `internal/central/pkg/environments/integration.go` - References to Sentry
38+
- `internal/central/pkg/environments/production.go` - Sentry configuration
39+
40+
### Configuration and Deployment Files
41+
42+
1. **Kubernetes Templates**:
43+
- `templates/secrets-template.yml`:
44+
- Lines 53-54: SENTRY_KEY parameter definition
45+
- Line 103: sentry.key secret mapping
46+
- `templates/service-template.yml`:
47+
- Lines 158-178: Sentry-related parameters (ENABLE_SENTRY, SENTRY_URL, etc.)
48+
- Lines 663, 946-951: Sentry configuration flags in deployment
49+
50+
2. **Makefile**:
51+
- References to `secrets/sentry.key` file
52+
- Sentry key parameter handling in deployment targets
53+
54+
### Documentation References
55+
56+
1. **Feature Documentation**:
57+
- `docs/legacy/feature-flags.md` - Lines 18, 93-100: Sentry feature flag documentation
58+
- `docs/development/running-fleet-manager.md` - Lines 11, 19: Environment descriptions mentioning Sentry
59+
- `docs/development/populating-configuration.md` - Lines 118-132: Sentry configuration setup
60+
- `docs/legacy/development/error-handling.md` - Lines 61, 66, 68, 90: Error handling with Sentry
61+
- `CONTRIBUTING.md` - Lines 116-136: Sentry logging examples
62+
63+
### Dependencies
64+
65+
1. **Go Module** (`go.mod`):
66+
- Line 22: `github.com/getsentry/sentry-go v0.34.0` dependency
67+
68+
### Configuration Files
69+
70+
1. **Environment Files**:
71+
- Various environment files contain Sentry-related configurations that need to be cleaned up
72+
73+
## Removal Plan
74+
75+
### Phase 1: Remove Core Sentry Package
76+
1. Delete the entire `pkg/services/sentry/` directory and all its contents:
77+
- `pkg/services/sentry/sentry.go`
78+
- `pkg/services/sentry/config.go`
79+
- `pkg/services/sentry/providers.go`
80+
81+
### Phase 2: Remove Integration Points
82+
1. **Update `pkg/logger/logger.go`**:
83+
- Remove Sentry import (line 10)
84+
- Remove `sentryHub *sentry.Hub` field from logger struct (line 99)
85+
- Remove Sentry hub initialization in `NewUHCLogger` (line 108)
86+
- Remove `captureSentryEvent` calls from `Warningf`, `Errorf`, and `Fatalf` methods
87+
- Remove entire `captureSentryEvent` method (lines 243-253)
88+
- Simplify `Error` method to remove Sentry capture logic (lines 229-234)
89+
90+
2. **Update `pkg/providers/core.go`**:
91+
- Remove Sentry import (line 23)
92+
- Remove Sentry config providers call (line 49)
93+
94+
### Phase 3: Clean Up Configuration and Deployment
95+
1. **Update Kubernetes templates**:
96+
- `templates/secrets-template.yml`:
97+
- Remove SENTRY_KEY parameter (lines 53-54)
98+
- Remove sentry.key from secrets (line 103)
99+
- `templates/service-template.yml`:
100+
- Remove all Sentry-related parameters (lines 158-178)
101+
- Remove Sentry command-line flags from deployment (lines 946-951)
102+
- Remove sentry.key from fake configmap (line 663)
103+
104+
2. **Update Makefile**:
105+
- Remove any Sentry key handling logic
106+
- Remove references to `secrets/sentry.key`
107+
108+
### Phase 4: Remove Environment Configurations
109+
1. **Update environment files**:
110+
- `internal/central/pkg/environments/development.go` - Remove Sentry references
111+
- `internal/central/pkg/environments/integration.go` - Remove Sentry references
112+
- `internal/central/pkg/environments/production.go` - Remove Sentry references
113+
114+
### Phase 5: Update Documentation
115+
1. **Remove or update documentation files**:
116+
- `docs/legacy/feature-flags.md` - Remove Sentry section (lines 18, 93-100)
117+
- `docs/development/running-fleet-manager.md` - Remove Sentry mentions from environment descriptions
118+
- `docs/development/populating-configuration.md` - Remove Sentry configuration section (lines 118-132)
119+
- `docs/legacy/development/error-handling.md` - Remove Sentry references and update error handling examples
120+
- `CONTRIBUTING.md` - Remove Sentry logging examples and references
121+
122+
### Phase 6: Remove Dependencies
123+
1. **Update `go.mod`**:
124+
- Remove `github.com/getsentry/sentry-go v0.34.0` dependency
125+
- Run `go mod tidy` to clean up unused dependencies
126+
127+
### Phase 7: Clean Up Configuration Files
128+
1. **Remove Sentry configuration from environment config files**:
129+
- Check `dev/config/` directory for any Sentry-related configurations
130+
- Remove any `secrets/sentry.key` file references or creation scripts
131+
132+
## Testing Considerations
133+
134+
1. **Unit Tests**: Verify that logger tests still pass after removing Sentry integration
135+
2. **Integration Tests**: Ensure that error handling still works correctly without Sentry
136+
3. **Build Verification**: Confirm that the application builds and starts successfully after removal
137+
4. **Deployment Tests**: Verify that Kubernetes deployments work without Sentry configuration
138+
139+
## Migration Notes
140+
141+
1. **No Data Migration Required**: Since Sentry is not currently used, no data migration is needed
142+
2. **Configuration Updates**: Existing deployments will need configuration updates to remove Sentry-related parameters
143+
3. **Error Monitoring**: If error monitoring is needed in the future, a different solution should be implemented
144+
145+
## Verification Steps
146+
147+
After implementing the removal:
148+
149+
1. **Code Verification**:
150+
- Search codebase for any remaining "sentry" references (case-insensitive)
151+
- Verify `go mod tidy` removes the Sentry dependency
152+
- Ensure all imports are valid and no unused imports remain
153+
154+
2. **Build Verification**:
155+
- Run `make binary` to ensure compilation succeeds
156+
- Run `make test` to verify tests pass
157+
- Run `make lint` to check for any linting issues
158+
159+
3. **Deployment Verification**:
160+
- Verify Kubernetes templates are valid after removing Sentry parameters
161+
- Test local deployment without Sentry configuration
162+
163+
## Benefits
164+
165+
1. **Reduced Complexity**: Removes unused code paths and configurations
166+
2. **Cleaner Dependencies**: Eliminates unnecessary external dependency
167+
3. **Simplified Configuration**: Reduces the number of configuration parameters
168+
4. **Maintainability**: Less code to maintain and fewer potential security vulnerabilities
169+
5. **Documentation Clarity**: Removes outdated documentation about unused features
170+
171+
## Risks
172+
173+
1. **Accidental Usage**: Low risk that some code might still reference Sentry (mitigated by comprehensive search)
174+
2. **Future Needs**: If error monitoring is needed later, it will require re-implementation (acceptable trade-off for current cleanup)
175+
176+
## Estimated Effort
177+
178+
- **Development Time**: ~2-3 hours for a junior engineer
179+
- **Testing Time**: ~1-2 hours
180+
- **Documentation Update**: ~1 hour
181+
- **Total**: ~4-6 hours
182+
183+
This removal should be straightforward since Sentry is not actively used, making it a low-risk cleanup operation.

0 commit comments

Comments
 (0)