From b7406f0581cf6ceaf385cff1352956165bdbc746 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Thu, 23 May 2024 17:02:13 +0530 Subject: [PATCH 1/9] otelzap: Implement with method --- bridges/otelzap/core.go | 21 +++++++++++++++++++-- bridges/otelzap/core_test.go | 16 ++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 82680c233ba..28660684c3f 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -7,6 +7,7 @@ package otelzap // import "go.opentelemetry.io/contrib/bridges/otelzap" import ( "context" + "slices" "go.uber.org/zap/zapcore" @@ -88,6 +89,7 @@ func WithLoggerProvider(provider log.LoggerProvider) Option { // Core is a [zapcore.Core] that sends logging records to OpenTelemetry. type Core struct { logger log.Logger + attr []log.KeyValue } // Compile-time check *Core implements zapcore.Core. @@ -108,10 +110,21 @@ func (o *Core) Enabled(level zapcore.Level) bool { return o.logger.Enabled(context.Background(), r) } -// TODO // With adds structured context to the Core. func (o *Core) With(fields []zapcore.Field) zapcore.Core { - return o + clone := o.clone() + if len(fields) > 0 { + attrbuf := convertField(fields) + clone.attr = append(clone.attr, attrbuf...) + } + return clone +} + +func (o *Core) clone() *Core { + return &Core{ + logger: o.logger, + attr: slices.Clone(o.attr), + } } // TODO @@ -145,7 +158,11 @@ func (o *Core) Write(ent zapcore.Entry, fields []zapcore.Field) error { if len(fields) > 0 { attrbuf := convertField(fields) + attrbuf = slices.Grow(attrbuf, len(o.attr)) + attrbuf = append(attrbuf, o.attr...) r.AddAttributes(attrbuf...) + } else { + r.AddAttributes(o.attr...) } o.logger.Emit(context.Background(), r) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 1d7b519cce6..0c580ceced1 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -40,6 +40,22 @@ func TestCore(t *testing.T) { assert.Equal(t, testValue, value2Result(kv.Value)) return true }) + + rec.Reset() + + // test child logger with accumulated fields + childlogger := logger.With(zap.String("workplace", "otel")) + childlogger.Info(testMessage) + + got = rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, 1, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + assert.Equal(t, "workplace", string(kv.Key)) + assert.Equal(t, "otel", value2Result(kv.Value)) + return true + }) } func TestCoreEnabled(t *testing.T) { From caa59bc1e65726c2b33f93e31c2d8ed431990c06 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Fri, 24 May 2024 09:24:35 +0530 Subject: [PATCH 2/9] to not use extra attribute variable --- bridges/otelzap/core.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 28660684c3f..89f0e580ed9 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -114,8 +114,7 @@ func (o *Core) Enabled(level zapcore.Level) bool { func (o *Core) With(fields []zapcore.Field) zapcore.Core { clone := o.clone() if len(fields) > 0 { - attrbuf := convertField(fields) - clone.attr = append(clone.attr, attrbuf...) + clone.attr = append(clone.attr, convertField(fields)...) } return clone } @@ -156,13 +155,9 @@ func (o *Core) Write(ent zapcore.Entry, fields []zapcore.Field) error { // TODO: Handle zap.Array. // TODO: Handle ent.LoggerName. + r.AddAttributes(o.attr...) if len(fields) > 0 { - attrbuf := convertField(fields) - attrbuf = slices.Grow(attrbuf, len(o.attr)) - attrbuf = append(attrbuf, o.attr...) - r.AddAttributes(attrbuf...) - } else { - r.AddAttributes(o.attr...) + r.AddAttributes(convertField(fields)...) } o.logger.Emit(context.Background(), r) From 1b64b20ac67b331c8ecc798a8efe993a1b3323fa Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 24 May 2024 12:36:46 +0530 Subject: [PATCH 3/9] Update bridges/otelzap/core.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- bridges/otelzap/core.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 89f0e580ed9..00396681555 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -112,11 +112,11 @@ func (o *Core) Enabled(level zapcore.Level) bool { // With adds structured context to the Core. func (o *Core) With(fields []zapcore.Field) zapcore.Core { - clone := o.clone() + cloned := o.clone() if len(fields) > 0 { - clone.attr = append(clone.attr, convertField(fields)...) + cloned.attr = append(cloned.attr, convertField(fields)...) } - return clone + return cloned } func (o *Core) clone() *Core { From c58ba25d3f7c5bfdaaf36ab2dc3a8b8ad719d74a Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Fri, 24 May 2024 15:39:12 +0530 Subject: [PATCH 4/9] more tests --- bridges/otelzap/core_test.go | 69 +++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 0c580ceced1..0016899e6bb 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -29,32 +29,61 @@ func TestCore(t *testing.T) { zc := NewCore(loggerName, WithLoggerProvider(rec)) logger := zap.New(zc) - logger.Info(testMessage, zap.String(testKey, testValue)) - - got := rec.Result()[0].Records[0] - assert.Equal(t, testMessage, got.Body().AsString()) - assert.Equal(t, log.SeverityInfo, got.Severity()) - assert.Equal(t, 1, got.AttributesLen()) - got.WalkAttributes(func(kv log.KeyValue) bool { - assert.Equal(t, testKey, string(kv.Key)) - assert.Equal(t, testValue, value2Result(kv.Value)) - return true + t.Run("Test `Write` method", func(t *testing.T) { + logger.Info(testMessage, zap.String(testKey, testValue)) + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, 1, got.AttributesLen()) + got.WalkAttributes(func(kv log.KeyValue) bool { + assert.Equal(t, testKey, string(kv.Key)) + assert.Equal(t, testValue, value2Result(kv.Value)) + return true + }) }) rec.Reset() // test child logger with accumulated fields - childlogger := logger.With(zap.String("workplace", "otel")) - childlogger.Info(testMessage) + t.Run("Test 'With' method", func(t *testing.T) { + testCases := [][]string{{"test1", "value1"}, {"test2", "value2"}} + childlogger := logger.With(zap.String(testCases[0][0], testCases[0][1])) + childlogger.Info(testMessage, zap.String(testCases[1][0], testCases[1][1])) + + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, 2, got.AttributesLen()) + + index := 0 + got.WalkAttributes(func(kv log.KeyValue) bool { + assert.Equal(t, testCases[index][0], string(kv.Key)) + assert.Equal(t, testCases[index][1], value2Result(kv.Value)) + index++ + return true + }) + }) - got = rec.Result()[0].Records[0] - assert.Equal(t, testMessage, got.Body().AsString()) - assert.Equal(t, log.SeverityInfo, got.Severity()) - assert.Equal(t, 1, got.AttributesLen()) - got.WalkAttributes(func(kv log.KeyValue) bool { - assert.Equal(t, "workplace", string(kv.Key)) - assert.Equal(t, "otel", value2Result(kv.Value)) - return true + rec.Reset() + + t.Run("Test 'With' method - twice", func(t *testing.T) { + testCases := [][]string{{"test1", "value1"}, {"test2", "value2"}, {"test3", "value3"}} + childlogger := logger.With(zap.String(testCases[0][0], testCases[0][1])) + childlogger2 := childlogger.With(zap.String(testCases[1][0], testCases[1][1])) + childlogger2.Info(testMessage, zap.String(testCases[2][0], testCases[2][1])) + + got := rec.Result()[0].Records[0] + assert.Equal(t, testMessage, got.Body().AsString()) + assert.Equal(t, log.SeverityInfo, got.Severity()) + assert.Equal(t, 3, got.AttributesLen()) + + index := 0 + got.WalkAttributes(func(kv log.KeyValue) bool { + assert.Equal(t, testCases[index][0], string(kv.Key)) + assert.Equal(t, testCases[index][1], value2Result(kv.Value)) + index++ + return true + }) }) } From 0164645a8164dc2bf08f0f079cff43f552e4d325 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Fri, 24 May 2024 15:40:50 +0530 Subject: [PATCH 5/9] lint --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 0016899e6bb..9927c973390 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -29,7 +29,7 @@ func TestCore(t *testing.T) { zc := NewCore(loggerName, WithLoggerProvider(rec)) logger := zap.New(zc) - t.Run("Test `Write` method", func(t *testing.T) { + t.Run("Test 'Write' method", func(t *testing.T) { logger.Info(testMessage, zap.String(testKey, testValue)) got := rec.Result()[0].Records[0] assert.Equal(t, testMessage, got.Body().AsString()) From bf3fd73c6406941306e680cab004926c5a3bbe14 Mon Sep 17 00:00:00 2001 From: khushijain21 Date: Wed, 29 May 2024 12:46:39 +0530 Subject: [PATCH 6/9] update test desc --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 9927c973390..70ca963b772 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -66,7 +66,7 @@ func TestCore(t *testing.T) { rec.Reset() - t.Run("Test 'With' method - twice", func(t *testing.T) { + t.Run("Test 'With` method for nested child loggers ", func(t *testing.T) { testCases := [][]string{{"test1", "value1"}, {"test2", "value2"}, {"test3", "value3"}} childlogger := logger.With(zap.String(testCases[0][0], testCases[0][1])) childlogger2 := childlogger.With(zap.String(testCases[1][0], testCases[1][1])) From 5edd1a378d2a3dfe66612f080c5e6a7a690a0cd0 Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 May 2024 12:25:31 +0530 Subject: [PATCH 7/9] Update bridges/otelzap/core_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 70ca963b772..fdeb8efa125 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -29,7 +29,7 @@ func TestCore(t *testing.T) { zc := NewCore(loggerName, WithLoggerProvider(rec)) logger := zap.New(zc) - t.Run("Test 'Write' method", func(t *testing.T) { + t.Run("Write", func(t *testing.T) { logger.Info(testMessage, zap.String(testKey, testValue)) got := rec.Result()[0].Records[0] assert.Equal(t, testMessage, got.Body().AsString()) From f53ebce302c36864639175d7240883cd79c2986c Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 May 2024 12:25:40 +0530 Subject: [PATCH 8/9] Update bridges/otelzap/core_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index fdeb8efa125..0c19cc0a7d6 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -45,7 +45,7 @@ func TestCore(t *testing.T) { rec.Reset() // test child logger with accumulated fields - t.Run("Test 'With' method", func(t *testing.T) { + t.Run("With", func(t *testing.T) { testCases := [][]string{{"test1", "value1"}, {"test2", "value2"}} childlogger := logger.With(zap.String(testCases[0][0], testCases[0][1])) childlogger.Info(testMessage, zap.String(testCases[1][0], testCases[1][1])) From 85e12114ad8f4f8022d63a02e8c01cab0c1d6ad2 Mon Sep 17 00:00:00 2001 From: Khushi Jain Date: Fri, 31 May 2024 12:25:49 +0530 Subject: [PATCH 9/9] Update bridges/otelzap/core_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- bridges/otelzap/core_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/otelzap/core_test.go b/bridges/otelzap/core_test.go index 0c19cc0a7d6..82e6a23b8a6 100644 --- a/bridges/otelzap/core_test.go +++ b/bridges/otelzap/core_test.go @@ -66,7 +66,7 @@ func TestCore(t *testing.T) { rec.Reset() - t.Run("Test 'With` method for nested child loggers ", func(t *testing.T) { + t.Run("WithMultiple", func(t *testing.T) { testCases := [][]string{{"test1", "value1"}, {"test2", "value2"}, {"test3", "value3"}} childlogger := logger.With(zap.String(testCases[0][0], testCases[0][1])) childlogger2 := childlogger.With(zap.String(testCases[1][0], testCases[1][1]))