fix(inputs.tail): Retry opening file after permission denied #14357

ericmiller-wpengine · 2023-11-27T21:13:30Z

Required for all PRs

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

resolves #13154

Gather() calls tailNewFiles() and creates a tailer for any that are not in the map of tailers already. Deleting the old one from the map on "permission denied" was sufficient to get the plugin to reopen the file.

a lightly redacted section of logs. in another terminal i forced a logrotate that left the log file with the wrong permissions (0600) and then immediately chmod'ed to 0644, easier to catch than the race condition.

2023-11-27T21:58:37Z I! Loaded inputs: conntrack disk filestat http (3x) http_listener_v2 httpjson memcached mysql net_response (5x) nginx (2x) prometheus tail (3x)
2023-11-27T21:58:37Z I! Loaded aggregators:
2023-11-27T21:58:37Z I! Loaded processors: regex rename (2x)
2023-11-27T21:58:37Z I! Loaded secretstores:
2023-11-27T21:58:37Z I! Loaded outputs: file
2023-11-27T21:58:37Z I! Tags enabled: <some tags were here>
2023-11-27T21:58:37Z W! Deprecated inputs: 1 and 0 options
2023-11-27T21:58:37Z I! [agent] Config: Interval:30s, Quiet:false, Hostname:"fake", Flush Interval:1m0s
2023-11-27T21:58:37Z I! [inputs.prometheus] Using the label selector:  and field selector:
2023-11-27T21:58:37Z I! [inputs.http_listener_v2] Listening on [::]:8185
{"fields":{"account_name":"foobar",<more fields omitted>},"name":"nginxlog","tags":{"datacenter":"self-contained",<more tags omitted>},"timestamp":1701122545}
2023-11-27T22:03:31Z E! [inputs.tail] Tailing "/.../access.json": Unable to open file /.../access.json: open /.../access.json: permission denied
2023-11-27T22:03:31Z E! [inputs.tail] Deleting tailer for "/.../access.json"
{"fields":{"account_name":"foobar",<more fields omitted>},"name":"nginxlog","tags":{"datacenter":"self-contained",<more tags omitted>},"timestamp":1701122692}

telegraf-tiger · 2023-11-27T21:13:35Z

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

ericmiller-wpengine · 2023-11-27T21:16:09Z

!signed-cla

ericmiller-wpengine · 2023-11-27T22:37:39Z

Out of a concern the offsets would be affected and cause the wrong behavior, I did a further test:

with telegraf tailing a file, appended a line, saw it printed by outputs.file
forced logrotate with config that created new log file with 0600 perms, saw "permission denied"
appended 3 lines to new empty log file
chmod 0644 the log file, saw telegraf print all 3 new lines.

Hipska · 2023-11-28T08:15:51Z

What happens in the scenario the file becomes inaccessible, then few lines added, then accessible again?

ericmiller-wpengine · 2023-11-28T15:10:46Z

If the tail plugin has my log open, changing the permissions on the already-open file to something that would cause a problem for telegraf opening it later does not affect telegraf immediately. It already has the file open, and reads new lines written to the file.

srebhan

Thanks for the PR @ericmiller-wpengine! One question and one suggestion from my side...

plugins/inputs/tail/tail.go

srebhan · 2023-11-30T17:05:43Z

plugins/inputs/tail/tail.go

@@ -236,6 +236,10 @@ func (t *Tail) tailNewFiles(fromBeginning bool) error {

 				if err := tailer.Err(); err != nil {
 					t.Log.Errorf("Tailing %q: %s", tailer.Filename, err.Error())
+					if strings.HasSuffix(err.Error(), "permission denied") {
+						t.Log.Errorf("Deleting tailer for %q", tailer.Filename)


How about adding some details like

Suggested change

t.Log.Errorf("Deleting tailer for %q", tailer.Filename)

t.Log.Errorf("Deleting tailer for %q: %v", tailer.Filename, err)

I moved the previous log message into an else at the same time, hopefully this is adequately idiomatic.

ericmiller-wpengine · 2023-11-30T19:42:45Z

Now that I pushed again, there is a failed check but the failure looks like its in linting a README I did not touch. I looked but don't see somewhere I can trigger it again without another push.

Also some more checks failed after that, also in unrelated-looking ways.

srebhan

Thanks for tackling this issue @ericmiller-wpengine! Very much appreciated!

Hipska · 2023-11-30T21:14:04Z

@ericmiller-wpengine yeah something went wrong for multiple PRs today. Just merge latest master branch and it should be fine.

telegraf-tiger · 2023-11-30T22:03:18Z

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz

powersj

Thanks for coming up with this!

telegraf-tiger bot added area/tail feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Nov 27, 2023

ericmiller-wpengine force-pushed the tail-plugin-retry-file-open branch from 7e9224b to ed70d7e Compare November 27, 2023 21:19

ericmiller-wpengine changed the title ~~feat(inputs.tail): retry opening file after permission denied~~ feat(inputs.tail): Retry opening file after permission denied Nov 27, 2023

ericmiller-wpengine changed the title ~~feat(inputs.tail): Retry opening file after permission denied~~ fix(inputs.tail): Retry opening file after permission denied Nov 27, 2023

fix(inputs.tail): If permission denied delete old tailer

4b867c5

ericmiller-wpengine force-pushed the tail-plugin-retry-file-open branch from ed70d7e to 4b867c5 Compare November 27, 2023 21:24

dayvar14 mentioned this pull request Nov 27, 2023

Permission denied in a middle of logrotate process #13154

Closed

Hipska approved these changes Nov 28, 2023

View reviewed changes

Hipska added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 28, 2023

srebhan reviewed Nov 30, 2023

View reviewed changes

srebhan self-assigned this Nov 30, 2023

srebhan removed the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 30, 2023

Single log, containing error in either case

264219c

srebhan approved these changes Nov 30, 2023

View reviewed changes

srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 30, 2023

srebhan assigned powersj and unassigned srebhan Nov 30, 2023

Hipska approved these changes Nov 30, 2023

View reviewed changes

Merge branch 'influxdata:master' into tail-plugin-retry-file-open

7c227ac

powersj approved these changes Nov 30, 2023

View reviewed changes

powersj merged commit e4d7be0 into influxdata:master Nov 30, 2023
23 checks passed

github-actions bot added this to the v1.29.0 milestone Nov 30, 2023

Porkepix mentioned this pull request Dec 11, 2023

telegraf 1.29.0 Homebrew/homebrew-core#156969

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(inputs.tail): Retry opening file after permission denied #14357

fix(inputs.tail): Retry opening file after permission denied #14357

ericmiller-wpengine commented Nov 27, 2023 •

edited

Loading

telegraf-tiger bot commented Nov 27, 2023

ericmiller-wpengine commented Nov 27, 2023

ericmiller-wpengine commented Nov 27, 2023

Hipska commented Nov 28, 2023

ericmiller-wpengine commented Nov 28, 2023

srebhan left a comment

srebhan Nov 30, 2023

ericmiller-wpengine Nov 30, 2023

ericmiller-wpengine commented Nov 30, 2023

srebhan left a comment

Hipska commented Nov 30, 2023

telegraf-tiger bot commented Nov 30, 2023

Artifact URLs

powersj left a comment

	t.Log.Errorf("Deleting tailer for %q", tailer.Filename)
	t.Log.Errorf("Deleting tailer for %q: %v", tailer.Filename, err)

fix(inputs.tail): Retry opening file after permission denied #14357

fix(inputs.tail): Retry opening file after permission denied #14357

Conversation

ericmiller-wpengine commented Nov 27, 2023 • edited Loading

Required for all PRs

telegraf-tiger bot commented Nov 27, 2023

ericmiller-wpengine commented Nov 27, 2023

ericmiller-wpengine commented Nov 27, 2023

Hipska commented Nov 28, 2023

ericmiller-wpengine commented Nov 28, 2023

srebhan left a comment

Choose a reason for hiding this comment

srebhan Nov 30, 2023

Choose a reason for hiding this comment

ericmiller-wpengine Nov 30, 2023

Choose a reason for hiding this comment

ericmiller-wpengine commented Nov 30, 2023

srebhan left a comment

Choose a reason for hiding this comment

Hipska commented Nov 30, 2023

telegraf-tiger bot commented Nov 30, 2023

Artifact URLs

powersj left a comment

Choose a reason for hiding this comment

ericmiller-wpengine commented Nov 27, 2023 •

edited

Loading