From 562baaf1e860a3d9f1f5371d13e644296a6b89f7 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Fri, 19 Apr 2024 14:40:07 -0600 Subject: [PATCH 1/6] Write logs once we have a logger --- otelcol/collector.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/otelcol/collector.go b/otelcol/collector.go index f90956f8104..107373ed5e7 100644 --- a/otelcol/collector.go +++ b/otelcol/collector.go @@ -15,6 +15,7 @@ import ( "go.uber.org/multierr" "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/confmap" @@ -109,6 +110,8 @@ type Collector struct { signalsChannel chan os.Signal // asyncErrorChannel is used to signal a fatal error from any component. asyncErrorChannel chan error + + ol *observer.ObservedLogs } // NewCollector creates and returns a new instance of Collector. @@ -116,7 +119,8 @@ func NewCollector(set CollectorSettings) (*Collector, error) { var err error configProvider := set.ConfigProvider - set.ConfigProviderSettings.ResolverSettings.ProviderSettings = confmap.ProviderSettings{Logger: zap.NewNop()} + core, ol := observer.New(zap.DebugLevel) + set.ConfigProviderSettings.ResolverSettings.ProviderSettings = confmap.ProviderSettings{Logger: zap.New(core)} set.ConfigProviderSettings.ResolverSettings.ConverterSettings = confmap.ConverterSettings{} if configProvider == nil { @@ -137,6 +141,7 @@ func NewCollector(set CollectorSettings) (*Collector, error) { signalsChannel: make(chan os.Signal, 3), asyncErrorChannel: make(chan error), configProvider: configProvider, + ol: ol, }, nil } @@ -202,6 +207,12 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error { return err } + if col.ol != nil { + for _, log := range col.ol.All() { + col.service.Logger().Log(log.Level, log.Message) + } + } + if !col.set.SkipSettingGRPCLogger { grpclog.SetLogger(col.service.Logger(), cfg.Service.Telemetry.Logs.Level) } From 31b4ef9f11fe78d9afbc1011fe21fff48213e348 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 23 Apr 2024 11:20:31 -0600 Subject: [PATCH 2/6] Add a test --- otelcol/collector_test.go | 19 +++++++++++++++ otelcol/testdata/otelcol-env-var.yaml | 34 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 otelcol/testdata/otelcol-env-var.yaml diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index 2ae56fc58c0..a92ba38d4fe 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -476,6 +476,25 @@ func TestPassConfmapToServiceFailure(t *testing.T) { require.Error(t, err) } +func TestCollectorConfmapLogs(t *testing.T) { + set := CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: nopFactories, + ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}), + } + col, err := NewCollector(set) + require.NoError(t, err) + + wg := startCollector(context.Background(), t, col) + + col.Shutdown() + wg.Wait() + assert.Equal(t, StateClosed, col.GetState()) + + assert.NotNil(t, col.ol) + assert.True(t, col.ol.Len() > 0) +} + func startCollector(ctx context.Context, t *testing.T, col *Collector) *sync.WaitGroup { wg := &sync.WaitGroup{} wg.Add(1) diff --git a/otelcol/testdata/otelcol-env-var.yaml b/otelcol/testdata/otelcol-env-var.yaml new file mode 100644 index 00000000000..c4c9929ae82 --- /dev/null +++ b/otelcol/testdata/otelcol-env-var.yaml @@ -0,0 +1,34 @@ +receivers: + nop: + +processors: + nop: + +exporters: + nop: + +extensions: + nop: + +connectors: + nop/con: + +service: + telemetry: + logs: + initial_fields: + "test": ${env:DOESNOTEXIST} + extensions: [nop] + pipelines: + traces: + receivers: [nop] + processors: [nop] + exporters: [nop, nop/con] + metrics: + receivers: [nop] + processors: [nop] + exporters: [nop] + logs: + receivers: [nop, nop/con] + processors: [nop] + exporters: [nop] From d23b50a6336a63a43f2bd5e8b3b7d86904a77829 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 23 Apr 2024 11:22:36 -0600 Subject: [PATCH 3/6] Use `Greater` assertion --- otelcol/collector_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index a92ba38d4fe..48660415a8a 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -480,7 +480,7 @@ func TestCollectorConfmapLogs(t *testing.T) { set := CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}), + ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-env-var.yaml")}), } col, err := NewCollector(set) require.NoError(t, err) @@ -492,7 +492,7 @@ func TestCollectorConfmapLogs(t *testing.T) { assert.Equal(t, StateClosed, col.GetState()) assert.NotNil(t, col.ol) - assert.True(t, col.ol.Len() > 0) + assert.Greater(t, col.ol.Len(), 0) } func startCollector(ctx context.Context, t *testing.T, col *Collector) *sync.WaitGroup { From 47b5f0cacd410e9cc6912745a157b82cad8045c2 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Thu, 25 Apr 2024 11:30:35 -0600 Subject: [PATCH 4/6] Update test to use Provider that logs expected message --- otelcol/collector_test.go | 39 +++++++++++++++++-- ...-var.yaml => otelcol-confmap-logging.yaml} | 2 +- 2 files changed, 37 insertions(+), 4 deletions(-) rename otelcol/testdata/{otelcol-env-var.yaml => otelcol-confmap-logging.yaml} (92%) diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index 48660415a8a..cfb125898a2 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/confmap" @@ -476,11 +477,42 @@ func TestPassConfmapToServiceFailure(t *testing.T) { require.Error(t, err) } +type provider struct { + logger *zap.Logger +} + +func newWithSettings(ps confmap.ProviderSettings) confmap.Provider { + return &provider{ + logger: ps.Logger, + } +} + +func newFactory() confmap.ProviderFactory { + return confmap.NewProviderFactory(newWithSettings) +} + +func (p *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { + p.logger.Info("this is a test log") + + return confmap.NewRetrieved("test") +} + +func (*provider) Scheme() string { + return "test" +} + +func (*provider) Shutdown(context.Context) error { + return nil +} + func TestCollectorConfmapLogs(t *testing.T) { + configProviderSettings := newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-confmap-logging.yaml")}) + configProviderSettings.ResolverSettings.ProviderFactories = append(configProviderSettings.ResolverSettings.ProviderFactories, newFactory()) + set := CollectorSettings{ BuildInfo: component.NewDefaultBuildInfo(), Factories: nopFactories, - ConfigProviderSettings: newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-env-var.yaml")}), + ConfigProviderSettings: configProviderSettings, } col, err := NewCollector(set) require.NoError(t, err) @@ -491,8 +523,9 @@ func TestCollectorConfmapLogs(t *testing.T) { wg.Wait() assert.Equal(t, StateClosed, col.GetState()) - assert.NotNil(t, col.ol) - assert.Greater(t, col.ol.Len(), 0) + require.NotNil(t, col.ol) + require.Greater(t, col.ol.Len(), 0) + assert.Equal(t, "this is a test log", col.ol.All()[0].Message) } func startCollector(ctx context.Context, t *testing.T, col *Collector) *sync.WaitGroup { diff --git a/otelcol/testdata/otelcol-env-var.yaml b/otelcol/testdata/otelcol-confmap-logging.yaml similarity index 92% rename from otelcol/testdata/otelcol-env-var.yaml rename to otelcol/testdata/otelcol-confmap-logging.yaml index c4c9929ae82..c8e7e0c434f 100644 --- a/otelcol/testdata/otelcol-env-var.yaml +++ b/otelcol/testdata/otelcol-confmap-logging.yaml @@ -17,7 +17,7 @@ service: telemetry: logs: initial_fields: - "test": ${env:DOESNOTEXIST} + "test": ${test:ANYTHING} extensions: [nop] pipelines: traces: From 8f3211ea4388caab2632f0eb3e1b5e2df9a7e685 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Fri, 26 Apr 2024 09:12:12 -0600 Subject: [PATCH 5/6] Improve test --- otelcol/collector_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index cfb125898a2..6a7bd1f78b3 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -493,7 +493,6 @@ func newFactory() confmap.ProviderFactory { func (p *provider) Retrieve(_ context.Context, uri string, _ confmap.WatcherFunc) (*confmap.Retrieved, error) { p.logger.Info("this is a test log") - return confmap.NewRetrieved("test") } @@ -525,7 +524,12 @@ func TestCollectorConfmapLogs(t *testing.T) { require.NotNil(t, col.ol) require.Greater(t, col.ol.Len(), 0) - assert.Equal(t, "this is a test log", col.ol.All()[0].Message) + + messages := make([]string, 0) + for _, l := range col.ol.All() { + messages = append(messages, l.Message) + } + assert.Contains(t, messages, "this is a test log") } func startCollector(ctx context.Context, t *testing.T, col *Collector) *sync.WaitGroup { From 8e9bce41cc9c961ac2b76fdbc093dde18ab71772 Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Fri, 26 Apr 2024 10:32:47 -0600 Subject: [PATCH 6/6] Update otelcol/collector_test.go Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- otelcol/collector_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otelcol/collector_test.go b/otelcol/collector_test.go index 6a7bd1f78b3..7f42b3d71a4 100644 --- a/otelcol/collector_test.go +++ b/otelcol/collector_test.go @@ -525,7 +525,7 @@ func TestCollectorConfmapLogs(t *testing.T) { require.NotNil(t, col.ol) require.Greater(t, col.ol.Len(), 0) - messages := make([]string, 0) + messages := make([]string, 0, col.ol.Len()) for _, l := range col.ol.All() { messages = append(messages, l.Message) }