From 9dd65d097b12384d7d465fe5846ab49da0b3f59a Mon Sep 17 00:00:00 2001 From: bonczj Date: Tue, 26 Sep 2017 10:22:10 -0400 Subject: [PATCH 1/2] Added tag log option to json-logger Fixes #19803 Updated the json-logger to utilize the common log option 'tag' that can define container/image information to include as part of logging. When the 'tag' log option is not included, there is no change to the log content via the json-logger. When the 'tag' log option is included, the tag will be parsed as a template and the result will be stored within each log entry as the attribute 'tag'. Update: Removing test added to integration_cli as those have been deprecated. Update: Using proper test calls (require and assert) in jsonfilelog_test.go based on review. Update: Added new unit test configs for logs with tag. Updated unit test error checking. Update: Cleanup check in jsonlogbytes_test.go to match pending changes in PR #34946. Update: Merging to correct conflicts from PR #34946. Signed-off-by: bonczj Upstream-commit: 5f50f4f511cd84e79bf005817af346b1764df27f Component: engine --- .../daemon/logger/jsonfilelog/jsonfilelog.go | 14 +++++- .../logger/jsonfilelog/jsonfilelog_test.go | 45 ++++++++++++++++++- .../logger/jsonfilelog/jsonlog/jsonlog.go | 3 ++ .../jsonfilelog/jsonlog/jsonlogbytes.go | 10 +++++ .../jsonfilelog/jsonlog/jsonlogbytes_test.go | 2 + .../daemon/logger/jsonfilelog/read_test.go | 2 +- 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go b/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go index 7aa92f3d37..19505db3fc 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go @@ -27,6 +27,7 @@ type JSONFileLogger struct { closed bool writer *loggerutils.LogFile readers map[*logger.LogWatcher]struct{} // stores the active log followers + tag string // tag values requested by the user to log } func init() { @@ -61,6 +62,12 @@ func New(info logger.Info) (logger.Logger, error) { } } + // no default template. only use a tag if the user asked for it + tag, err := loggerutils.ParseLogTag(info, "") + if err != nil { + return nil, err + } + var extra []byte attrs, err := info.ExtraAttributes(nil) if err != nil { @@ -76,7 +83,7 @@ func New(info logger.Info) (logger.Logger, error) { buf := bytes.NewBuffer(nil) marshalFunc := func(msg *logger.Message) ([]byte, error) { - if err := marshalMessage(msg, extra, buf); err != nil { + if err := marshalMessage(msg, extra, buf, tag); err != nil { return nil, err } b := buf.Bytes() @@ -92,6 +99,7 @@ func New(info logger.Info) (logger.Logger, error) { return &JSONFileLogger{ writer: writer, readers: make(map[*logger.LogWatcher]struct{}), + tag: tag, }, nil } @@ -103,7 +111,7 @@ func (l *JSONFileLogger) Log(msg *logger.Message) error { return err } -func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffer) error { +func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffer, tag string) error { logLine := msg.Line if !msg.Partial { logLine = append(msg.Line, '\n') @@ -113,6 +121,7 @@ func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffe Stream: msg.Source, Created: msg.Timestamp, RawAttrs: extra, + Tag: tag, }).MarshalJSONBuf(buf) if err != nil { return errors.Wrap(err, "error writing log message to buffer") @@ -130,6 +139,7 @@ func ValidateLogOpt(cfg map[string]string) error { case "labels": case "env": case "env-regex": + case "tag": default: return fmt.Errorf("unknown log opt '%s' for json-file log driver", key) } diff --git a/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go b/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go index 893c054669..e1fd46e7de 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go @@ -57,6 +57,49 @@ func TestJSONFileLogger(t *testing.T) { } } +func TestJSONFileLoggerWithTags(t *testing.T) { + cid := "a7317399f3f857173c6179d44823594f8294678dea9999662e5c625b5a1c7657" + cname := "test-container" + tmp, err := ioutil.TempDir("", "docker-logger-") + + require.NoError(t, err) + + defer os.RemoveAll(tmp) + filename := filepath.Join(tmp, "container.log") + l, err := New(logger.Info{ + Config: map[string]string{ + "tag": "{{.ID}}/{{.Name}}", // first 12 characters of ContainerID and full ContainerName + }, + ContainerID: cid, + ContainerName: cname, + LogPath: filename, + }) + + require.NoError(t, err) + defer l.Close() + + err = l.Log(&logger.Message{Line: []byte("line1"), Source: "src1"}) + require.NoError(t, err) + + err = l.Log(&logger.Message{Line: []byte("line2"), Source: "src2"}) + require.NoError(t, err) + + err = l.Log(&logger.Message{Line: []byte("line3"), Source: "src3"}) + require.NoError(t, err) + + res, err := ioutil.ReadFile(filename) + require.NoError(t, err) + + expected := `{"log":"line1\n","stream":"src1","tag":"a7317399f3f8/test-container","time":"0001-01-01T00:00:00Z"} +{"log":"line2\n","stream":"src2","tag":"a7317399f3f8/test-container","time":"0001-01-01T00:00:00Z"} +{"log":"line3\n","stream":"src3","tag":"a7317399f3f8/test-container","time":"0001-01-01T00:00:00Z"} +` + + if string(res) != expected { + t.Fatalf("Wrong log content: %q, expected %q", res, expected) + } +} + func BenchmarkJSONFileLoggerLog(b *testing.B) { tmp := fs.NewDir(b, "bench-jsonfilelog") defer tmp.Remove() @@ -82,7 +125,7 @@ func BenchmarkJSONFileLoggerLog(b *testing.B) { } buf := bytes.NewBuffer(nil) - require.NoError(b, marshalMessage(msg, nil, buf)) + require.NoError(b, marshalMessage(msg, nil, buf, "")) b.SetBytes(int64(buf.Len())) b.ResetTimer() diff --git a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go index 549e355855..3121a8539f 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go @@ -14,6 +14,8 @@ type JSONLog struct { Created time.Time `json:"time"` // Attrs is the list of extra attributes provided by the user Attrs map[string]string `json:"attrs,omitempty"` + // Tags requested the operator + Tag string `json:"tag,omitempty"` } // Reset all fields to their zero value. @@ -22,4 +24,5 @@ func (jl *JSONLog) Reset() { jl.Stream = "" jl.Created = time.Time{} jl.Attrs = make(map[string]string) + jl.Tag = "" } diff --git a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go index 37604ae549..00e2d00f50 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go @@ -12,6 +12,7 @@ type JSONLogs struct { Log []byte `json:"log,omitempty"` Stream string `json:"stream,omitempty"` Created time.Time `json:"time"` + Tag string `json:"tag,omitempty"` // json-encoded bytes RawAttrs json.RawMessage `json:"attrs,omitempty"` @@ -37,6 +38,15 @@ func (mj *JSONLogs) MarshalJSONBuf(buf *bytes.Buffer) error { buf.WriteString(`"stream":`) ffjsonWriteJSONBytesAsString(buf, []byte(mj.Stream)) } + if len(mj.Tag) > 0 { + if first { + first = false + } else { + buf.WriteString(`,`) + } + buf.WriteString(`"tag":`) + ffjsonWriteJSONBytesAsString(buf, []byte(mj.Tag)) + } if len(mj.RawAttrs) > 0 { if first { first = false diff --git a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go index 4645cd4faa..cb506a3006 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go @@ -29,6 +29,8 @@ func TestJSONLogsMarshalJSONBuf(t *testing.T) { {Log: []byte{0x7F}}: `^{\"log\":\"\x7f\",\"time\":`, // with raw attributes {Log: []byte("A log line"), RawAttrs: []byte(`{"hello":"world","value":1234}`)}: `^{\"log\":\"A log line\",\"attrs\":{\"hello\":\"world\",\"value\":1234},\"time\":`, + // with Tag set + {Log: []byte("A log line with tag"), Tag: "test-tag"}: `^{\"log\":\"A log line with tag\",\"tag\":\"test-tag\",\"time\":`, } for jsonLog, expression := range logs { var buf bytes.Buffer diff --git a/components/engine/daemon/logger/jsonfilelog/read_test.go b/components/engine/daemon/logger/jsonfilelog/read_test.go index 599fdf9336..ddc1265f03 100644 --- a/components/engine/daemon/logger/jsonfilelog/read_test.go +++ b/components/engine/daemon/logger/jsonfilelog/read_test.go @@ -35,7 +35,7 @@ func BenchmarkJSONFileLoggerReadLogs(b *testing.B) { } buf := bytes.NewBuffer(nil) - require.NoError(b, marshalMessage(msg, nil, buf)) + require.NoError(b, marshalMessage(msg, nil, buf, "")) b.SetBytes(int64(buf.Len())) b.ResetTimer() From 3cf8a0c4429a7b6bb6da2450fba3894898837059 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Mon, 8 Jan 2018 00:54:58 +0000 Subject: [PATCH 2/2] Carry 34248 Added tag log option to json-logger and use RawAttrs This fix carries PR 34248: Added tag log option to json-logger This fix changes to use RawAttrs based on review feedback. This fix fixes 19803, this fix closes 34248. Signed-off-by: Yong Tang Upstream-commit: e77267c5a682e2c5aaa32469f2c83c2479d57566 Component: engine --- .../daemon/logger/jsonfilelog/jsonfilelog.go | 17 ++++++++++------- .../logger/jsonfilelog/jsonfilelog_test.go | 14 ++++++-------- .../logger/jsonfilelog/jsonlog/jsonlog.go | 3 --- .../logger/jsonfilelog/jsonlog/jsonlogbytes.go | 10 ---------- .../jsonfilelog/jsonlog/jsonlogbytes_test.go | 2 +- .../daemon/logger/jsonfilelog/read_test.go | 2 +- 6 files changed, 18 insertions(+), 30 deletions(-) diff --git a/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go b/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go index 19505db3fc..720a7c3b61 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonfilelog.go @@ -62,17 +62,21 @@ func New(info logger.Info) (logger.Logger, error) { } } + attrs, err := info.ExtraAttributes(nil) + if err != nil { + return nil, err + } + // no default template. only use a tag if the user asked for it tag, err := loggerutils.ParseLogTag(info, "") if err != nil { return nil, err } + if tag != "" { + attrs["tag"] = tag + } var extra []byte - attrs, err := info.ExtraAttributes(nil) - if err != nil { - return nil, err - } if len(attrs) > 0 { var err error extra, err = json.Marshal(attrs) @@ -83,7 +87,7 @@ func New(info logger.Info) (logger.Logger, error) { buf := bytes.NewBuffer(nil) marshalFunc := func(msg *logger.Message) ([]byte, error) { - if err := marshalMessage(msg, extra, buf, tag); err != nil { + if err := marshalMessage(msg, extra, buf); err != nil { return nil, err } b := buf.Bytes() @@ -111,7 +115,7 @@ func (l *JSONFileLogger) Log(msg *logger.Message) error { return err } -func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffer, tag string) error { +func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffer) error { logLine := msg.Line if !msg.Partial { logLine = append(msg.Line, '\n') @@ -121,7 +125,6 @@ func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffe Stream: msg.Source, Created: msg.Timestamp, RawAttrs: extra, - Tag: tag, }).MarshalJSONBuf(buf) if err != nil { return errors.Wrap(err, "error writing log message to buffer") diff --git a/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go b/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go index e1fd46e7de..e988e862fd 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonfilelog_test.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/daemon/logger" "github.com/docker/docker/daemon/logger/jsonfilelog/jsonlog" "github.com/gotestyourself/gotestyourself/fs" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -90,14 +91,11 @@ func TestJSONFileLoggerWithTags(t *testing.T) { res, err := ioutil.ReadFile(filename) require.NoError(t, err) - expected := `{"log":"line1\n","stream":"src1","tag":"a7317399f3f8/test-container","time":"0001-01-01T00:00:00Z"} -{"log":"line2\n","stream":"src2","tag":"a7317399f3f8/test-container","time":"0001-01-01T00:00:00Z"} -{"log":"line3\n","stream":"src3","tag":"a7317399f3f8/test-container","time":"0001-01-01T00:00:00Z"} + expected := `{"log":"line1\n","stream":"src1","attrs":{"tag":"a7317399f3f8/test-container"},"time":"0001-01-01T00:00:00Z"} +{"log":"line2\n","stream":"src2","attrs":{"tag":"a7317399f3f8/test-container"},"time":"0001-01-01T00:00:00Z"} +{"log":"line3\n","stream":"src3","attrs":{"tag":"a7317399f3f8/test-container"},"time":"0001-01-01T00:00:00Z"} ` - - if string(res) != expected { - t.Fatalf("Wrong log content: %q, expected %q", res, expected) - } + assert.Equal(t, expected, string(res)) } func BenchmarkJSONFileLoggerLog(b *testing.B) { @@ -125,7 +123,7 @@ func BenchmarkJSONFileLoggerLog(b *testing.B) { } buf := bytes.NewBuffer(nil) - require.NoError(b, marshalMessage(msg, nil, buf, "")) + require.NoError(b, marshalMessage(msg, nil, buf)) b.SetBytes(int64(buf.Len())) b.ResetTimer() diff --git a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go index 3121a8539f..549e355855 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlog.go @@ -14,8 +14,6 @@ type JSONLog struct { Created time.Time `json:"time"` // Attrs is the list of extra attributes provided by the user Attrs map[string]string `json:"attrs,omitempty"` - // Tags requested the operator - Tag string `json:"tag,omitempty"` } // Reset all fields to their zero value. @@ -24,5 +22,4 @@ func (jl *JSONLog) Reset() { jl.Stream = "" jl.Created = time.Time{} jl.Attrs = make(map[string]string) - jl.Tag = "" } diff --git a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go index 00e2d00f50..37604ae549 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes.go @@ -12,7 +12,6 @@ type JSONLogs struct { Log []byte `json:"log,omitempty"` Stream string `json:"stream,omitempty"` Created time.Time `json:"time"` - Tag string `json:"tag,omitempty"` // json-encoded bytes RawAttrs json.RawMessage `json:"attrs,omitempty"` @@ -38,15 +37,6 @@ func (mj *JSONLogs) MarshalJSONBuf(buf *bytes.Buffer) error { buf.WriteString(`"stream":`) ffjsonWriteJSONBytesAsString(buf, []byte(mj.Stream)) } - if len(mj.Tag) > 0 { - if first { - first = false - } else { - buf.WriteString(`,`) - } - buf.WriteString(`"tag":`) - ffjsonWriteJSONBytesAsString(buf, []byte(mj.Tag)) - } if len(mj.RawAttrs) > 0 { if first { first = false diff --git a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go index cb506a3006..e52d32c037 100644 --- a/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go +++ b/components/engine/daemon/logger/jsonfilelog/jsonlog/jsonlogbytes_test.go @@ -30,7 +30,7 @@ func TestJSONLogsMarshalJSONBuf(t *testing.T) { // with raw attributes {Log: []byte("A log line"), RawAttrs: []byte(`{"hello":"world","value":1234}`)}: `^{\"log\":\"A log line\",\"attrs\":{\"hello\":\"world\",\"value\":1234},\"time\":`, // with Tag set - {Log: []byte("A log line with tag"), Tag: "test-tag"}: `^{\"log\":\"A log line with tag\",\"tag\":\"test-tag\",\"time\":`, + {Log: []byte("A log line with tag"), RawAttrs: []byte(`{"hello":"world","value":1234}`)}: `^{\"log\":\"A log line with tag\",\"attrs\":{\"hello\":\"world\",\"value\":1234},\"time\":`, } for jsonLog, expression := range logs { var buf bytes.Buffer diff --git a/components/engine/daemon/logger/jsonfilelog/read_test.go b/components/engine/daemon/logger/jsonfilelog/read_test.go index ddc1265f03..599fdf9336 100644 --- a/components/engine/daemon/logger/jsonfilelog/read_test.go +++ b/components/engine/daemon/logger/jsonfilelog/read_test.go @@ -35,7 +35,7 @@ func BenchmarkJSONFileLoggerReadLogs(b *testing.B) { } buf := bytes.NewBuffer(nil) - require.NoError(b, marshalMessage(msg, nil, buf, "")) + require.NoError(b, marshalMessage(msg, nil, buf)) b.SetBytes(int64(buf.Len())) b.ResetTimer()