From 817a6eb678cc40bf950ea81d335e2c6882f0ff3a Mon Sep 17 00:00:00 2001 From: Sergiu Ghitea <28300158+sergiught@users.noreply.github.com> Date: Wed, 19 Apr 2023 16:15:40 +0200 Subject: [PATCH 1/5] Fix logs tail issue when there are no logs --- internal/cli/logs.go | 41 ++++++------ internal/cli/logs_test.go | 128 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 21 deletions(-) diff --git a/internal/cli/logs.go b/internal/cli/logs.go index c8b164534..594a08e16 100644 --- a/internal/cli/logs.go +++ b/internal/cli/logs.go @@ -9,6 +9,10 @@ import ( "github.com/spf13/cobra" ) +// Pagination max out at 1000 entries in total +// https://auth0.com/docs/logs/retrieve-log-events-using-mgmt-api#limitations +const logsPerPageLimit = 1000 + var ( logsFilter = Flag{ Name: "Filter", @@ -63,7 +67,7 @@ func listLogsCmd(cli *cli) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { list, err := getLatestLogs(cli, inputs.Num, inputs.Filter) if err != nil { - return fmt.Errorf("An unexpected error occurred while getting logs: %v", err) + return fmt.Errorf("failed to get logs: %w", err) } hasFilter := inputs.Filter != "" @@ -100,28 +104,26 @@ func tailLogsCmd(cli *cli) *cobra.Command { auth0 logs tail --filter "type:f" # See the full list of type codes at https://auth0.com/docs/logs/log-event-type-codes auth0 logs tail -n 100`, RunE: func(cmd *cobra.Command, args []string) error { - lastLogID := "" list, err := getLatestLogs(cli, inputs.Num, inputs.Filter) if err != nil { - return fmt.Errorf("An unexpected error occurred while getting logs: %v", err) + return fmt.Errorf("failed to get logs: %w", err) } - // TODO(cyx): This is a hack for now to make the - // streaming work faster. - // - // Create a `set` to detect duplicates clientside. - set := make(map[string]struct{}) - list = dedupeLogs(list, set) - - if len(list) > 0 { - lastLogID = list[len(list)-1].GetLogID() + if len(list) == 0 { + cli.renderer.EmptyState("logs") + cli.renderer.Infof("To generate logs, run a test command like 'auth0 test login' or 'auth0 test token'") + return nil } logsCh := make(chan []*management.Log) + lastLogID := list[len(list)-1].GetLogID() + + // Create a `set` to detect duplicates clientside. + set := make(map[string]struct{}) + list = dedupeLogs(list, set) + go func() { - // This is pretty important and allows - // us to close / terminate the command. defer close(logsCh) for { @@ -138,7 +140,7 @@ func tailLogsCmd(cli *cli) *cobra.Command { list, err = cli.api.Log.List(queryParams...) if err != nil { - cli.renderer.Errorf("An unexpected error occurred while getting logs: %v", err) + cli.renderer.Errorf("Failed to get latest logs: %v", err) return } @@ -148,7 +150,7 @@ func tailLogsCmd(cli *cli) *cobra.Command { } if len(list) < 90 { - // Not a lot is happening, sleep on it + // Not a lot is happening, sleep on it. time.Sleep(1 * time.Second) } } @@ -168,11 +170,8 @@ func tailLogsCmd(cli *cli) *cobra.Command { func getLatestLogs(cli *cli, n int, filter string) ([]*management.Log, error) { page := 0 perPage := n - - if perPage > 1000 { - // Pagination max out at 1000 entries in total - // https://auth0.com/docs/logs/retrieve-log-events-using-mgmt-api#limitations - perPage = 1000 + if perPage > logsPerPageLimit { + perPage = logsPerPageLimit } queryParams := []management.RequestOption{ diff --git a/internal/cli/logs_test.go b/internal/cli/logs_test.go index 50b62f886..06b3fe8b2 100644 --- a/internal/cli/logs_test.go +++ b/internal/cli/logs_test.go @@ -1,16 +1,144 @@ package cli import ( + "bytes" + "fmt" + "io" "testing" "time" "github.com/auth0/go-auth0/management" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/auth0/auth0-cli/internal/auth0" + "github.com/auth0/auth0-cli/internal/auth0/mock" + "github.com/auth0/auth0-cli/internal/display" ) +func TestTailLogsCommand(t *testing.T) { + t.Run("it returns early with a message to generate logs when there are no logs", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + logsAPI := mock.NewMockLogAPI(ctrl) + logsAPI.EXPECT(). + List(gomock.Any()). + Return([]*management.Log{}, nil) + + expectedResult := `No logs available. + + ▸ To generate logs, run a test command like 'auth0 test login' or 'auth0 test token' +` + + stdout := &bytes.Buffer{} + cli := &cli{ + renderer: &display.Renderer{ + MessageWriter: stdout, + ResultWriter: io.Discard, + }, + api: &auth0.API{Log: logsAPI}, + } + + cmd := tailLogsCmd(cli) + cmd.SetArgs([]string{"--number", "9000", "--filter", "user_id:123"}) + err := cmd.Execute() + + assert.NoError(t, err) + assert.Equal(t, expectedResult, stdout.String()) + }) + + t.Run("it returns an error when it fails to get the logs on the first request", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + logsAPI := mock.NewMockLogAPI(ctrl) + logsAPI.EXPECT(). + List(gomock.Any()). + Return(nil, fmt.Errorf("generic error")) + + cli := &cli{ + api: &auth0.API{Log: logsAPI}, + } + + cmd := tailLogsCmd(cli) + cmd.SetArgs([]string{"--number", "9000", "--filter", "user_id:123"}) + err := cmd.Execute() + + assert.EqualError(t, err, "failed to get logs: generic error") + }) + + t.Run("it returns an error when it fails to get the logs on the 3rd request", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + logsAPI := mock.NewMockLogAPI(ctrl) + logsAPI.EXPECT(). + List(gomock.Any()). + Return( + []*management.Log{ + { + LogID: auth0.String("354234"), + Type: auth0.String("sapi"), + Description: auth0.String("Update branding settings"), + }, + }, + nil, + ) + + logsAPI.EXPECT(). + List(gomock.Any()). + Return( + []*management.Log{ + { + LogID: auth0.String("354234"), + Type: auth0.String("sapi"), + Description: auth0.String("Update branding settings"), + }, + { + LogID: auth0.String("354236"), + Type: auth0.String("sapi"), + Description: auth0.String("Update tenant settings"), + }, + }, + nil, + ) + + logsAPI.EXPECT(). + List(gomock.Any()). + Return(nil, fmt.Errorf("generic error")) + + expectedMessage := ` +=== auth0-cli-tests.eu.auth0.com logs + + ▸ Failed to get latest logs: generic error +` + expectedResult := `TYPE DESCRIPTION DATE CONNECTION CLIENT +API Operation Update branding settings Jan 01 00:00:00.000 N/A N/A +` + + message := &bytes.Buffer{} + result := &bytes.Buffer{} + cli := &cli{ + renderer: &display.Renderer{ + Tenant: "auth0-cli-tests.eu.auth0.com", + MessageWriter: message, + ResultWriter: result, + }, + api: &auth0.API{Log: logsAPI}, + } + + cmd := tailLogsCmd(cli) + cmd.SetArgs([]string{"--number", "9000", "--filter", "user_id:123"}) + err := cmd.Execute() + assert.NoError(t, err) + + assert.Equal(t, expectedMessage, message.String()) + assert.Equal(t, expectedResult, result.String()) + }) +} + func TestDedupeLogs(t *testing.T) { t.Run("removes duplicate logs and sorts by date asc", func(t *testing.T) { logs := []*management.Log{ From 2a6270d3de9592b4bb370ab49b06f44c0cd4f9e2 Mon Sep 17 00:00:00 2001 From: Sergiu Ghitea <28300158+sergiught@users.noreply.github.com> Date: Thu, 20 Apr 2023 14:16:38 +0200 Subject: [PATCH 2/5] Test improvements --- internal/cli/logs_test.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/internal/cli/logs_test.go b/internal/cli/logs_test.go index 06b3fe8b2..7ce0ae071 100644 --- a/internal/cli/logs_test.go +++ b/internal/cli/logs_test.go @@ -27,11 +27,6 @@ func TestTailLogsCommand(t *testing.T) { List(gomock.Any()). Return([]*management.Log{}, nil) - expectedResult := `No logs available. - - ▸ To generate logs, run a test command like 'auth0 test login' or 'auth0 test token' -` - stdout := &bytes.Buffer{} cli := &cli{ renderer: &display.Renderer{ @@ -42,11 +37,12 @@ func TestTailLogsCommand(t *testing.T) { } cmd := tailLogsCmd(cli) - cmd.SetArgs([]string{"--number", "9000", "--filter", "user_id:123"}) + cmd.SetArgs([]string{"--number", "90", "--filter", "user_id:123"}) err := cmd.Execute() assert.NoError(t, err) - assert.Equal(t, expectedResult, stdout.String()) + assert.Contains(t, stdout.String(), "No logs available.") + assert.Contains(t, stdout.String(), "To generate logs, run a test command like 'auth0 test login' or 'auth0 test token'") }) t.Run("it returns an error when it fails to get the logs on the first request", func(t *testing.T) { @@ -63,7 +59,7 @@ func TestTailLogsCommand(t *testing.T) { } cmd := tailLogsCmd(cli) - cmd.SetArgs([]string{"--number", "9000", "--filter", "user_id:123"}) + cmd.SetArgs([]string{"--number", "90", "--filter", "user_id:123"}) err := cmd.Execute() assert.EqualError(t, err, "failed to get logs: generic error") @@ -109,11 +105,6 @@ func TestTailLogsCommand(t *testing.T) { List(gomock.Any()). Return(nil, fmt.Errorf("generic error")) - expectedMessage := ` -=== auth0-cli-tests.eu.auth0.com logs - - ▸ Failed to get latest logs: generic error -` expectedResult := `TYPE DESCRIPTION DATE CONNECTION CLIENT API Operation Update branding settings Jan 01 00:00:00.000 N/A N/A ` @@ -130,11 +121,13 @@ API Operation Update branding settings } cmd := tailLogsCmd(cli) - cmd.SetArgs([]string{"--number", "9000", "--filter", "user_id:123"}) + cmd.SetArgs([]string{"--number", "90", "--filter", "user_id:123"}) err := cmd.Execute() assert.NoError(t, err) - assert.Equal(t, expectedMessage, message.String()) + assert.Contains(t, message.String(), "auth0-cli-tests.eu.auth0.com") // Ensure we display the tenant name. + assert.Contains(t, message.String(), "logs") // Ensure header is set in output. + assert.Contains(t, message.String(), "Failed to get latest logs: generic error") assert.Equal(t, expectedResult, result.String()) }) } From 5200afd8a5249333b81da4dcc76a2eca83c07c77 Mon Sep 17 00:00:00 2001 From: Sergiu Ghitea <28300158+sergiught@users.noreply.github.com> Date: Thu, 20 Apr 2023 14:17:03 +0200 Subject: [PATCH 3/5] Fix per page limit --- internal/cli/logs.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/cli/logs.go b/internal/cli/logs.go index 594a08e16..1d4dab323 100644 --- a/internal/cli/logs.go +++ b/internal/cli/logs.go @@ -9,9 +9,10 @@ import ( "github.com/spf13/cobra" ) -// Pagination max out at 1000 entries in total +// Besides the limitation of 100 log events per request to retrieve logs, +// we may only paginate through up to 1000 search results. // https://auth0.com/docs/logs/retrieve-log-events-using-mgmt-api#limitations -const logsPerPageLimit = 1000 +const logsPerPageLimit = 100 var ( logsFilter = Flag{ From 33562f5f4c647beb482d2fc7c4d4ee1c631bc766 Mon Sep 17 00:00:00 2001 From: Sergiu Ghitea <28300158+sergiught@users.noreply.github.com> Date: Thu, 20 Apr 2023 14:17:11 +0200 Subject: [PATCH 4/5] Fix concurrency issue --- internal/cli/logs.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/cli/logs.go b/internal/cli/logs.go index 1d4dab323..1880b4024 100644 --- a/internal/cli/logs.go +++ b/internal/cli/logs.go @@ -124,7 +124,7 @@ func tailLogsCmd(cli *cli) *cobra.Command { set := make(map[string]struct{}) list = dedupeLogs(list, set) - go func() { + go func(lastLogID string) { defer close(logsCh) for { @@ -139,7 +139,7 @@ func tailLogsCmd(cli *cli) *cobra.Command { queryParams = append(queryParams, management.Query(inputs.Filter)) } - list, err = cli.api.Log.List(queryParams...) + list, err := cli.api.Log.List(queryParams...) if err != nil { cli.renderer.Errorf("Failed to get latest logs: %v", err) return @@ -150,12 +150,12 @@ func tailLogsCmd(cli *cli) *cobra.Command { lastLogID = list[len(list)-1].GetLogID() } - if len(list) < 90 { + if len(list) < logsPerPageLimit { // Not a lot is happening, sleep on it. - time.Sleep(1 * time.Second) + time.Sleep(time.Second) } } - }() + }(lastLogID) cli.renderer.LogTail(list, logsCh, !cli.debug) return nil From b6dc2726959c039a718e949a2f99b51e1994b316 Mon Sep 17 00:00:00 2001 From: Sergiu Ghitea <28300158+sergiught@users.noreply.github.com> Date: Thu, 20 Apr 2023 16:24:54 +0200 Subject: [PATCH 5/5] Stop returning early when there are no logs --- internal/cli/logs.go | 16 ++++++++-------- internal/cli/logs_test.go | 28 ---------------------------- 2 files changed, 8 insertions(+), 36 deletions(-) diff --git a/internal/cli/logs.go b/internal/cli/logs.go index 1880b4024..2a1a98c79 100644 --- a/internal/cli/logs.go +++ b/internal/cli/logs.go @@ -110,15 +110,12 @@ func tailLogsCmd(cli *cli) *cobra.Command { return fmt.Errorf("failed to get logs: %w", err) } - if len(list) == 0 { - cli.renderer.EmptyState("logs") - cli.renderer.Infof("To generate logs, run a test command like 'auth0 test login' or 'auth0 test token'") - return nil - } - logsCh := make(chan []*management.Log) - lastLogID := list[len(list)-1].GetLogID() + var lastLogID string + if len(list) > 0 { + lastLogID = list[len(list)-1].GetLogID() + } // Create a `set` to detect duplicates clientside. set := make(map[string]struct{}) @@ -129,12 +126,15 @@ func tailLogsCmd(cli *cli) *cobra.Command { for { queryParams := []management.RequestOption{ - management.Query(fmt.Sprintf("log_id:[%s TO *]", lastLogID)), management.Parameter("page", "0"), management.Parameter("per_page", "100"), management.Parameter("sort", "date:-1"), } + if lastLogID != "" { + queryParams = append(queryParams, management.Query(fmt.Sprintf("log_id:[%s TO *]", lastLogID))) + } + if inputs.Filter != "" { queryParams = append(queryParams, management.Query(inputs.Filter)) } diff --git a/internal/cli/logs_test.go b/internal/cli/logs_test.go index 7ce0ae071..14a6a1b7f 100644 --- a/internal/cli/logs_test.go +++ b/internal/cli/logs_test.go @@ -3,7 +3,6 @@ package cli import ( "bytes" "fmt" - "io" "testing" "time" @@ -18,33 +17,6 @@ import ( ) func TestTailLogsCommand(t *testing.T) { - t.Run("it returns early with a message to generate logs when there are no logs", func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - logsAPI := mock.NewMockLogAPI(ctrl) - logsAPI.EXPECT(). - List(gomock.Any()). - Return([]*management.Log{}, nil) - - stdout := &bytes.Buffer{} - cli := &cli{ - renderer: &display.Renderer{ - MessageWriter: stdout, - ResultWriter: io.Discard, - }, - api: &auth0.API{Log: logsAPI}, - } - - cmd := tailLogsCmd(cli) - cmd.SetArgs([]string{"--number", "90", "--filter", "user_id:123"}) - err := cmd.Execute() - - assert.NoError(t, err) - assert.Contains(t, stdout.String(), "No logs available.") - assert.Contains(t, stdout.String(), "To generate logs, run a test command like 'auth0 test login' or 'auth0 test token'") - }) - t.Run("it returns an error when it fails to get the logs on the first request", func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish()