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

enhance: add new formatter for syslog #1608

Merged
merged 1 commit into from
Jul 5, 2018
Merged

enhance: add new formatter for syslog #1608

merged 1 commit into from
Jul 5, 2018

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented Jul 1, 2018

Ⅰ. Describe what this PR did

A lot of log collectors consumes log data and indexs it by the timestamp in millisecond format. In order to make the collector to consume data in the order, we need to add the sequence number in the log. Simplify the case, PouchContainer will uses the nano timestamp as sequene number.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how you did it

Add one more syslog formatter

Ⅳ. Describe how to verify it

First, we should follow the /var/log/syslog and run pouch run --log-driver syslog --log-opt syslog-format=rfc5424micro-seq --log-opt tag={{.DaemonName}} busybox echo hi.

We will got the message in the syslog like:

Jul  2 15:41:02 ubuntu-xenial 1 1530517262291559773 2018-07-02T15:41:02.291559+08:00 ubuntu-xenial pouchd 7335 pouchd - hi

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jul 1, 2018

Codecov Report

Merging #1608 into master will decrease coverage by <.01%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1608      +/-   ##
==========================================
- Coverage   41.99%   41.99%   -0.01%     
==========================================
  Files         270      270              
  Lines       17597    17607      +10     
==========================================
+ Hits         7390     7394       +4     
- Misses       9298     9304       +6     
  Partials      909      909
Impacted Files Coverage Δ
daemon/logger/syslog/fmt.go 0% <0%> (ø) ⬆️
daemon/logger/syslog/validate.go 52.63% <100%> (+2.08%) ⬆️

@@ -17,9 +17,9 @@ func rfc5424FormatterWithTagAsAppName(p srslog.Priority, hostname, tag, content
}

func rfc5424MicroFormatterWithTagAsAppName(p srslog.Priority, hostname, tag, content string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

does rfc5424FormatterWithTagAsAppName need to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is WIP for check. I will close it.

@fuweid fuweid closed this Jul 2, 2018
@fuweid
Copy link
Contributor Author

fuweid commented Jul 2, 2018

Reuse this branch @shaloulcy sorry for that

@fuweid fuweid reopened this Jul 2, 2018
@fuweid fuweid changed the title [WIP] enhance: add unix nano timestamp number in the rfc5424micro enhance: add new formatter for syslog Jul 2, 2018
@fuweid
Copy link
Contributor Author

fuweid commented Jul 2, 2018

Update the code and comment. Please take a look @shaloulcy

@pouchrobot
Copy link
Collaborator

ping @fuweid
We found that this PR is over 10 commits behind master.
Please rebase the branch against master and push it back again. Thanks a lot.

A lot of log collectors consumes log data and indexs it by the timestamp
in millisecond format. In order to make the collector to consume data in
the order, we need to add the sequence number in the log. Simplify the
case, PouchContainer will uses the nano timestamp as sequene number.

Signed-off-by: Wei Fu <fhfuwei@163.com>
@shaloulcy
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jul 5, 2018
@shaloulcy
Copy link
Contributor

@fuweid please rebase your code~

@fuweid
Copy link
Contributor Author

fuweid commented Jul 5, 2018

I have rebased my code with latest upstream...

@fuweid fuweid merged commit 7f4f23d into AliyunContainerService:master Jul 5, 2018
@fuweid fuweid deleted the enhance_show_unixnano_timestamp_in_syslog branch July 5, 2018 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM one maintainer or community participant agrees to merge the pull reuqest. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants