Skip to content

Commit 74c2cf3

Browse files
TylerHelmuthdjaglowskimx-psi
authored andcommitted
[otelcol] Add a custom zapcore.Core for confmap logging (open-telemetry#10056)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Provides a logger to confmap that buffers logs in memory until the primary logger can be used. Once the primary logger exists, places that used the original logger are given the updated Core. If an error occurs that would shut down the collector before the primary logger could be created, the logs are written to stdout/err using a fallback logger. Alternative to open-telemetry#10008 I've pushed the testing I did to show how the logger successfully updates. Before config resolution the debug log in confmap is not printed, but afterwards it is. test config: ```yaml receivers: nop: exporters: otlphttp: endpoint: http://0.0.0.0:4317 headers: # Not set x-test: ${env:TEMP3} debug: # set to "detailed" verbosity: $TEMP service: telemetry: logs: level: debug pipelines: traces: receivers: - nop exporters: - debug ``` ![image](https://github.com/open-telemetry/opentelemetry-collector/assets/12352919/6a17993f-1f97-4c54-9165-5c34dd58d108) <!-- Issue number if applicable --> #### Link to tracking issue Related to open-telemetry#9162 Related to open-telemetry#5615 <!--Describe what testing was performed and which tests were added.--> #### Testing If we like this approach I'll add tests <!--Describe the documentation added.--> #### Documentation --------- Co-authored-by: Dan Jaglowski <[email protected]> Co-authored-by: Pablo Baeyens <[email protected]>
1 parent aff7525 commit 74c2cf3

File tree

7 files changed

+415
-6
lines changed

7 files changed

+415
-6
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: otelcol
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Enable logging during configuration resolution
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [10056]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: []

otelcol/buffered_core.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
// This logger implements zapcore.Core and is based on zaptest/observer.
5+
6+
package otelcol // import "go.opentelemetry.io/collector/otelcol"
7+
8+
import (
9+
"fmt"
10+
"sync"
11+
12+
"go.uber.org/zap/zapcore"
13+
)
14+
15+
type loggedEntry struct {
16+
zapcore.Entry
17+
Context []zapcore.Field
18+
}
19+
20+
func newBufferedCore(enab zapcore.LevelEnabler) *bufferedCore {
21+
return &bufferedCore{LevelEnabler: enab}
22+
}
23+
24+
var _ zapcore.Core = (*bufferedCore)(nil)
25+
26+
type bufferedCore struct {
27+
zapcore.LevelEnabler
28+
mu sync.RWMutex
29+
logs []loggedEntry
30+
context []zapcore.Field
31+
logsTaken bool
32+
}
33+
34+
func (bc *bufferedCore) Level() zapcore.Level {
35+
return zapcore.LevelOf(bc.LevelEnabler)
36+
}
37+
38+
func (bc *bufferedCore) Check(ent zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry {
39+
if bc.Enabled(ent.Level) {
40+
return ce.AddCore(ent, bc)
41+
}
42+
return ce
43+
}
44+
45+
func (bc *bufferedCore) With(fields []zapcore.Field) zapcore.Core {
46+
return &bufferedCore{
47+
LevelEnabler: bc.LevelEnabler,
48+
logs: bc.logs,
49+
logsTaken: bc.logsTaken,
50+
context: append(bc.context, fields...),
51+
}
52+
}
53+
54+
func (bc *bufferedCore) Write(ent zapcore.Entry, fields []zapcore.Field) error {
55+
bc.mu.Lock()
56+
defer bc.mu.Unlock()
57+
if bc.logsTaken {
58+
return fmt.Errorf("the buffered logs have already been taken so writing is no longer supported")
59+
}
60+
all := make([]zapcore.Field, 0, len(fields)+len(bc.context))
61+
all = append(all, bc.context...)
62+
all = append(all, fields...)
63+
bc.logs = append(bc.logs, loggedEntry{ent, all})
64+
return nil
65+
}
66+
67+
func (bc *bufferedCore) Sync() error {
68+
return nil
69+
}
70+
71+
func (bc *bufferedCore) TakeLogs() []loggedEntry {
72+
if !bc.logsTaken {
73+
bc.mu.Lock()
74+
defer bc.mu.Unlock()
75+
logs := bc.logs
76+
bc.logs = nil
77+
bc.logsTaken = true
78+
return logs
79+
}
80+
return nil
81+
}

otelcol/buffered_core_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package otelcol
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"go.uber.org/zap/zapcore"
12+
)
13+
14+
func Test_bufferedCore_Level(t *testing.T) {
15+
bc := newBufferedCore(zapcore.InfoLevel)
16+
assert.Equal(t, zapcore.InfoLevel, bc.Level())
17+
}
18+
19+
func Test_bufferedCore_Check(t *testing.T) {
20+
t.Run("check passed", func(t *testing.T) {
21+
bc := newBufferedCore(zapcore.InfoLevel)
22+
e := zapcore.Entry{
23+
Level: zapcore.InfoLevel,
24+
}
25+
expected := &zapcore.CheckedEntry{}
26+
expected = expected.AddCore(e, bc)
27+
ce := bc.Check(e, nil)
28+
assert.Equal(t, expected, ce)
29+
})
30+
31+
t.Run("check did not pass", func(t *testing.T) {
32+
bc := newBufferedCore(zapcore.InfoLevel)
33+
e := zapcore.Entry{
34+
Level: zapcore.DebugLevel,
35+
}
36+
ce := bc.Check(e, nil)
37+
assert.Nil(t, ce)
38+
})
39+
}
40+
41+
func Test_bufferedCore_With(t *testing.T) {
42+
bc := newBufferedCore(zapcore.InfoLevel)
43+
bc.logsTaken = true
44+
bc.context = []zapcore.Field{
45+
{Key: "original", String: "context"},
46+
}
47+
inputs := []zapcore.Field{
48+
{Key: "test", String: "passed"},
49+
}
50+
expected := []zapcore.Field{
51+
{Key: "original", String: "context"},
52+
{Key: "test", String: "passed"},
53+
}
54+
newBC := bc.With(inputs)
55+
assert.Equal(t, expected, newBC.(*bufferedCore).context)
56+
assert.True(t, newBC.(*bufferedCore).logsTaken)
57+
}
58+
59+
func Test_bufferedCore_Write(t *testing.T) {
60+
bc := newBufferedCore(zapcore.InfoLevel)
61+
e := zapcore.Entry{
62+
Level: zapcore.DebugLevel,
63+
Message: "test",
64+
}
65+
fields := []zapcore.Field{
66+
{Key: "field1", String: "value1"},
67+
}
68+
err := bc.Write(e, fields)
69+
require.NoError(t, err)
70+
71+
expected := loggedEntry{
72+
e,
73+
fields,
74+
}
75+
require.Equal(t, 1, len(bc.logs))
76+
require.Equal(t, expected, bc.logs[0])
77+
}
78+
79+
func Test_bufferedCore_Sync(t *testing.T) {
80+
bc := newBufferedCore(zapcore.InfoLevel)
81+
assert.NoError(t, bc.Sync())
82+
}
83+
84+
func Test_bufferedCore_TakeLogs(t *testing.T) {
85+
bc := newBufferedCore(zapcore.InfoLevel)
86+
e := zapcore.Entry{
87+
Level: zapcore.DebugLevel,
88+
Message: "test",
89+
}
90+
fields := []zapcore.Field{
91+
{Key: "field1", String: "value1"},
92+
}
93+
err := bc.Write(e, fields)
94+
require.NoError(t, err)
95+
96+
expected := []loggedEntry{
97+
{
98+
e,
99+
fields,
100+
},
101+
}
102+
assert.Equal(t, expected, bc.TakeLogs())
103+
assert.Nil(t, bc.logs)
104+
105+
assert.Error(t, bc.Write(e, fields))
106+
assert.Nil(t, bc.TakeLogs())
107+
}

otelcol/collector.go

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package otelcol // import "go.opentelemetry.io/collector/otelcol"
77

88
import (
99
"context"
10+
"errors"
1011
"fmt"
1112
"os"
1213
"os/signal"
@@ -15,6 +16,7 @@ import (
1516

1617
"go.uber.org/multierr"
1718
"go.uber.org/zap"
19+
"go.uber.org/zap/zapcore"
1820

1921
"go.opentelemetry.io/collector/component"
2022
"go.opentelemetry.io/collector/confmap"
@@ -108,15 +110,20 @@ type Collector struct {
108110
// signalsChannel is used to receive termination signals from the OS.
109111
signalsChannel chan os.Signal
110112
// asyncErrorChannel is used to signal a fatal error from any component.
111-
asyncErrorChannel chan error
113+
asyncErrorChannel chan error
114+
bc *bufferedCore
115+
updateConfigProviderLogger func(core zapcore.Core)
112116
}
113117

114118
// NewCollector creates and returns a new instance of Collector.
115119
func NewCollector(set CollectorSettings) (*Collector, error) {
116120
var err error
117121
configProvider := set.ConfigProvider
118122

119-
set.ConfigProviderSettings.ResolverSettings.ProviderSettings = confmap.ProviderSettings{Logger: zap.NewNop()}
123+
bc := newBufferedCore(zapcore.DebugLevel)
124+
cc := &collectorCore{core: bc}
125+
options := append([]zap.Option{zap.WithCaller(true)}, set.LoggingOptions...)
126+
set.ConfigProviderSettings.ResolverSettings.ProviderSettings = confmap.ProviderSettings{Logger: zap.New(cc, options...)}
120127
set.ConfigProviderSettings.ResolverSettings.ConverterSettings = confmap.ConverterSettings{}
121128

122129
if configProvider == nil {
@@ -134,9 +141,11 @@ func NewCollector(set CollectorSettings) (*Collector, error) {
134141
shutdownChan: make(chan struct{}),
135142
// Per signal.Notify documentation, a size of the channel equaled with
136143
// the number of signals getting notified on is recommended.
137-
signalsChannel: make(chan os.Signal, 3),
138-
asyncErrorChannel: make(chan error),
139-
configProvider: configProvider,
144+
signalsChannel: make(chan os.Signal, 3),
145+
asyncErrorChannel: make(chan error),
146+
configProvider: configProvider,
147+
bc: bc,
148+
updateConfigProviderLogger: cc.SetCore,
140149
}, nil
141150
}
142151

@@ -202,6 +211,18 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error {
202211
if err != nil {
203212
return err
204213
}
214+
if col.updateConfigProviderLogger != nil {
215+
col.updateConfigProviderLogger(col.service.Logger().Core())
216+
}
217+
if col.bc != nil {
218+
x := col.bc.TakeLogs()
219+
for _, log := range x {
220+
ce := col.service.Logger().Core().Check(log.Entry, nil)
221+
if ce != nil {
222+
ce.Write(log.Context...)
223+
}
224+
}
225+
}
205226

206227
if !col.set.SkipSettingGRPCLogger {
207228
grpclog.SetLogger(col.service.Logger(), cfg.Service.Telemetry.Logs.Level)
@@ -243,12 +264,40 @@ func (col *Collector) DryRun(ctx context.Context) error {
243264
return cfg.Validate()
244265
}
245266

267+
func newFallbackLogger(options []zap.Option) (*zap.Logger, error) {
268+
ec := zap.NewProductionEncoderConfig()
269+
ec.EncodeTime = zapcore.ISO8601TimeEncoder
270+
zapCfg := &zap.Config{
271+
Level: zap.NewAtomicLevelAt(zapcore.DebugLevel),
272+
Encoding: "console",
273+
EncoderConfig: ec,
274+
OutputPaths: []string{"stderr"},
275+
ErrorOutputPaths: []string{"stderr"},
276+
}
277+
return zapCfg.Build(options...)
278+
}
279+
246280
// Run starts the collector according to the given configuration, and waits for it to complete.
247281
// Consecutive calls to Run are not allowed, Run shouldn't be called once a collector is shut down.
248282
// Sets up the control logic for config reloading and shutdown.
249283
func (col *Collector) Run(ctx context.Context) error {
250284
if err := col.setupConfigurationComponents(ctx); err != nil {
251285
col.setCollectorState(StateClosed)
286+
logger, loggerErr := newFallbackLogger(col.set.LoggingOptions)
287+
if loggerErr != nil {
288+
return errors.Join(err, fmt.Errorf("unable to create fallback logger: %w", loggerErr))
289+
}
290+
291+
if col.bc != nil {
292+
x := col.bc.TakeLogs()
293+
for _, log := range x {
294+
ce := logger.Core().Check(log.Entry, nil)
295+
if ce != nil {
296+
ce.Write(log.Context...)
297+
}
298+
}
299+
}
300+
252301
return err
253302
}
254303

otelcol/collector_core.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package otelcol // import "go.opentelemetry.io/collector/otelcol"
5+
6+
import (
7+
"sync"
8+
9+
"go.uber.org/zap/zapcore"
10+
)
11+
12+
var _ zapcore.Core = (*collectorCore)(nil)
13+
14+
type collectorCore struct {
15+
core zapcore.Core
16+
rw sync.RWMutex
17+
}
18+
19+
func (c *collectorCore) Enabled(l zapcore.Level) bool {
20+
c.rw.RLock()
21+
defer c.rw.RUnlock()
22+
return c.core.Enabled(l)
23+
}
24+
25+
func (c *collectorCore) With(f []zapcore.Field) zapcore.Core {
26+
c.rw.RLock()
27+
defer c.rw.RUnlock()
28+
return &collectorCore{
29+
core: c.core.With(f),
30+
}
31+
}
32+
33+
func (c *collectorCore) Check(e zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry {
34+
c.rw.RLock()
35+
defer c.rw.RUnlock()
36+
if c.core.Enabled(e.Level) {
37+
return ce.AddCore(e, c)
38+
}
39+
return ce
40+
}
41+
42+
func (c *collectorCore) Write(e zapcore.Entry, f []zapcore.Field) error {
43+
c.rw.RLock()
44+
defer c.rw.RUnlock()
45+
return c.core.Write(e, f)
46+
}
47+
48+
func (c *collectorCore) Sync() error {
49+
c.rw.RLock()
50+
defer c.rw.RUnlock()
51+
return c.core.Sync()
52+
}
53+
54+
func (c *collectorCore) SetCore(core zapcore.Core) {
55+
c.rw.Lock()
56+
defer c.rw.Unlock()
57+
c.core = core
58+
}

0 commit comments

Comments
 (0)