From 8584a5994176492d0b0c9d814ef924d798165d98 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 11 May 2026 16:25:56 +0200 Subject: [PATCH 1/3] JGC-481 - Report usage of api command --- general/api/cli.go | 56 ++++++++++++++++- general/api/cli_test.go | 133 +++++++++++++++++++++++++++++++++++++++- go.mod | 2 +- go.sum | 4 +- 4 files changed, 187 insertions(+), 8 deletions(-) diff --git a/general/api/cli.go b/general/api/cli.go index 7bf2f866c..9dbb0643e 100644 --- a/general/api/cli.go +++ b/general/api/cli.go @@ -9,6 +9,7 @@ import ( commonCliUtils "github.com/jfrog/jfrog-cli-core/v2/common/cliutils" coreconfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" + "github.com/jfrog/jfrog-cli-core/v2/common/commands" "github.com/jfrog/jfrog-cli/utils/cliutils" "github.com/jfrog/jfrog-client-go/http/httpclient" "github.com/jfrog/jfrog-client-go/utils/errorutils" @@ -46,14 +47,28 @@ func Command(c *cli.Context) error { return err } - return runApiCmd(c, serverDetails, os.Stdout) + var flagsUsed []string + for _, f := range c.Command.Flags { + name := f.GetName() + if c.IsSet(name) { + flagsUsed = append(flagsUsed, name) + } + } + + return runApiCmd(c, serverDetails, os.Stdout, flagsUsed) } -func runApiCmd(c commandContext, serverDetails *coreconfig.ServerDetails, stdOut io.Writer) error { +func runApiCmd(c commandContext, serverDetails *coreconfig.ServerDetails, stdOut io.Writer, flagsUsed []string) error { if serverDetails.GetUrl() == "" { return errorutils.CheckErrorf("no JFrog Platform URL specified, either via the --url flag or as part of the server configuration") } + commands.CollectMetrics("api", flagsUsed) + + timeout := time.Duration(c.Int(flagTimeout)) * time.Second + waitUsageReport := startUsageReport(serverDetails) + defer waitForUsageReport(waitUsageReport, timeout) + pathArg := c.Args().First() fullURL, err := joinPlatformAPIURL(serverDetails.GetUrl(), pathArg) if err != nil { @@ -71,7 +86,6 @@ func runApiCmd(c commandContext, serverDetails *coreconfig.ServerDetails, stdOut return err } - timeout := time.Duration(c.Int(flagTimeout)) * time.Second client, err := newPlatformHttpClient(serverDetails, timeout) if err != nil { return err @@ -80,6 +94,42 @@ func runApiCmd(c commandContext, serverDetails *coreconfig.ServerDetails, stdOut return exchangeAndPrint(client, c, method, fullURL, body, details, stdOut) } +// reportUsageFn is the usage reporter invoked by startUsageReport. Overridable +// in tests to inject fakes (panic, slow, fast) without touching HTTP. +var reportUsageFn = commands.ReportUsage + +// startUsageReport launches the usage report in a goroutine, recovering from +// any panic so that a misbehaving reporter cannot crash the process. The +// returned channel is signaled by ReportUsage on success, or closed by the +// recover path on panic — either way the caller's receive unblocks. +func startUsageReport(serverDetails *coreconfig.ServerDetails) chan bool { + waitUsageReport := make(chan bool) + go func() { + defer func() { + if r := recover(); r != nil { + log.Debug("jf api: usage report panicked:", r) + close(waitUsageReport) + } + }() + reportUsageFn("api", serverDetails, waitUsageReport) + }() + return waitUsageReport +} + +// waitForUsageReport blocks until the usage report completes or the timeout +// elapses. A non-positive timeout waits indefinitely. +func waitForUsageReport(wait <-chan bool, timeout time.Duration) { + if timeout <= 0 { + <-wait + return + } + select { + case <-wait: + case <-time.After(timeout): + log.Debug("jf api: usage report did not complete within", timeout) + } +} + func httpMethodOrDefault(c commandContext) string { method := strings.ToUpper(strings.TrimSpace(c.String(flagMethod))) if method == "" { diff --git a/general/api/cli_test.go b/general/api/cli_test.go index cdf82c43d..4abe7e490 100644 --- a/general/api/cli_test.go +++ b/general/api/cli_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/jfrog/jfrog-cli-core/v2/common/commands" coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" testhelpers "github.com/jfrog/jfrog-cli/utils/tests" "github.com/jfrog/jfrog-client-go/utils/errorutils" @@ -486,7 +487,7 @@ func TestApi(t *testing.T) { t.Cleanup(func() { clientlog.SetLogger(prevLogger) }) clientlog.SetLogger(clientlog.NewLoggerWithFlags(clientlog.INFO, &stdErr, 0)) - err := runApiCmd(ctx, serverDetails, &stdOut) + err := runApiCmd(ctx, serverDetails, &stdOut, nil) if tt.wantErr != nil { assert.Error(t, err) @@ -529,7 +530,7 @@ func TestApiTimeoutExpired(t *testing.T) { }) var stdOut bytes.Buffer - err := runApiCmd(ctx, serverDetails, &stdOut) + err := runApiCmd(ctx, serverDetails, &stdOut, nil) assert.Error(t, err, "expected a timeout error") } @@ -648,3 +649,131 @@ func (mc *mockContext) setInt(name string, value int) { mc.intMap[name] = value mc.setMap[name] = true } + +// swapReportUsageFn replaces reportUsageFn for the duration of the test and +// restores it on cleanup. Tests using this must not run in parallel. +func swapReportUsageFn(t *testing.T, fn func(string, *coreConfig.ServerDetails, chan<- bool)) { + t.Helper() + prev := reportUsageFn + reportUsageFn = fn + t.Cleanup(func() { reportUsageFn = prev }) +} + +// newTestServer starts an httptest server that returns 200/OK and serverDetails +// pointing at it. Adequate for tests that only care about runApiCmd flow. +func newTestServer(t *testing.T) (*httptest.Server, *coreConfig.ServerDetails) { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte("OK")); err != nil { + t.Log(err) + } + })) + t.Cleanup(srv.Close) + return srv, &coreConfig.ServerDetails{Url: srv.URL, AccessToken: "my-token"} +} + +func TestRunApiCmd_CollectsMetrics(t *testing.T) { + // Prevent the real reporter from doing anything during the test. + swapReportUsageFn(t, func(_ string, _ *coreConfig.ServerDetails, ch chan<- bool) { + ch <- true + }) + + _, serverDetails := newTestServer(t) + ctx := newMockContext(&commandArgs{path: "/success"}) + + var stdOut bytes.Buffer + flags := []string{"verbose", "header"} + require.NoError(t, runApiCmd(ctx, serverDetails, &stdOut, flags)) + + got := commands.GetCollectedMetrics("api") + require.NotNil(t, got, "expected metrics to be collected for command \"api\"") + assert.Equal(t, flags, got.Flags) +} + +func TestWaitForUsageReport(t *testing.T) { + t.Run("non-positive timeout waits for signal", func(t *testing.T) { + ch := make(chan bool, 1) + ch <- true + start := time.Now() + waitForUsageReport(ch, 0) + assert.Less(t, time.Since(start), 50*time.Millisecond) + }) + + t.Run("returns immediately when signaled before timeout", func(t *testing.T) { + ch := make(chan bool, 1) + ch <- true + start := time.Now() + waitForUsageReport(ch, time.Second) + assert.Less(t, time.Since(start), 50*time.Millisecond) + }) + + t.Run("returns at timeout when never signaled", func(t *testing.T) { + ch := make(chan bool) + start := time.Now() + waitForUsageReport(ch, 100*time.Millisecond) + elapsed := time.Since(start) + assert.GreaterOrEqual(t, elapsed, 100*time.Millisecond) + assert.Less(t, elapsed, 500*time.Millisecond) + }) +} + +func TestRunApiCmd_UsageReportDisabled(t *testing.T) { + t.Setenv("JFROG_CLI_REPORT_USAGE", "false") + + _, serverDetails := newTestServer(t) + ctx := newMockContext(&commandArgs{path: "/success"}) + + var stdOut bytes.Buffer + start := time.Now() + require.NoError(t, runApiCmd(ctx, serverDetails, &stdOut, nil)) + // With reporting disabled the real reporter is a near-instant no-op. + assert.Less(t, time.Since(start), 2*time.Second) +} + +func TestStartUsageReport_PanicIsRecovered(t *testing.T) { + swapReportUsageFn(t, func(_ string, _ *coreConfig.ServerDetails, _ chan<- bool) { + panic("boom") + }) + + ch := startUsageReport(&coreConfig.ServerDetails{}) + select { + case <-ch: + // Channel was closed by the recover path; receive returns the zero value. + case <-time.After(time.Second): + t.Fatal("startUsageReport did not unblock after panic") + } +} + +func TestRunApiCmd_UsageReportTimeout(t *testing.T) { + swapReportUsageFn(t, func(_ string, _ *coreConfig.ServerDetails, ch chan<- bool) { + time.Sleep(3 * time.Second) + ch <- true + }) + + _, serverDetails := newTestServer(t) + ctx := newMockContext(&commandArgs{path: "/success", timeout: 1}) + + var stdOut bytes.Buffer + start := time.Now() + require.NoError(t, runApiCmd(ctx, serverDetails, &stdOut, nil)) + elapsed := time.Since(start) + // Should bail out at the 1 s usage-report timeout, not wait the full 3 s. + assert.GreaterOrEqual(t, elapsed, time.Second) + assert.Less(t, elapsed, 2500*time.Millisecond) +} + +func TestRunApiCmd_UsageReportCompletes(t *testing.T) { + swapReportUsageFn(t, func(_ string, _ *coreConfig.ServerDetails, ch chan<- bool) { + ch <- true + }) + + _, serverDetails := newTestServer(t) + ctx := newMockContext(&commandArgs{path: "/success", timeout: 30}) + + var stdOut bytes.Buffer + start := time.Now() + require.NoError(t, runApiCmd(ctx, serverDetails, &stdOut, nil)) + // Reporter signals immediately, so the call should not approach the timeout. + assert.Less(t, time.Since(start), 2*time.Second) +} diff --git a/go.mod b/go.mod index 78e4089af..559782f04 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/jfrog/gofrog v1.7.6 github.com/jfrog/jfrog-cli-application v1.0.2-0.20260405065840-c930d515ef34 github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260429074430-a5871f2898b5 - github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260430125911-ad12ac6f1316 + github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260511135547-73790f4edd53 github.com/jfrog/jfrog-cli-evidence v0.9.2 github.com/jfrog/jfrog-cli-platform-services v1.10.1-0.20260430094150-ce7d9b371c6f github.com/jfrog/jfrog-cli-security v1.28.0 diff --git a/go.sum b/go.sum index a227a9f55..b350201cf 100644 --- a/go.sum +++ b/go.sum @@ -420,10 +420,10 @@ github.com/jfrog/jfrog-cli-application v1.0.2-0.20260405065840-c930d515ef34 h1:q github.com/jfrog/jfrog-cli-application v1.0.2-0.20260405065840-c930d515ef34/go.mod h1:xum2HquWO5uExa/A7MQs3TgJJVEeoqTR+6Z4mfBr1Xw= github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260429074430-a5871f2898b5 h1:+52DDmdSZFP1dxgeu0pkB1sQuoHa0PWbW7HVdFOqK3A= github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260429074430-a5871f2898b5/go.mod h1:BV+aCTQsaZeFec2WjgmQjqlxecju4CkkM9NqfiFyjo0= -github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260430091103-6242ecf15d29 h1:J5+08rOpv/avgt53jNFZ+j5gU8mllcj7Dcfja5Ewodw= -github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260430091103-6242ecf15d29/go.mod h1:bjAkVD8c2W+jg4whqy10bSXDC/c+Se8/ll/GPp5F/+0= github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260430125911-ad12ac6f1316 h1:xAl5D+SjLeRH1gCsSHFPpXJeQQBv2HDGqDTDkFOKJ2s= github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260430125911-ad12ac6f1316/go.mod h1:bjAkVD8c2W+jg4whqy10bSXDC/c+Se8/ll/GPp5F/+0= +github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260511135547-73790f4edd53 h1:ResGsR2FN6msj7gndD35Pu056hNVusKcCDB3AZJYZ+I= +github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260511135547-73790f4edd53/go.mod h1:bjAkVD8c2W+jg4whqy10bSXDC/c+Se8/ll/GPp5F/+0= github.com/jfrog/jfrog-cli-evidence v0.9.2 h1:huiBzQSI9z3OF3l2RphthdXl1aH9zBsvAt+zLsApORI= github.com/jfrog/jfrog-cli-evidence v0.9.2/go.mod h1:R9faPfyQESBmKrdZCmHvlpmYSHmffswjNnFeT3RMq8I= github.com/jfrog/jfrog-cli-platform-services v1.10.1-0.20260430094150-ce7d9b371c6f h1:M1cesbKYSznwPA76dNctjCELxGx34TSSjwoYnJm9/6Y= From bd207e74286d0fafd5506ee469ac62653e13c2ab Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 11 May 2026 16:26:41 +0200 Subject: [PATCH 2/3] JGC-485 - Report usage of api command From 7afbd7da6c3f321edf971c4ba275bbda3a62994e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 11 May 2026 16:35:56 +0200 Subject: [PATCH 3/3] JGC-485 - Fix lint issues --- general/api/cli_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/general/api/cli_test.go b/general/api/cli_test.go index 4abe7e490..26ed3ef37 100644 --- a/general/api/cli_test.go +++ b/general/api/cli_test.go @@ -661,7 +661,7 @@ func swapReportUsageFn(t *testing.T, fn func(string, *coreConfig.ServerDetails, // newTestServer starts an httptest server that returns 200/OK and serverDetails // pointing at it. Adequate for tests that only care about runApiCmd flow. -func newTestServer(t *testing.T) (*httptest.Server, *coreConfig.ServerDetails) { +func newTestServer(t *testing.T) *coreConfig.ServerDetails { t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) @@ -670,7 +670,7 @@ func newTestServer(t *testing.T) (*httptest.Server, *coreConfig.ServerDetails) { } })) t.Cleanup(srv.Close) - return srv, &coreConfig.ServerDetails{Url: srv.URL, AccessToken: "my-token"} + return &coreConfig.ServerDetails{Url: srv.URL, AccessToken: "my-token"} } func TestRunApiCmd_CollectsMetrics(t *testing.T) { @@ -679,7 +679,7 @@ func TestRunApiCmd_CollectsMetrics(t *testing.T) { ch <- true }) - _, serverDetails := newTestServer(t) + serverDetails := newTestServer(t) ctx := newMockContext(&commandArgs{path: "/success"}) var stdOut bytes.Buffer @@ -721,7 +721,7 @@ func TestWaitForUsageReport(t *testing.T) { func TestRunApiCmd_UsageReportDisabled(t *testing.T) { t.Setenv("JFROG_CLI_REPORT_USAGE", "false") - _, serverDetails := newTestServer(t) + serverDetails := newTestServer(t) ctx := newMockContext(&commandArgs{path: "/success"}) var stdOut bytes.Buffer @@ -751,7 +751,7 @@ func TestRunApiCmd_UsageReportTimeout(t *testing.T) { ch <- true }) - _, serverDetails := newTestServer(t) + serverDetails := newTestServer(t) ctx := newMockContext(&commandArgs{path: "/success", timeout: 1}) var stdOut bytes.Buffer @@ -768,7 +768,7 @@ func TestRunApiCmd_UsageReportCompletes(t *testing.T) { ch <- true }) - _, serverDetails := newTestServer(t) + serverDetails := newTestServer(t) ctx := newMockContext(&commandArgs{path: "/success", timeout: 30}) var stdOut bytes.Buffer