Skip to content

Commit 7b8d3af

Browse files
committed
Fixes from self review
1 parent beee2b3 commit 7b8d3af

File tree

5 files changed

+20
-45
lines changed

5 files changed

+20
-45
lines changed

common/commands/command.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,10 @@ type Command interface {
2929
CommandName() string
3030
}
3131

32-
// Exec executes a command and collects enhanced metrics
3332
func Exec(command Command) error {
3433
commandName := command.CommandName()
3534
flags := GetContextFlags()
36-
37-
log.Debug("Exec() collecting metrics for command:", commandName, "with flags:", flags)
3835
CollectMetrics(commandName, flags)
39-
4036
channel := make(chan bool)
4137
go reportUsage(command, channel)
4238
err := command.Run()
@@ -118,9 +114,7 @@ func reportUsageToVisibilitySystem(command Command, serverDetails *config.Server
118114

119115
commandName := command.CommandName()
120116
metricsData := GetCollectedMetrics(commandName)
121-
122117
if metricsData != nil {
123-
log.Debug("Using enhanced metrics for command:", commandName)
124118
visibilityMetricsData := &visibility.MetricsData{
125119
FlagsUsed: metricsData.FlagsUsed,
126120
OS: metricsData.OS,
@@ -132,7 +126,6 @@ func reportUsageToVisibilitySystem(command Command, serverDetails *config.Server
132126
commandsCountMetric = visibility.NewCommandsCountMetricWithEnhancedData(commandName, visibilityMetricsData)
133127
ClearCollectedMetrics(commandName)
134128
} else {
135-
log.Debug("No enhanced metrics found for command:", commandName, "- using standard metric")
136129
commandsCountMetric = visibility.NewCommandsCountMetric(commandName)
137130
}
138131

common/commands/metrics_collector.go

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type metricsCollector struct {
2323
data map[string]*MetricsData
2424
}
2525

26+
var contextFlags []string
2627
var globalMetricsCollector = &metricsCollector{
2728
data: make(map[string]*MetricsData),
2829
}
@@ -163,42 +164,14 @@ func ClearCollectedMetrics(commandName string) {
163164
delete(globalMetricsCollector.data, commandName)
164165
}
165166

166-
// ClearAllMetrics clears all stored metrics
167-
func ClearAllMetrics() {
168-
globalMetricsCollector.mu.Lock()
169-
defer globalMetricsCollector.mu.Unlock()
170-
globalMetricsCollector.data = make(map[string]*MetricsData)
171-
}
172-
173-
var contextFlags []string
174-
var contextFlagsMutex sync.Mutex
175-
176167
// SetContextFlags stores flags for the current command execution
177168
func SetContextFlags(flags []string) {
178-
contextFlagsMutex.Lock()
179-
defer contextFlagsMutex.Unlock()
180169
contextFlags = append([]string(nil), flags...)
181170
}
182171

183172
// GetContextFlags retrieves and clears the stored flags
184173
func GetContextFlags() []string {
185-
contextFlagsMutex.Lock()
186-
defer contextFlagsMutex.Unlock()
187174
flags := contextFlags
188175
contextFlags = nil
189176
return flags
190177
}
191-
192-
// TransferMetrics moves metrics from one command name to another.
193-
// Useful when collecting metrics with a temporary key that needs to be moved to the actual command name.
194-
func TransferMetrics(fromCommandName, toCommandName string) bool {
195-
globalMetricsCollector.mu.Lock()
196-
defer globalMetricsCollector.mu.Unlock()
197-
198-
if metricsData, exists := globalMetricsCollector.data[fromCommandName]; exists {
199-
globalMetricsCollector.data[toCommandName] = metricsData
200-
delete(globalMetricsCollector.data, fromCommandName)
201-
return true
202-
}
203-
return false
204-
}

common/commands/metrics_collector_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ import (
1313
"github.com/jfrog/jfrog-cli-core/v2/utils/usage/visibility"
1414
)
1515

16+
// ClearAllMetrics clears all stored metrics
17+
func ClearAllMetrics() {
18+
globalMetricsCollector.mu.Lock()
19+
defer globalMetricsCollector.mu.Unlock()
20+
globalMetricsCollector.data = make(map[string]*MetricsData)
21+
}
22+
1623
func TestCollectMetrics(t *testing.T) {
1724
// Clear any existing metrics
1825
globalMetricsCollector.mu.Lock()

plugins/components/conversionlayer.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ func convertBoolFlag(f BoolFlag) cli.Flag {
368368
}
369369
}
370370

371-
// getActionFunc creates an action function that converts CLI context and collects metrics
371+
// getActionFunc wrap the base's ActionFunc with our own, while retrieving needed information from the Context.
372372
func getActionFunc(cmd Command) cli.ActionFunc {
373373
return func(baseContext *cli.Context) error {
374374
pluginContext, err := ConvertContext(baseContext, cmd.Flags...)
@@ -428,18 +428,20 @@ func fillFlagMaps(c *Context, baseContext *cli.Context, originalFlags []Flag) er
428428
}
429429
if boolFlag, ok := flag.(BoolFlag); ok {
430430
val := getValueForBoolFlag(boolFlag, baseContext)
431+
// Only store the flag if:
432+
// - The user explicitly set it, OR
433+
// - The resolved value is 'true' (e.g., default is true and user didn't override it)
434+
// This avoids adding unnecessary 'false' flags that aren't relevant.
431435
if baseContext.IsSet(boolFlag.Name) || val {
436+
// Store the flag and its resolved value in the context map
432437
c.boolFlags[boolFlag.Name] = val
433438
}
434439
}
435440
}
436-
437441
c.FlagsUsed = flagsUsed
438-
439442
if c.CommandName != "" {
440443
commands.CollectMetrics(c.CommandName, flagsUsed)
441444
}
442-
443445
return nil
444446
}
445447

utils/usage/visibility/commands_count_metric.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type commandsCountLabels struct {
2222
ProductID string `json:"product_id"`
2323
ProductVersion string `json:"product_version"`
2424
FeatureID string `json:"feature_id"`
25-
OIDCUsed string `json:"oidc_used"`
25+
ProviderType string `json:"provider_type"`
2626
JobID string `json:"job_id"`
2727
RunID string `json:"run_id"`
2828
GitRepo string `json:"git_repo"`
@@ -43,7 +43,7 @@ func NewCommandsCountMetric(commandName string) services.VisibilityMetric {
4343
ProductID: coreutils.GetCliUserAgentName(),
4444
ProductVersion: coreutils.GetCliUserAgentVersion(),
4545
FeatureID: commandName,
46-
OIDCUsed: os.Getenv(coreutils.OidcProviderType),
46+
ProviderType: os.Getenv(coreutils.OidcProviderType),
4747
JobID: os.Getenv(coreutils.CIJobID),
4848
RunID: os.Getenv(coreutils.CIRunID),
4949
GitRepo: os.Getenv(coreutils.SourceCodeRepository),
@@ -63,10 +63,10 @@ func NewCommandsCountMetricWithEnhancedData(commandName string, metricsData *Met
6363
ProductID: coreutils.GetCliUserAgentName(),
6464
ProductVersion: coreutils.GetCliUserAgentVersion(),
6565
FeatureID: commandName,
66-
OIDCUsed: os.Getenv("JFROG_CLI_USAGE_OIDC_USED"),
67-
JobID: os.Getenv("JFROG_CLI_USAGE_JOB_ID"),
68-
RunID: os.Getenv("JFROG_CLI_USAGE_RUN_ID"),
69-
GitRepo: os.Getenv("JFROG_CLI_USAGE_GIT_REPO"),
66+
ProviderType: os.Getenv(coreutils.OidcProviderType),
67+
JobID: os.Getenv(coreutils.CIJobID),
68+
RunID: os.Getenv(coreutils.CIRunID),
69+
GitRepo: os.Getenv(coreutils.SourceCodeRepository),
7070
GhTokenForCodeScanningAlertsProvided: os.Getenv("JFROG_CLI_USAGE_GH_TOKEN_FOR_CODE_SCANNING_ALERTS_PROVIDED"),
7171
Flags: "",
7272
Platform: "",

0 commit comments

Comments
 (0)