From 1fb3c14937ccb0abd4c89f6fea9f457801433874 Mon Sep 17 00:00:00 2001 From: Shubham Rasal Date: Mon, 12 Feb 2024 15:18:04 +0530 Subject: [PATCH 1/3] feat: allow logging PageSize to override --- logging/logadmin/logadmin.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/logging/logadmin/logadmin.go b/logging/logadmin/logadmin.go index 1ec2b4c22e43..cdbc4ae4e69f 100644 --- a/logging/logadmin/logadmin.go +++ b/logging/logadmin/logadmin.go @@ -223,6 +223,13 @@ type newestFirst struct{} func (newestFirst) set(r *logpb.ListLogEntriesRequest) { r.OrderBy = "timestamp desc" } +// PageSize provide a way to override number of results to return from each request. Default is 50 +func PageSize(p int32) EntriesOption { return pageSize(p) } + +type pageSize int32 + +func (p pageSize) set(r *logpb.ListLogEntriesRequest) { r.PageSize = int32(p) } + // Entries returns an EntryIterator for iterating over log entries. By default, // the log entries will be restricted to those from the project passed to // NewClient. This may be overridden by passing a ProjectIDs option. Requires ReadScope or AdminScope. From 269c1de59203d753ff28763602a26cc8d9a21d3e Mon Sep 17 00:00:00 2001 From: Shubham Rasal Date: Fri, 16 Feb 2024 11:31:50 +0530 Subject: [PATCH 2/3] add test for pageSize set --- logging/logadmin/logadmin.go | 2 +- logging/logadmin/logadmin_test.go | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/logging/logadmin/logadmin.go b/logging/logadmin/logadmin.go index cdbc4ae4e69f..c32e831e4a7f 100644 --- a/logging/logadmin/logadmin.go +++ b/logging/logadmin/logadmin.go @@ -223,7 +223,7 @@ type newestFirst struct{} func (newestFirst) set(r *logpb.ListLogEntriesRequest) { r.OrderBy = "timestamp desc" } -// PageSize provide a way to override number of results to return from each request. Default is 50 +// PageSize provide a way to override number of results to return from each request. func PageSize(p int32) EntriesOption { return pageSize(p) } type pageSize int32 diff --git a/logging/logadmin/logadmin_test.go b/logging/logadmin/logadmin_test.go index 2ca4a6c691ea..16acdfac99f4 100644 --- a/logging/logadmin/logadmin_test.go +++ b/logging/logadmin/logadmin_test.go @@ -288,31 +288,35 @@ func TestListLogEntriesRequest(t *testing.T) { resourceNames []string filterPrefix string orderBy string + pageSize int32 }{ // Timestamp default does not override user's filter {[]EntriesOption{NewestFirst(), Filter(`timestamp > "2020-10-30T15:39:09Z"`)}, - []string{"projects/PROJECT_ID"}, `timestamp > "2020-10-30T15:39:09Z"`, "timestamp desc"}, + []string{"projects/PROJECT_ID"}, `timestamp > "2020-10-30T15:39:09Z"`, "timestamp desc", 0}, {[]EntriesOption{NewestFirst(), Filter("f")}, - []string{"projects/PROJECT_ID"}, "f AND timestamp >= \"", "timestamp desc"}, + []string{"projects/PROJECT_ID"}, "f AND timestamp >= \"", "timestamp desc", 0}, {[]EntriesOption{ProjectIDs([]string{"foo"})}, - []string{"projects/foo"}, "timestamp >= \"", ""}, + []string{"projects/foo"}, "timestamp >= \"", "", 0}, {[]EntriesOption{ResourceNames([]string{"folders/F", "organizations/O"})}, - []string{"folders/F", "organizations/O"}, "timestamp >= \"", ""}, + []string{"folders/F", "organizations/O"}, "timestamp >= \"", "", 0}, {[]EntriesOption{NewestFirst(), Filter("f"), ProjectIDs([]string{"foo"})}, - []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc"}, + []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc", 0}, {[]EntriesOption{NewestFirst(), Filter("f"), ProjectIDs([]string{"foo"})}, - []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc"}, + []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc", 0}, // If there are repeats, last one wins. {[]EntriesOption{NewestFirst(), Filter("no"), ProjectIDs([]string{"foo"}), Filter("f")}, - []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc"}, + []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc", 0}, + {[]EntriesOption{ProjectIDs([]string{"foo"}), PageSize(100)}, + []string{"projects/foo"}, "timestamp >= \"", "", 100}, } { got := listLogEntriesRequest("projects/PROJECT_ID", test.opts) want := &logpb.ListLogEntriesRequest{ ResourceNames: test.resourceNames, Filter: test.filterPrefix, OrderBy: test.orderBy, + PageSize: test.pageSize, } - if !testutil.Equal(got.ResourceNames, want.ResourceNames) || !strings.HasPrefix(got.Filter, want.Filter) || got.OrderBy != want.OrderBy { + if !testutil.Equal(got.ResourceNames, want.ResourceNames) || !strings.HasPrefix(got.Filter, want.Filter) || got.OrderBy != want.OrderBy || got.PageSize != want.PageSize { t.Errorf("got: %v; want %v (mind wanted Filter is prefix)", got, want) } } From 16d2f3266ced12f8d74db5746d0e1438b50fa872 Mon Sep 17 00:00:00 2001 From: shubhamrasal Date: Thu, 29 Feb 2024 19:30:46 +0530 Subject: [PATCH 3/3] make test cases more readable - remove duplicated test case --- logging/logadmin/logadmin_test.go | 88 +++++++++++++++++++++++++------ 1 file changed, 71 insertions(+), 17 deletions(-) diff --git a/logging/logadmin/logadmin_test.go b/logging/logadmin/logadmin_test.go index 16acdfac99f4..d63b2600fe49 100644 --- a/logging/logadmin/logadmin_test.go +++ b/logging/logadmin/logadmin_test.go @@ -291,23 +291,77 @@ func TestListLogEntriesRequest(t *testing.T) { pageSize int32 }{ // Timestamp default does not override user's filter - {[]EntriesOption{NewestFirst(), Filter(`timestamp > "2020-10-30T15:39:09Z"`)}, - []string{"projects/PROJECT_ID"}, `timestamp > "2020-10-30T15:39:09Z"`, "timestamp desc", 0}, - {[]EntriesOption{NewestFirst(), Filter("f")}, - []string{"projects/PROJECT_ID"}, "f AND timestamp >= \"", "timestamp desc", 0}, - {[]EntriesOption{ProjectIDs([]string{"foo"})}, - []string{"projects/foo"}, "timestamp >= \"", "", 0}, - {[]EntriesOption{ResourceNames([]string{"folders/F", "organizations/O"})}, - []string{"folders/F", "organizations/O"}, "timestamp >= \"", "", 0}, - {[]EntriesOption{NewestFirst(), Filter("f"), ProjectIDs([]string{"foo"})}, - []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc", 0}, - {[]EntriesOption{NewestFirst(), Filter("f"), ProjectIDs([]string{"foo"})}, - []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc", 0}, - // If there are repeats, last one wins. - {[]EntriesOption{NewestFirst(), Filter("no"), ProjectIDs([]string{"foo"}), Filter("f")}, - []string{"projects/foo"}, "f AND timestamp >= \"", "timestamp desc", 0}, - {[]EntriesOption{ProjectIDs([]string{"foo"}), PageSize(100)}, - []string{"projects/foo"}, "timestamp >= \"", "", 100}, + { + // default resource name and timestamp filter + opts: []EntriesOption{ + NewestFirst(), + Filter(`timestamp > "2020-10-30T15:39:09Z"`), + }, + resourceNames: []string{"projects/PROJECT_ID"}, + filterPrefix: `timestamp > "2020-10-30T15:39:09Z"`, + orderBy: "timestamp desc", + }, + { + // default resource name and user's filter + opts: []EntriesOption{ + NewestFirst(), + Filter("f"), + }, + resourceNames: []string{"projects/PROJECT_ID"}, + filterPrefix: "f AND timestamp >= \"", + orderBy: "timestamp desc", + }, + { + // user's project id and default timestamp filter + opts: []EntriesOption{ + ProjectIDs([]string{"foo"}), + }, + resourceNames: []string{"projects/foo"}, + filterPrefix: "timestamp >= \"", + orderBy: "", + }, + { + // user's resource name and default timestamp filter + opts: []EntriesOption{ + ResourceNames([]string{"folders/F", "organizations/O"}), + }, + resourceNames: []string{"folders/F", "organizations/O"}, + filterPrefix: "timestamp >= \"", + orderBy: "", + }, + { + // user's project id and user's options + opts: []EntriesOption{ + NewestFirst(), + Filter("f"), + ProjectIDs([]string{"foo"}), + }, + resourceNames: []string{"projects/foo"}, + filterPrefix: "f AND timestamp >= \"", + orderBy: "timestamp desc", + }, + { + // user's project id with multiple filter options + opts: []EntriesOption{ + NewestFirst(), + Filter("no"), + ProjectIDs([]string{"foo"}), + Filter("f"), + }, + resourceNames: []string{"projects/foo"}, + filterPrefix: "f AND timestamp >= \"", + orderBy: "timestamp desc", + }, + { + // user's project id and custom page size + opts: []EntriesOption{ + ProjectIDs([]string{"foo"}), + PageSize(100), + }, + resourceNames: []string{"projects/foo"}, + filterPrefix: "timestamp >= \"", + pageSize: 100, + }, } { got := listLogEntriesRequest("projects/PROJECT_ID", test.opts) want := &logpb.ListLogEntriesRequest{