From 050bb11e3302840480e57ec734e9033e7398683a Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Thu, 2 May 2024 11:35:15 -0400 Subject: [PATCH] [pkg/stanza] Take into account the `ascending` option when sorting by mtime (#32820) **Description:** * Allow ascending sort when sorting by mtime **Link to tracking Issue:** Closes #32792 **Testing:** * Unit tests --- .chloggen/feat_mtime-ascending-sort.yaml | 13 +++++++++ .../matcher/internal/filter/sort.go | 28 ++++++++++++++----- .../matcher/internal/filter/sort_test.go | 17 ++++++++++- pkg/stanza/fileconsumer/matcher/matcher.go | 2 +- 4 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 .chloggen/feat_mtime-ascending-sort.yaml diff --git a/.chloggen/feat_mtime-ascending-sort.yaml b/.chloggen/feat_mtime-ascending-sort.yaml new file mode 100644 index 0000000000000..16a0bd869f3fe --- /dev/null +++ b/.chloggen/feat_mtime-ascending-sort.yaml @@ -0,0 +1,13 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: "pkg/stanza" + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Allow sorting by ascending order when using the mtime sort_type. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [32792] diff --git a/pkg/stanza/fileconsumer/matcher/internal/filter/sort.go b/pkg/stanza/fileconsumer/matcher/internal/filter/sort.go index a8da985b5b70a..2ca233bd38b4e 100644 --- a/pkg/stanza/fileconsumer/matcher/internal/filter/sort.go +++ b/pkg/stanza/fileconsumer/matcher/internal/filter/sort.go @@ -142,7 +142,9 @@ func (t TopNOption) apply(items []*item) ([]*item, error) { return items[:t], nil } -type mtimeSortOption struct{} +type mtimeSortOption struct { + ascending bool +} type mtimeItem struct { mtime time.Time @@ -168,10 +170,20 @@ func (m mtimeSortOption) apply(items []*item) ([]*item, error) { }) } - sort.SliceStable(mtimeItems, func(i, j int) bool { - // This checks if item i > j, in order to reverse the sort (most recently modified file is first in the list) - return mtimeItems[i].mtime.After(mtimeItems[j].mtime) - }) + var lessFunc func(i, j int) bool + if m.ascending { + lessFunc = func(i, j int) bool { + // This checks if item i < j + return mtimeItems[i].mtime.Before(mtimeItems[j].mtime) + } + } else { + lessFunc = func(i, j int) bool { + // This checks if item i > j, in order to reverse the sort (most recently modified file is first in the list) + return mtimeItems[i].mtime.After(mtimeItems[j].mtime) + } + } + + sort.SliceStable(mtimeItems, lessFunc) filteredValues := make([]*item, 0, len(items)) for _, mtimeItem := range mtimeItems { @@ -181,6 +193,8 @@ func (m mtimeSortOption) apply(items []*item) ([]*item, error) { return filteredValues, errs } -func SortMtime() Option { - return mtimeSortOption{} +func SortMtime(ascending bool) Option { + return mtimeSortOption{ + ascending: ascending, + } } diff --git a/pkg/stanza/fileconsumer/matcher/internal/filter/sort_test.go b/pkg/stanza/fileconsumer/matcher/internal/filter/sort_test.go index 9257d8b12c182..e5f0b89c1cafd 100644 --- a/pkg/stanza/fileconsumer/matcher/internal/filter/sort_test.go +++ b/pkg/stanza/fileconsumer/matcher/internal/filter/sort_test.go @@ -179,6 +179,7 @@ func TestMTimeFilter(t *testing.T) { name string files []string fileMTimes []time.Time + ascending bool expectedErr string expect []string }{ @@ -200,6 +201,20 @@ func TestMTimeFilter(t *testing.T) { fileMTimes: []time.Time{epoch, epoch.Add(time.Hour)}, expect: []string{"b.log", "a.log"}, }, + { + name: "Multiple files in order, ascending sort", + files: []string{"a.log", "b.log"}, + fileMTimes: []time.Time{epoch, epoch.Add(time.Hour)}, + ascending: true, + expect: []string{"a.log", "b.log"}, + }, + { + name: "Multiple files out of order, ascending sort", + files: []string{"a.log", "b.log"}, + fileMTimes: []time.Time{epoch.Add(time.Hour), epoch}, + ascending: true, + expect: []string{"b.log", "a.log"}, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -221,7 +236,7 @@ func TestMTimeFilter(t *testing.T) { items = append(items, it) } - f := SortMtime() + f := SortMtime(tc.ascending) result, err := f.apply(items) if tc.expectedErr != "" { require.EqualError(t, err, tc.expectedErr) diff --git a/pkg/stanza/fileconsumer/matcher/matcher.go b/pkg/stanza/fileconsumer/matcher/matcher.go index adcf42da6909e..948f18852a98b 100644 --- a/pkg/stanza/fileconsumer/matcher/matcher.go +++ b/pkg/stanza/fileconsumer/matcher/matcher.go @@ -131,7 +131,7 @@ func New(c Criteria) (*Matcher, error) { if !mtimeSortTypeFeatureGate.IsEnabled() { return nil, fmt.Errorf("the %q feature gate must be enabled to use %q sort type", mtimeSortTypeFeatureGate.ID(), sortTypeMtime) } - m.filterOpts = append(m.filterOpts, filter.SortMtime()) + m.filterOpts = append(m.filterOpts, filter.SortMtime(sc.Ascending)) default: return nil, fmt.Errorf("'sort_type' must be specified") }