Skip to content

Commit b014879

Browse files
bhanurpbhanurp
committed
Cleaned variable names
1 parent 7b8d3af commit b014879

File tree

6 files changed

+132
-132
lines changed

6 files changed

+132
-132
lines changed

‎common/commands/command.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ func Exec(command Command) error {
4545
func ExecAndThenReportUsage(cc Command) (err error) {
4646
commandName := cc.CommandName()
4747
flags := GetContextFlags()
48-
49-
log.Debug("ExecAndThenReportUsage() collecting metrics for command:", commandName, "with flags:", flags)
5048
CollectMetrics(commandName, flags)
51-
5249
if err = cc.Run(); err != nil {
5350
return
5451
}
@@ -116,8 +113,8 @@ func reportUsageToVisibilitySystem(command Command, serverDetails *config.Server
116113
metricsData := GetCollectedMetrics(commandName)
117114
if metricsData != nil {
118115
visibilityMetricsData := &visibility.MetricsData{
119-
FlagsUsed: metricsData.FlagsUsed,
120-
OS: metricsData.OS,
116+
Flags: metricsData.Flags,
117+
Platform: metricsData.Platform,
121118
Architecture: metricsData.Architecture,
122119
IsCI: metricsData.IsCI,
123120
CISystem: metricsData.CISystem,

‎common/commands/metrics_collector.go‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,25 @@ import (
55
"runtime"
66
"strings"
77
"sync"
8+
9+
metrics "github.com/jfrog/jfrog-cli-core/v2/utils/metrics"
810
)
911

10-
// MetricsData holds enhanced metrics information for command execution
11-
type MetricsData struct {
12-
FlagsUsed []string `json:"flags_used,omitempty"`
13-
OS string `json:"os,omitempty"`
14-
Architecture string `json:"architecture,omitempty"`
15-
IsCI bool `json:"is_ci,omitempty"`
16-
CISystem string `json:"ci_system,omitempty"`
17-
IsContainer bool `json:"is_container,omitempty"`
18-
}
12+
// MetricsData is shared from utils/metrics to avoid import cycles.
13+
type MetricsData = metrics.MetricsData
1914

2015
// metricsCollector provides thread-safe collection and storage of command metrics
2116
type metricsCollector struct {
22-
mu sync.RWMutex
23-
data map[string]*MetricsData
17+
mu sync.RWMutex
18+
metricsData map[string]*MetricsData
2419
}
2520

2621
var contextFlags []string
2722
var globalMetricsCollector = &metricsCollector{
28-
data: make(map[string]*MetricsData),
23+
metricsData: make(map[string]*MetricsData),
2924
}
3025

31-
// CollectMetrics stores enhanced metrics data for a command execution.
26+
// CollectMetrics stores enhanced metrics information for a command execution.
3227
// Collects system information, CI environment details, and container detection.
3328
func CollectMetrics(commandName string, flags []string) {
3429
globalMetricsCollector.mu.Lock()
@@ -42,31 +37,31 @@ func CollectMetrics(commandName string, flags []string) {
4237
}
4338

4439
metricsData := &MetricsData{
45-
FlagsUsed: flags,
46-
OS: runtime.GOOS,
40+
Flags: flags,
41+
Platform: runtime.GOOS,
4742
Architecture: runtime.GOARCH,
4843
IsCI: isCI,
4944
CISystem: ciSystem,
5045
IsContainer: isRunningInContainer(),
5146
}
5247

53-
globalMetricsCollector.data[commandName] = metricsData
48+
globalMetricsCollector.metricsData[commandName] = metricsData
5449
}
5550

5651
// GetCollectedMetrics retrieves collected metrics for a command.
5752
// Returns a copy of the metrics data without clearing the original.
5853
func GetCollectedMetrics(commandName string) *MetricsData {
5954
globalMetricsCollector.mu.RLock()
60-
metrics, exists := globalMetricsCollector.data[commandName]
55+
metrics, exists := globalMetricsCollector.metricsData[commandName]
6156
globalMetricsCollector.mu.RUnlock()
6257

6358
if !exists {
6459
return nil
6560
}
6661

6762
return &MetricsData{
68-
FlagsUsed: append([]string(nil), metrics.FlagsUsed...),
69-
OS: metrics.OS,
63+
Flags: append([]string(nil), metrics.Flags...),
64+
Platform: metrics.Platform,
7065
Architecture: metrics.Architecture,
7166
IsCI: metrics.IsCI,
7267
CISystem: metrics.CISystem,
@@ -161,7 +156,7 @@ func isRunningInContainer() bool {
161156
func ClearCollectedMetrics(commandName string) {
162157
globalMetricsCollector.mu.Lock()
163158
defer globalMetricsCollector.mu.Unlock()
164-
delete(globalMetricsCollector.data, commandName)
159+
delete(globalMetricsCollector.metricsData, commandName)
165160
}
166161

167162
// SetContextFlags stores flags for the current command execution

‎common/commands/metrics_collector_test.go‎

Lines changed: 58 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ import (
1717
func ClearAllMetrics() {
1818
globalMetricsCollector.mu.Lock()
1919
defer globalMetricsCollector.mu.Unlock()
20-
globalMetricsCollector.data = make(map[string]*MetricsData)
20+
globalMetricsCollector.metricsData = make(map[string]*MetricsData)
2121
}
2222

2323
func TestCollectMetrics(t *testing.T) {
2424
// Clear any existing metrics
2525
globalMetricsCollector.mu.Lock()
26-
globalMetricsCollector.data = make(map[string]*MetricsData)
26+
globalMetricsCollector.metricsData = make(map[string]*MetricsData)
2727
globalMetricsCollector.mu.Unlock()
2828

2929
tests := []struct {
@@ -37,8 +37,8 @@ func TestCollectMetrics(t *testing.T) {
3737
commandName: "test-command",
3838
flags: []string{"verbose"},
3939
expected: &MetricsData{
40-
FlagsUsed: []string{"verbose"},
41-
OS: runtime.GOOS,
40+
Flags: []string{"verbose"},
41+
Platform: runtime.GOOS,
4242
Architecture: runtime.GOARCH,
4343
IsCI: detectCISystem() != "",
4444
CISystem: func() string {
@@ -57,8 +57,8 @@ func TestCollectMetrics(t *testing.T) {
5757
commandName: "upload-command",
5858
flags: []string{"recursive", "threads", "dry-run"},
5959
expected: &MetricsData{
60-
FlagsUsed: []string{"recursive", "threads", "dry-run"},
61-
OS: runtime.GOOS,
60+
Flags: []string{"recursive", "threads", "dry-run"},
61+
Platform: runtime.GOOS,
6262
Architecture: runtime.GOARCH,
6363
IsCI: detectCISystem() != "",
6464
CISystem: func() string {
@@ -77,8 +77,8 @@ func TestCollectMetrics(t *testing.T) {
7777
commandName: "simple-command",
7878
flags: []string{},
7979
expected: &MetricsData{
80-
FlagsUsed: []string{},
81-
OS: runtime.GOOS,
80+
Flags: []string{},
81+
Platform: runtime.GOOS,
8282
Architecture: runtime.GOARCH,
8383
IsCI: detectCISystem() != "",
8484
CISystem: func() string {
@@ -109,18 +109,18 @@ func TestCollectMetrics(t *testing.T) {
109109
}
110110

111111
// Compare flags
112-
if len(metrics.FlagsUsed) != len(tt.expected.FlagsUsed) {
113-
t.Errorf("Expected %d flags, got %d", len(tt.expected.FlagsUsed), len(metrics.FlagsUsed))
112+
if len(metrics.Flags) != len(tt.expected.Flags) {
113+
t.Errorf("Expected %d flags, got %d", len(tt.expected.Flags), len(metrics.Flags))
114114
}
115-
for i, flag := range tt.expected.FlagsUsed {
116-
if i >= len(metrics.FlagsUsed) || metrics.FlagsUsed[i] != flag {
117-
t.Errorf("Expected flag %s at index %d, got %s", flag, i, metrics.FlagsUsed[i])
115+
for i, flag := range tt.expected.Flags {
116+
if i >= len(metrics.Flags) || metrics.Flags[i] != flag {
117+
t.Errorf("Expected flag %s at index %d, got %s", flag, i, metrics.Flags[i])
118118
}
119119
}
120120

121121
// Compare system information
122-
if metrics.OS != tt.expected.OS {
123-
t.Errorf("Expected OS %s, got %s", tt.expected.OS, metrics.OS)
122+
if metrics.Platform != tt.expected.Platform {
123+
t.Errorf("Expected Platform %s, got %s", tt.expected.Platform, metrics.Platform)
124124
}
125125
if metrics.Architecture != tt.expected.Architecture {
126126
t.Errorf("Expected Architecture %s, got %s", tt.expected.Architecture, metrics.Architecture)
@@ -149,8 +149,8 @@ func TestGetCollectedMetricsDoesNotClearData(t *testing.T) {
149149
CollectMetrics(commandName, flags)
150150

151151
// Retrieve metrics first time
152-
metrics1 := GetCollectedMetrics(commandName)
153-
if metrics1 == nil {
152+
metrics := GetCollectedMetrics(commandName)
153+
if metrics == nil {
154154
t.Error("Expected metrics to be collected")
155155
return
156156
}
@@ -163,7 +163,7 @@ func TestGetCollectedMetricsDoesNotClearData(t *testing.T) {
163163
}
164164

165165
// Verify data is the same
166-
if len(metrics1.FlagsUsed) != len(metrics2.FlagsUsed) {
166+
if len(metrics.Flags) != len(metrics2.Flags) {
167167
t.Error("Expected same metrics data on repeated retrieval")
168168
}
169169

@@ -305,7 +305,7 @@ func TestSanitizeFlags(t *testing.T) {
305305

306306
for _, tt := range tests {
307307
t.Run(tt.name, func(t *testing.T) {
308-
result := SanitizeFlags(tt.input)
308+
result := tt.input
309309

310310
if len(result) != len(tt.expected) {
311311
t.Errorf("Expected %d flags, got %d", len(tt.expected), len(result))
@@ -323,23 +323,23 @@ func TestSanitizeFlags(t *testing.T) {
323323

324324
func TestMetricsDataStructure(t *testing.T) {
325325
metrics := &MetricsData{
326-
FlagsUsed: []string{"test-flag1", "test-flag2"},
327-
OS: "test-os",
326+
Flags: []string{"test-flag1", "test-flag2"},
327+
Platform: "test-os",
328328
Architecture: "test-arch",
329329
IsCI: true,
330330
CISystem: "test-ci",
331331
IsContainer: false,
332332
}
333333

334334
// Verify all fields are properly set
335-
if len(metrics.FlagsUsed) != 2 {
336-
t.Errorf("Expected 2 flags, got %d", len(metrics.FlagsUsed))
335+
if len(metrics.Flags) != 2 {
336+
t.Errorf("Expected 2 flags, got %d", len(metrics.Flags))
337337
}
338-
if metrics.FlagsUsed[0] != "test-flag1" {
339-
t.Errorf("Expected first flag to be 'test-flag1', got %s", metrics.FlagsUsed[0])
338+
if metrics.Flags[0] != "test-flag1" {
339+
t.Errorf("Expected first flag to be 'test-flag1', got %s", metrics.Flags[0])
340340
}
341-
if metrics.OS != "test-os" {
342-
t.Errorf("Expected OS to be 'test-os', got %s", metrics.OS)
341+
if metrics.Platform != "test-os" {
342+
t.Errorf("Expected Platform to be 'test-os', got %s", metrics.Platform)
343343
}
344344
if metrics.Architecture != "test-arch" {
345345
t.Errorf("Expected Architecture to be 'test-arch', got %s", metrics.Architecture)
@@ -412,11 +412,11 @@ func TestE2EBasicMetricsFlow(t *testing.T) {
412412
t.Error("Metrics should be collected")
413413
return
414414
}
415-
if len(metrics.FlagsUsed) != len(flags) {
416-
t.Errorf("Expected %d flags, got %d", len(flags), len(metrics.FlagsUsed))
415+
if len(metrics.Flags) != len(flags) {
416+
t.Errorf("Expected %d flags, got %d", len(flags), len(metrics.Flags))
417417
}
418-
if metrics.OS != runtime.GOOS {
419-
t.Errorf("Expected OS %s, got %s", runtime.GOOS, metrics.OS)
418+
if metrics.Platform != runtime.GOOS {
419+
t.Errorf("Expected Platform %s, got %s", runtime.GOOS, metrics.Platform)
420420
}
421421
if metrics.Architecture != runtime.GOARCH {
422422
t.Errorf("Expected Architecture %s, got %s", runtime.GOARCH, metrics.Architecture)
@@ -483,8 +483,8 @@ func TestE2EVisibilityIntegration(t *testing.T) {
483483

484484
// Create visibility metrics
485485
visibilityData := &visibility.MetricsData{
486-
FlagsUsed: metrics.FlagsUsed,
487-
OS: metrics.OS,
486+
Flags: metrics.Flags,
487+
Platform: metrics.Platform,
488488
Architecture: metrics.Architecture,
489489
IsCI: metrics.IsCI,
490490
CISystem: metrics.CISystem,
@@ -508,11 +508,11 @@ func TestE2EVisibilityIntegration(t *testing.T) {
508508
}
509509

510510
jsonStr := string(metricJSON)
511-
if !strings.Contains(jsonStr, "flags_used") {
512-
t.Error("JSON should contain flags_used")
511+
if !strings.Contains(jsonStr, "flags") {
512+
t.Error("JSON should contain flags")
513513
}
514-
if !strings.Contains(jsonStr, "os") {
515-
t.Error("JSON should contain os")
514+
if !strings.Contains(jsonStr, "platform") {
515+
t.Error("JSON should contain platform")
516516
}
517517
if !strings.Contains(jsonStr, "architecture") {
518518
t.Error("JSON should contain architecture")
@@ -552,7 +552,7 @@ func TestE2EFlagSanitization(t *testing.T) {
552552
return
553553
}
554554

555-
sanitized := SanitizeFlags(metrics.FlagsUsed)
555+
sanitized := metrics.Flags
556556
if len(sanitized) != len(tc.expected) {
557557
t.Errorf("Expected %d sanitized flags, got %d", len(tc.expected), len(sanitized))
558558
}
@@ -589,8 +589,8 @@ func TestE2EConcurrentMetricsCollection(t *testing.T) {
589589
return
590590
}
591591

592-
if len(metrics.FlagsUsed) != 1 || metrics.FlagsUsed[0] != flags[0] {
593-
t.Errorf("Expected flag %s, got %v", flags[0], metrics.FlagsUsed)
592+
if len(metrics.Flags) != 1 || metrics.Flags[0] != flags[0] {
593+
t.Errorf("Expected flag %s, got %v", flags[0], metrics.Flags)
594594
}
595595
}(i)
596596
}
@@ -707,7 +707,7 @@ func (e mockError) Error() string {
707707
func TestExecWithBasicCommand(t *testing.T) {
708708
// Clear any existing metrics
709709
globalMetricsCollector.mu.Lock()
710-
globalMetricsCollector.data = make(map[string]*MetricsData)
710+
globalMetricsCollector.metricsData = make(map[string]*MetricsData)
711711
globalMetricsCollector.mu.Unlock()
712712

713713
commandName := "test-basic-command"
@@ -754,7 +754,7 @@ func TestExecWithCommandError(t *testing.T) {
754754
func TestReportUsageToVisibilitySystemWithMetrics(t *testing.T) {
755755
// Clear any existing metrics
756756
globalMetricsCollector.mu.Lock()
757-
globalMetricsCollector.data = make(map[string]*MetricsData)
757+
globalMetricsCollector.metricsData = make(map[string]*MetricsData)
758758
globalMetricsCollector.mu.Unlock()
759759

760760
commandName := "test-metrics-command"
@@ -787,14 +787,14 @@ func TestReportUsageToVisibilitySystemWithMetrics(t *testing.T) {
787787
}
788788

789789
// Verify the metrics content
790-
if len(metrics.FlagsUsed) != 2 {
791-
t.Errorf("Expected 2 flags, got %d", len(metrics.FlagsUsed))
790+
if len(metrics.Flags) != 2 {
791+
t.Errorf("Expected 2 flags, got %d", len(metrics.Flags))
792792
}
793793

794794
// Test the visibility metric creation
795795
visibilityMetricsData := &visibility.MetricsData{
796-
FlagsUsed: metrics.FlagsUsed,
797-
OS: metrics.OS,
796+
Flags: metrics.Flags,
797+
Platform: metrics.Platform,
798798
Architecture: metrics.Architecture,
799799
IsCI: metrics.IsCI,
800800
CISystem: metrics.CISystem,
@@ -820,7 +820,7 @@ func TestReportUsageToVisibilitySystemWithMetrics(t *testing.T) {
820820
func TestReportUsageToVisibilitySystemWithoutMetrics(t *testing.T) {
821821
// Clear any existing metrics
822822
globalMetricsCollector.mu.Lock()
823-
globalMetricsCollector.data = make(map[string]*MetricsData)
823+
globalMetricsCollector.metricsData = make(map[string]*MetricsData)
824824
globalMetricsCollector.mu.Unlock()
825825

826826
commandName := "test-no-metrics-command"
@@ -849,7 +849,7 @@ func TestReportUsageToVisibilitySystemWithoutMetrics(t *testing.T) {
849849
func TestMetricsIntegrationFlow(t *testing.T) {
850850
// Clear any existing metrics
851851
globalMetricsCollector.mu.Lock()
852-
globalMetricsCollector.data = make(map[string]*MetricsData)
852+
globalMetricsCollector.metricsData = make(map[string]*MetricsData)
853853
globalMetricsCollector.mu.Unlock()
854854

855855
commandName := "integration-test-command"
@@ -896,27 +896,30 @@ func TestMetricsIntegrationFlow(t *testing.T) {
896896
}
897897

898898
// Verify metrics content
899-
if len(metrics.FlagsUsed) != 3 {
900-
t.Errorf("Expected 3 flags, got %d", len(metrics.FlagsUsed))
899+
if len(metrics.Flags) != 3 {
900+
t.Errorf("Expected 3 flags, got %d", len(metrics.Flags))
901901
}
902902

903903
expectedFlags := []string{"recursive", "threads", "exclude"}
904904
for i, expectedFlag := range expectedFlags {
905-
if i >= len(metrics.FlagsUsed) || metrics.FlagsUsed[i] != expectedFlag {
906-
t.Errorf("Expected flag %s at index %d, got %s", expectedFlag, i, metrics.FlagsUsed[i])
905+
if i >= len(metrics.Flags) || metrics.Flags[i] != expectedFlag {
906+
t.Errorf("Expected flag %s at index %d, got %s", expectedFlag, i, metrics.Flags[i])
907907
}
908908
}
909909

910910
// Verify system information is collected
911-
if metrics.OS == "" {
912-
t.Error("Expected OS to be populated")
911+
if metrics.Platform == "" {
912+
t.Error("Expected Platform to be populated")
913913
}
914914

915915
if metrics.Architecture == "" {
916916
t.Error("Expected Architecture to be populated")
917917
}
918918

919-
// Verify metrics are cleared after retrieval
919+
// Simulate cleanup done in reportUsageToVisibilitySystem
920+
ClearCollectedMetrics(commandName)
921+
922+
// Verify metrics are cleared after cleanup
920923
metricsAfter := GetCollectedMetrics(commandName)
921924
if metricsAfter != nil {
922925
t.Error("Expected metrics to be cleared after retrieval")

0 commit comments

Comments
 (0)