Remove `logging.files.suffix` option and always use datetime suffixes by kvch · Pull Request #28927 · elastic/beats

kvch · 2021-11-11T08:57:05Z

What does this PR do?

This PR contains several improvements to the output file log rotation:

Removes suffix option as discussed offline
Filenames end in extension name so users get a hint about the contents
Removes time.Sleep from testing code
Datetime format no longer contains hour and minutes. In case of conflict on rotation, an index is appended to the filename.

Why is it important?

Previously, log file suffixes were configurable. Users had the option to either add the count or the datetime to the end of the log file. From now on, we only allow datetime based naming. Hence the configuration option logging.files.suffix is removed.

Example:
Filebeat is writing logs to filebeat-20211111.ndjson actively. Then a few minutes later it gets rotated, and the new active file becomes filebeat-20211111-1.ndjson.

This change should help with Beats not being able to rotate files on Windows.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

mergify · 2021-11-11T08:57:22Z

This pull request does not have a backport label. Could you fix it @kvch? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2021-11-11T09:18:54Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-11-19T14:17:48.142+0000
Duration: 159 min 54 sec
Commit: 2ff01b3

Test stats 🧪

Test	Results
Failed	0
Passed	48922
Skipped	4295
Total	53217

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

botelastic · 2021-11-16T16:31:50Z

This pull request doesn't have a Team:<team> label.

elasticmachine · 2021-11-16T16:56:28Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

faec

This looks really nice!

faec · 2021-11-16T21:49:27Z

libbeat/common/file/rotator.go

 		interval:        0,
 		rotateOnStartup: true,
-		suffix:          SuffixCount,
+		clock:           &realClock{},


injectible clock makes me happy :-)

…g-suffix-type

kvch · 2021-11-19T16:41:49Z

/package

…#28927) ## What does this PR do? This PR contains several improvements to the output file log rotation: * Removes suffix option as discussed offline * Filenames end in extension name so users get a hint about the contents * Removes `time.Sleep` from testing code * Datetime format no longer contains hour and minutes. In case of conflict on rotation, an index is appended to the filename. ## Why is it important? Previously, log file suffixes were configurable. Users had the option to either add the count or the datetime to the end of the log file. From now on, we only allow datetime based naming. Hence the configuration option `logging.files.suffix` is removed. Example: Filebeat is writing logs to `filebeat-20211111.ndjson` actively. Then a few minutes later it gets rotated, and the new active file becomes `filebeat-20211111-1.ndjson`. This change should help with Beats not being able to rotate files on Windows. (cherry picked from commit f2ae281)

…#28927) (#29065) ## What does this PR do? This PR contains several improvements to the output file log rotation: * Removes suffix option as discussed offline * Filenames end in extension name so users get a hint about the contents * Removes `time.Sleep` from testing code * Datetime format no longer contains hour and minutes. In case of conflict on rotation, an index is appended to the filename. ## Why is it important? Previously, log file suffixes were configurable. Users had the option to either add the count or the datetime to the end of the log file. From now on, we only allow datetime based naming. Hence the configuration option `logging.files.suffix` is removed. Example: Filebeat is writing logs to `filebeat-20211111.ndjson` actively. Then a few minutes later it gets rotated, and the new active file becomes `filebeat-20211111-1.ndjson`. This change should help with Beats not being able to rotate files on Windows. (cherry picked from commit f2ae281) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

…ws-on-file-changes * upstream/master: Fix discovery of Nomad allocations (elastic#28700) Add null (`\u0000`) as a valid line terminator (elastic#28998) Remove `logging.files.suffix` option and always use datetime suffixes (elastic#28927) x-pack/filebeat/module: add note for default var.input (elastic#28324) Fix AccessList & AccessMask processing in security data_stream (elastic#29016) [Metricbeat] Fix wrong mapping on "info" subkey (elastic#28782) ci: daily/weekly jobs (elastic#29050) [mergify] report open backported PRs once a week (elastic#28964)

andrewkroh · 2021-12-15T23:02:25Z

@kvch The docs and config for the file output appear to not match the new behavior:

beats/libbeat/_meta/config/output-file.reference.yml.tmpl

Lines 18 to 20 in 3319f5b

    
           # Name of the generated files. The default is `{{.BeatName}}` and it generates 
        
           # files: `{{.BeatName}}`, `{{.BeatName}}.1`, `{{.BeatName}}.2`, etc. 
        
           #filename: {{.BeatName}}

beats/libbeat/outputs/fileout/docs/fileout.asciidoc

Lines 45 to 48 in 3319f5b

    
           ===== `filename` 
        
           The name of the generated files. The default is set to the Beat name. For example, the files 
        
           generated by default for {beatname_uc} would be "{beatname_lc}", "{beatname_lc}.1", "{beatname_lc}.2", and so on.

kvch · 2021-12-17T15:26:44Z

Yes, it has been on my TODO list. I've just opened PR: #29501

* Follow up changes in the docs for logging * add reference configuration

* Follow up changes in the docs for logging * add reference configuration (cherry picked from commit 391a948)

* Follow up changes in the docs for logging * add reference configuration (cherry picked from commit 391a948) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

This fixes the tests for beats and apm-server when checking for the output files. Relates to changes from elastic/beats#28927

This fixes the tests for beats when checking for the output files. Relates to changes from elastic/beats#28927 Add output ndjson suffix to APM Server output (unclear why this is necessary). * Increase timeout for output file and ensure it is written. The output file is only written when some actual values are processed. Enable self-instrumentation for APM to ensure the output file is written and increase timeout. For beats, enable metrics logging explicitly.

This fixes the tests for beats when checking for the output files. Relates to changes from elastic/beats#28927 Add output ndjson suffix to APM Server output (unclear why this is necessary). * Increase timeout for output file and ensure it is written. The output file is only written when some actual values are processed. Enable self-instrumentation for APM to ensure the output file is written and increase timeout. For beats, enable metrics logging explicitly. * Disable failing beats issues A follow up is necessary to fix and enable these tests if still useful.

kvch added 4 commits November 8, 2021 13:46

Initialize changes

fa6207d

init more

3d412ce

init

cd24959

pass clock to more places

708ad0d

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 11, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 11, 2021

kvch added 4 commits November 16, 2021 16:41

more follow up

9c18b62

more

66e6f06

less changes

cf43d55

cleanup

c22f413

kvch added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Nov 16, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 16, 2021

kvch added backport-v8.0.0 Automated backport with mergify needs_team Indicates that the issue/PR needs a Team:* label labels Nov 16, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 16, 2021

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Nov 16, 2021

kvch added 2 commits November 16, 2021 17:45

update config and docs

99c6f16

add changelog entry

6b8bb79

kvch changed the title ~~Remove libbeat log suffix type~~ Remove logging.files.suffix option and always use datetime suffixes Nov 16, 2021

kvch requested a review from faec November 16, 2021 16:56

kvch marked this pull request as ready for review November 16, 2021 16:56

kvch requested review from a team as code owners November 16, 2021 16:56

faec approved these changes Nov 16, 2021

View reviewed changes

kvch added 2 commits November 17, 2021 13:08

Merge remote-tracking branch 'upstream/master' into remove-libbeat-lo…

2399f45

…g-suffix-type

follow up

724275f

kvch added 8 commits November 17, 2021 15:42

more more

b117295

more

81b1ce8

add missing attribute

4124eff

adjust paths for test checks

038061b

more follow up

3d087d4

more test fixes

dd9b89e

final fixes

d68d4d5

adjust more things again

2ff01b3

kvch merged commit f2ae281 into elastic:master Nov 22, 2021

mergify bot mentioned this pull request Nov 22, 2021

[8.0](backport #28927) Remove logging.files.suffix option and always use datetime suffixes #29065

Merged

This was referenced Nov 23, 2021

Skip config check in autodiscover for duplicated configurations #29048

Merged

Get output files when they have a suffix elastic/e2e-testing#1829

Merged

jsoriano mentioned this pull request Dec 3, 2021

chore: remove assertions related to output files elastic/beats-tester#209

Closed

belimawr pushed a commit that referenced this pull request Dec 22, 2021

Follow up changes in documentation after #28927 (#29501)

391a948

* Follow up changes in the docs for logging * add reference configuration

mergify bot pushed a commit that referenced this pull request Dec 22, 2021

Follow up changes in documentation after #28927 (#29501)

729e166

* Follow up changes in the docs for logging * add reference configuration (cherry picked from commit 391a948)

simitt added a commit to simitt/beats-tester that referenced this pull request Dec 31, 2021

Adapt output file name to latest beats changes.

460db71

This fixes the tests for beats and apm-server when checking for the output files. Relates to changes from elastic/beats#28927

simitt mentioned this pull request Dec 31, 2021

beats-tester jobs are failing for beats and apm-server due to changes in output.file handling in beats elastic/beats-tester#211

Closed

This was referenced Jan 26, 2022

Update docker image to Ubuntu 20.04 elastic/apm-server#7138

Closed

8.0 update beats elastic/apm-server#7137

Merged

kvch mentioned this pull request Jun 7, 2022

Beats log files should be generated with timestamped filenames #24572

Closed

andrewkroh mentioned this pull request Feb 15, 2023

remove apm elastic/beats-tester#219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `logging.files.suffix` option and always use datetime suffixes#28927

Remove `logging.files.suffix` option and always use datetime suffixes#28927
kvch merged 20 commits intoelastic:masterfrom
kvch:remove-libbeat-log-suffix-type

kvch commented Nov 11, 2021 •

edited

Loading

Uh oh!

mergify bot commented Nov 11, 2021

Uh oh!

elasticmachine commented Nov 11, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Uh oh!

botelastic bot commented Nov 16, 2021

Uh oh!

elasticmachine commented Nov 16, 2021

Uh oh!

faec left a comment

Uh oh!

faec Nov 16, 2021

Uh oh!

kvch commented Nov 19, 2021

Uh oh!

andrewkroh commented Dec 15, 2021

Uh oh!

kvch commented Dec 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kvch commented Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

Uh oh!

mergify bot commented Nov 11, 2021

Uh oh!

elasticmachine commented Nov 11, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Uh oh!

botelastic bot commented Nov 16, 2021

Uh oh!

elasticmachine commented Nov 16, 2021

Uh oh!

faec left a comment

Choose a reason for hiding this comment

Uh oh!

faec Nov 16, 2021

Choose a reason for hiding this comment

Uh oh!

kvch commented Nov 19, 2021

Uh oh!

andrewkroh commented Dec 15, 2021

Uh oh!

kvch commented Dec 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kvch commented Nov 11, 2021 •

edited

Loading

elasticmachine commented Nov 11, 2021 •

edited by jenkins-beats-ci bot

Loading