Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/filelog] Implement ExcludeOlderThan matcher criterion #31916

Merged

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Mar 22, 2024

Description:
This PR implements a new matcher criterion in the Stanza fileconsumer matcher:

ExcludeOlderThan time.Duration        `mapstructure:"exclude_older_than"`

and the corresponding setting in the filelog receiver configuration:

Field Default Description
exclude_older_than Exclude files whose modification time is older than the specified age.

When specified, the matcher will exclude files whose modification times are older than the specified time.

Link to tracking Issue: #31053

Testing: Added unit tests.

Documentation: Documented exclude_older_than configuration setting in the filelogreceiver's README.

pkg/stanza/fileconsumer/matcher/matcher.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/matcher.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/matcher.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/matcher.go Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/matcher.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/internal/filter/sort.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/internal/filter/sort.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/matcher/internal/filter/sort.go Outdated Show resolved Hide resolved
@djaglowski
Copy link
Member

Please address CI failures

@ycombinator
Copy link
Contributor Author

Ugh, I had a bad merge which resulted in a lot of files being changed and lots of labels and reviewers being added to this PR. I've sinced fixed the commits but the labels and reviewers remain. Sorry about that. 😞

@djaglowski should I close this PR and open a new one or do you want to remove the unrelated labels and reviewers from this PR?

@lokesh-balla
Copy link

@ycombinator

I have a use case which is similar to this where i want to do

  • more than n files are modified in last x amount of time

    • we get the last modified n files
  • less than n files are modified in last x amount of time

    • we will only get the files modified in x amount of time (which is less that limit n)

If your change gets merged, and we set both

    exclude_older_than: 1h
    ordering_criteria:
      top_n: 3
      sort_by:
        - sort_type: mtime

will it work as described above?

Comment on lines +45 to +46
fileMTimes: []time.Time{twoHoursAgo, threeHoursAgo},
excludeOlderThan: 3 * time.Hour,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a followup PR but I think the test is set up a bit strange here because we're comparing "exactly 3 hours" to "3 hour + trivial time" (because we initialize threeHoursAgo above and compare it just a few cycles later.) I think we should instead make this an unambiguous comparison and consider ways to test the exactly equal case.

@djaglowski djaglowski merged commit 7579382 into open-telemetry:main Apr 15, 2024
170 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 15, 2024
ghost pushed a commit to opsramp/opentelemetry-collector-contrib that referenced this pull request May 5, 2024
…en-telemetry#31916)

**Description:**
This PR implements a new matcher criterion in the Stanza fileconsumer
matcher:

```
ExcludeOlderThan time.Duration        `mapstructure:"exclude_older_than"`
```

and the corresponding setting in the `filelog` receiver configuration:

| Field | Default | Description |

|-------------------------------------|--------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `exclude_older_than` | | Exclude files whose modification time is
older than the specified age. |

When specified, the matcher will exclude files whose modification times
are older than the specified time.

**Link to tracking Issue:** open-telemetry#31053

**Testing:** Added unit tests.

**Documentation:** Documented `exclude_older_than` configuration setting
in the `filelogreceiver`'s README.

---------

Co-authored-by: Daniel Jaglowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants