Skip to content

Commit c23e3be

Browse files
authored
Merge pull request #153 from syncable-dev/develop
Develop
2 parents fb453c1 + f6b61f1 commit c23e3be

16 files changed

Lines changed: 1608 additions & 335 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ target/
55

66
# These are backup files generated by rustfmt
77
**/*.rs.bk
8+
.qoder
89

910
# MSVC Windows builds of rustc generate these, which store debugging information
1011
*.pdb

.qoder/quests/analyze-folder.md

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
# Analyze Folder Command Telemetry Design
2+
3+
## Overview
4+
5+
This document outlines the design for improving the telemetry events for the `analyze` command in the syncable-cli application. Currently, the command generates two separate telemetry events when executed, which is incorrect. The goal is to generate only one event per command execution while still capturing the different modes of operation (detailed view, JSON output, etc.).
6+
7+
## Current Issues
8+
9+
1. **Duplicate Events**: When running `sync-ctl analyze .`, two events are generated:
10+
- A generic "analyze" event
11+
- A specific "Analyze Folder" event
12+
13+
2. **Lack of Differentiation**: The current implementation doesn't capture how the analysis was performed (JSON output, detailed view, matrix view, etc.)
14+
15+
## Proposed Solution
16+
17+
Replace the two separate events with a single "Analyze Folder" event that includes properties to differentiate the analysis mode.
18+
19+
## Architecture
20+
21+
### Event Structure
22+
23+
The new telemetry event will have the following structure:
24+
25+
Event Name: "Analyze Folder"
26+
Properties:
27+
- analysis_mode: string (one of: "json", "detailed", "matrix", "summary")
28+
- color_scheme: string (one of: "auto", "dark", "light")
29+
- only_filter: string[] (list of filtered analysis aspects)
30+
31+
### Implementation Plan
32+
33+
1. **Remove duplicate event calls**: Eliminate the separate `track_analyze()` call
34+
2. **Enhance the `track_analyze_folder()` method**: Add parameters to capture analysis mode
35+
3. **Modify the main function**: Pass analysis parameters to the telemetry event
36+
4. **Update the telemetry client**: Modify the `track_analyze_folder()` method to accept and process these parameters
37+
38+
## Detailed Design
39+
40+
### 1. Telemetry Client Modifications
41+
42+
The `TelemetryClient` struct will be updated to accept properties in the `track_analyze_folder` method:
43+
44+
Method signature:
45+
- Current: `track_analyze_folder(&self)`
46+
- New: `track_analyze_folder(&self, properties: HashMap<String, serde_json::Value>)`
47+
48+
Implementation:
49+
- The method will pass the properties to the track_event function
50+
- Properties will be merged with common properties before sending
51+
52+
### 2. Main Function Updates
53+
54+
In the main function, the analyze command handling will be modified:
55+
56+
Process for determining analysis mode:
57+
- If json flag is true → "json"
58+
- Else if detailed flag is true → "detailed"
59+
- Else based on display option:
60+
- Matrix or None → "matrix"
61+
- Detailed → "detailed"
62+
- Summary → "summary"
63+
64+
Properties to capture:
65+
- Analysis mode (determined by command flags)
66+
- Color scheme (if specified)
67+
- Only filter (if specified)
68+
69+
### 3. Remove Duplicate Event
70+
71+
The separate `telemetry_client.track_analyze()` call will be removed from the analyze command handling.
72+
73+
## Data Flow
74+
75+
``mermaid
76+
graph TD
77+
A[User runs analyze command] --> B[CLI Parser]
78+
B --> C[Main Function]
79+
C --> D[Create telemetry properties]
80+
D --> E[Track single Analyze Folder event]
81+
E --> F[Send to PostHog]
82+
```
83+
84+
## Benefits
85+
86+
1. **Single Event Per Command**: Only one telemetry event will be generated per analyze command execution
87+
2. **Mode Differentiation**: The analysis mode (JSON, detailed, matrix, summary) will be captured in event properties
88+
3. **Enhanced Analytics**: Better data for understanding how users interact with the analyze command
89+
4. **Consistency**: Aligns with the pattern used for other commands like security scans
90+
91+
## Implementation Steps
92+
93+
1. Modify the `track_analyze_folder` method in the telemetry client to accept properties
94+
2. Update the analyze command handling in main.rs to:
95+
- Remove the duplicate `track_analyze()` call
96+
- Create properties map with analysis mode and other relevant information
97+
- Call `track_analyze_folder` with the properties
98+
3. Test the implementation to ensure only one event is generated with correct properties
99+
4. Update any related tests
100+
101+
## Testing Plan
102+
103+
1. **Unit Tests**: Update telemetry tests to reflect the new method signature
104+
2. **Integration Tests**: Verify that only one event is generated when running the analyze command
105+
3. **Property Validation**: Confirm that the correct analysis mode is captured in event properties
106+
4. **Edge Cases**: Test with various combinations of command-line options
107+
108+
## Backward Compatibility
109+
110+
This change is backward compatible with existing telemetry infrastructure. The event name remains "Analyze Folder", and the core telemetry collection mechanism is unchanged. The only difference is in the data captured with the event.
111+
112+
## Future Enhancements
113+
114+
1. **Performance Metrics**: Add analysis duration and file count to the telemetry properties
115+
2. **Project Type Detection**: Include detected project types in the event properties
116+
3. **Error Tracking**: Add success/failure status to the events
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# Command Event Normalization Design
2+
3+
## Summary
4+
5+
This document outlines the changes needed to fix the duplicate telemetry events issue in the syncable-cli application. Currently, the `security` and `vulnerabilities` commands each generate two telemetry events, causing data duplication. The solution involves modifying the telemetry client to use descriptive event names directly and removing the duplicate event calls in the command handlers.
6+
7+
## Problem Statement
8+
9+
When running commands like `sync-ctl security .`, two events are generated:
10+
- "security" event with properties
11+
12+
13+
- "Security Scan" event
14+
15+
Similarly for `sync-ctl vulnerabilities .`:
16+
- "vulnerabilities" event with properties
17+
18+
19+
- "Vulnerability Scan" event
20+
21+
This duplication creates unnecessary noise in telemetry data and can skew analytics.
22+
23+
## Solution
24+
25+
The solution involves two key changes:
26+
27+
1. Modify the telemetry client methods to directly use the descriptive event names:
28+
- `track_security()` will track "Security Scan" events
29+
30+
31+
- `track_vulnerabilities()` will track "Vulnerability Scan" events
32+
33+
2. Remove the duplicate event calls in the command handlers:
34+
- Remove `track_security_scan()` call from the security command handler
35+
36+
37+
- Remove `track_vulnerability_scan()` call from the vulnerabilities command handler
38+
39+
## Implementation Details
40+
41+
### File: src/telemetry/client.rs
42+
43+
Update the `track_security` method to use the descriptive event name:
44+
```rust
45+
46+
pub fn track_security(&self, properties: HashMap<String, serde_json::Value>) {
47+
self.track_event("Security Scan", properties);
48+
}
49+
```
50+
51+
Update the `track_vulnerabilities` method to use the descriptive event name:
52+
```rust
53+
54+
pub fn track_vulnerabilities(&self, properties: HashMap<String, serde_json::Value>) {
55+
self.track_event("Vulnerability Scan", properties);
56+
}
57+
```
58+
59+
Update the deprecated methods to be no-ops with deprecation comments:
60+
```rust
61+
62+
63+
pub fn track_security_scan(&self) {
64+
// Deprecated: Use track_security with properties instead
65+
66+
67+
}
68+
69+
pub fn track_vulnerability_scan(&self) {
70+
// Deprecated: Use track_vulnerabilities with properties instead
71+
72+
73+
}
74+
```
75+
76+
### File: src/main.rs
77+
78+
In the Security command handler, remove the duplicate event call:
79+
```rust
80+
81+
82+
// Remove this duplicate call
83+
84+
85+
// if let Some(telemetry_client) = telemetry::get_telemetry_client() {
86+
// telemetry_client.track_security_scan();
87+
// }
88+
```
89+
90+
In the Vulnerabilities command handler, remove the duplicate event call:
91+
```rust
92+
93+
94+
// Remove this duplicate call
95+
96+
97+
// if let Some(telemetry_client) = telemetry::get_telemetry_client() {
98+
// telemetry_client.track_vulnerability_scan();
99+
// }
100+
```
101+
102+
## Benefits
103+
104+
1. **Eliminates Duplicate Events**: Each command will generate exactly one telemetry event
105+
106+
107+
2. **Maintains Event Properties**: All existing properties will still be captured
108+
109+
110+
3. **Consistent Naming**: Event names will clearly indicate the type of scan performed
111+
112+
113+
4. **Backward Compatibility**: Existing telemetry infrastructure remains unchanged
114+
115+
116+
5. **Cleaner Analytics**: Reduces noise in telemetry data, making analysis more accurate
117+

src/analyzer/runtime/detection.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,11 @@ impl RuntimeDetectionEngine {
1414
}
1515

1616
/// Get all available package managers in a project
17-
pub fn get_all_package_managers(project_path: &Path) -> Vec<String> {
17+
pub fn get_all_package_managers(project_path: &Path) -> Vec<super::javascript::PackageManager> {
1818
use super::javascript::RuntimeDetector;
1919

2020
let js_detector = RuntimeDetector::new(project_path.to_path_buf());
2121
js_detector.detect_all_package_managers()
22-
.into_iter()
23-
.map(|pm| pm.as_str().to_string())
24-
.collect()
2522
}
2623

2724
/// Check if a project uses a specific runtime

src/analyzer/security_analyzer.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,7 +1551,7 @@ impl SecurityAnalyzer {
15511551
}
15521552

15531553
// Additional helper methods...
1554-
fn collect_source_files(&self, project_root: &Path, language: &str) -> Result<Vec<PathBuf>, SecurityError> {
1554+
fn collect_source_files(&self, _project_root: &Path, _language: &str) -> Result<Vec<PathBuf>, SecurityError> {
15551555
// TODO: Implement source file collection based on language
15561556
Ok(vec![])
15571557
}
@@ -1874,7 +1874,7 @@ mod tests {
18741874
config.skip_gitignored_files = false;
18751875
config.downgrade_gitignored_severity = true;
18761876

1877-
let analyzer = SecurityAnalyzer::with_config(config).unwrap();
1877+
let _analyzer = SecurityAnalyzer::with_config(config).unwrap();
18781878
// Additional test logic could be added here for downgrade behavior
18791879
}
18801880

@@ -2027,7 +2027,7 @@ mod tests {
20272027

20282028
for pattern in &legitimate_patterns {
20292029
// These should either not match any secret pattern, or be filtered out by context detection
2030-
let matches_old_generic_pattern = pattern.to_lowercase().contains("secret") ||
2030+
let _matches_old_generic_pattern = pattern.to_lowercase().contains("secret") ||
20312031
pattern.to_lowercase().contains("key");
20322032

20332033
// Our new patterns should be more specific and not match env var access

0 commit comments

Comments
 (0)