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

fluentd log driver. failed parse last partial message in fluentd #38951 #38952

Merged
merged 1 commit into from
Apr 15, 2019

Conversation

alexei38
Copy link

I use fluentd logging driver
In my containers the log it is more than 16 kb
Such the log are marked with partial_message flag

fluent-plugin-concat waited for the message which came without partial_message from stopped to stick together the message for further transfer

Signed-off-by: Alexei Margasov alexei38@yandex.ru

@thaJeztah
Copy link
Member

ping @anusha-ragunathan ptal

@anusha-ragunathan
Copy link
Contributor

anusha-ragunathan commented Mar 27, 2019

FYI, you can also check for msg.PLogMetaData.Last to be true to start assembling the partial messages together.

The idea of this PR LGTM. The last message in a series of partial messages should not be flagged as Partial

@@ -163,7 +163,7 @@ func (f *fluentd) Log(msg *logger.Message) error {
for k, v := range f.extra {
data[k] = v
}
if msg.PLogMetaData != nil {
if msg.PLogMetaData != nil && !msg.PLogMetaData.Last {
data["partial_message"] = "true"
Copy link
Contributor

Choose a reason for hiding this comment

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

While we are here, lets also add the missing fields in the data map.

data["partial_ordinal"] = strconv.Itoa(msg.PLogMetaData.Ordinal)
data["partial_last"] = strconv.FormatBool(msg.PLogMetaData.Last)```

Copy link
Contributor

Choose a reason for hiding this comment

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

This is already taken care of by https://github.com/moby/moby/pull/38065/files, but CI is broke on that. You can carry over this specific change from that PR.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, well, this approach is much better

@anusha-ragunathan
Copy link
Contributor

experimental failed with unrelated test failure. Is this a flaky test @thaJeztah ?

03:57:51 --- FAIL: TestCheckpoint (1.40s)
03:57:51     checkpoint_test.go:40: Error (criu/util.c:816): exited, status=3
03:57:51         Warn  (criu/net.c:2840): Unable to get socket network namespace
03:57:51         Warn  (criu/net.c:2840): Unable to get tun network namespace
03:57:51         Warn  (criu/sk-unix.c:229): unix: Unable to open a socket file: Bad address
03:57:51         Warn  (criu/net.c:2840): Unable to get socket network namespace
03:57:51         Warn  (criu/kerndat.c:881): Can't keep kdat cache on non-tempfs
03:57:51         Looks good.
03:57:51     checkpoint_test.go:51: Start a container
03:57:51     checkpoint_test.go:69: ++ type -P true
03:57:51         ++ type -P ip6tables-restore
03:57:51         + mount --bind /bin/true /sbin/ip6tables-restore
03:57:51         ++ type -P true
03:57:51         ++ type -P ip6tables-save
03:57:51         + mount --bind /bin/true /sbin/ip6tables-save
03:57:51     checkpoint_test.go:81: Do a checkpoint and leave the container running
03:57:51     checkpoint_test.go:24: Exec: [touch /tmp/test-file]
03:57:51     checkpoint_test.go:28: 
03:57:51     checkpoint_test.go:115: Do a checkpoint and stop the container
03:57:51     checkpoint_test.go:117: assertion failed: error is not nil: Error response from daemon: Cannot checkpoint container 0ab3c1493bdd6aacbadd15ce20b338357cfdf98cfc3cfff325ef0fe065050491: cannot checkpoint a stopped container: unknown
03:57:51     checkpoint_test.go:77: ++ type -P ip6tables-restore
03:57:51         + umount -c -i -l /sbin/ip6tables-restore
03:57:51         ++ type -P ip6tables-save
03:57:51         + umount -c -i -l /sbin/ip6tables-save
03:57:51     main_test.go:32: assertion failed: error is not nil: Error response from daemon: Container 0ab3c1493bdd6aacbadd15ce20b338357cfdf98cfc3cfff325ef0fe065050491 is not paused: failed to unpause container 0ab3c1493bdd6aacbadd15ce20b338357cfdf98cfc3cfff325ef0fe065050491

@thaJeztah
Copy link
Member

doesn't look related no; retriggered ci

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

Merging #38952 into master will increase coverage by 0.42%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #38952      +/-   ##
==========================================
+ Coverage   36.89%   37.32%   +0.42%     
==========================================
  Files         612      612              
  Lines       45332    45846     +514     
==========================================
+ Hits        16727    17113     +386     
- Misses      26319    26393      +74     
- Partials     2286     2340      +54

…y#38951

Signed-off-by: Alexei Margasov <alexei38@yandex.ru>
@alexei38
Copy link
Author

My commit is not related to build issues.
How can I restart the build?

@alexei38
Copy link
Author

ping @cpuguy83 @anusha-ragunathan ptal

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

ping @anusha-ragunathan ptal

Copy link
Contributor

@anusha-ragunathan anusha-ragunathan left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit c524e15 into moby:master Apr 15, 2019
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.

None yet

5 participants