Skip to content

Commit 8991b48

Browse files
Avoid allocating too much memory, sampling logic is not re-applied (#13015)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Avoid re-creating sampler counters every time we wrap with attributes. <!-- Issue number if applicable --> #### Link to tracking issue Updates #13014 <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.--> --------- Signed-off-by: Bogdan Drutu <[email protected]> Co-authored-by: Jade Guiton <[email protected]>
1 parent f846829 commit 8991b48

File tree

5 files changed

+46
-42
lines changed

5 files changed

+46
-42
lines changed

.chloggen/fix-13014.yaml

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: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: telemetry
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Allocate less memory per component when OTLP exporting of logs is disabled
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13014]
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: [user]

internal/telemetry/componentattribute/logger_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func TestCore(t *testing.T) {
9090
createLogger: func() (*zap.Logger, logRecorder) {
9191
core, observed := createZapCore()
9292
recorder := logtest.NewRecorder()
93-
core = componentattribute.NewOTelTeeCoreWithAttributes(core, recorder, "testinstr", zap.DebugLevel, attribute.NewSet())
93+
core = componentattribute.NewOTelTeeCoreWithAttributes(core, recorder, "testinstr", zap.DebugLevel, attribute.NewSet(), func(c zapcore.Core) zapcore.Core { return c })
9494
return zap.New(core), logRecorder{zapLogs: observed, otelLogs: recorder}
9595
},
9696
check: func(t *testing.T, rec logRecorder) {

internal/telemetry/componentattribute/logger_zap.go

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ type otelTeeCoreWithAttributes struct {
7676
lp log.LoggerProvider
7777
scopeName string
7878
level zapcore.Level
79+
wrapper func(zapcore.Core) zapcore.Core
7980
}
8081

8182
var _ coreWithAttributes = (*otelTeeCoreWithAttributes)(nil)
@@ -84,7 +85,7 @@ var _ coreWithAttributes = (*otelTeeCoreWithAttributes)(nil)
8485
// logs, component attributes are injected as instrumentation scope attributes.
8586
//
8687
// This is used when service::telemetry::logs::processors is configured.
87-
func NewOTelTeeCoreWithAttributes(consoleCore zapcore.Core, lp log.LoggerProvider, scopeName string, level zapcore.Level, attrs attribute.Set) zapcore.Core {
88+
func NewOTelTeeCoreWithAttributes(consoleCore zapcore.Core, lp log.LoggerProvider, scopeName string, level zapcore.Level, attrs attribute.Set, wrapper func(zapcore.Core) zapcore.Core) zapcore.Core {
8889
// TODO: Use `otelzap.WithAttributes` and remove `LoggerProviderWithAttributes`
8990
// once we've upgraded to otelzap v0.11.0.
9091
lpwa := LoggerProviderWithAttributes(lp, attrs)
@@ -97,45 +98,17 @@ func NewOTelTeeCoreWithAttributes(consoleCore zapcore.Core, lp log.LoggerProvide
9798
}
9899

99100
return &otelTeeCoreWithAttributes{
100-
Core: zapcore.NewTee(consoleCore, otelCore),
101+
Core: zapcore.NewTee(consoleCore, wrapper(otelCore)),
101102
consoleCore: consoleCore,
102103
lp: lp,
103104
scopeName: scopeName,
104105
level: level,
106+
wrapper: wrapper,
105107
}
106108
}
107109

108110
func (ocwa *otelTeeCoreWithAttributes) withAttributeSet(attrs attribute.Set) zapcore.Core {
109-
return NewOTelTeeCoreWithAttributes(
110-
tryWithAttributeSet(ocwa.consoleCore, attrs),
111-
ocwa.lp, ocwa.scopeName, ocwa.level,
112-
attrs,
113-
)
114-
}
115-
116-
type wrapperCoreWithAttributes struct {
117-
zapcore.Core
118-
from zapcore.Core
119-
wrapper func(zapcore.Core) zapcore.Core
120-
}
121-
122-
var _ coreWithAttributes = (*wrapperCoreWithAttributes)(nil)
123-
124-
// NewWrapperCoreWithAttributes applies a wrapper function to a core, similar to [zap.WrapCore]. The resulting wrapped core
125-
// allows setting component attributes on the inner core and reapplying the wrapper function when
126-
// needed.
127-
//
128-
// This is used when adding [zapcore.NewSamplerWithOptions] to our logger stack.
129-
func NewWrapperCoreWithAttributes(from zapcore.Core, wrapper func(zapcore.Core) zapcore.Core) zapcore.Core {
130-
return &wrapperCoreWithAttributes{
131-
Core: wrapper(from),
132-
from: from,
133-
wrapper: wrapper,
134-
}
135-
}
136-
137-
func (wcwa *wrapperCoreWithAttributes) withAttributeSet(attrs attribute.Set) zapcore.Core {
138-
return NewWrapperCoreWithAttributes(tryWithAttributeSet(wcwa.from, attrs), wcwa.wrapper)
111+
return NewOTelTeeCoreWithAttributes(tryWithAttributeSet(ocwa.consoleCore, attrs), ocwa.lp, ocwa.scopeName, ocwa.level, attrs, ocwa.wrapper)
139112
}
140113

141114
// ZapLoggerWithAttributes creates a Zap Logger with a new set of injected component attributes.

service/telemetry/logger.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func newLogger(set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error
4444
// We do NOT add them to the logger using With, because that would apply to all logs, even ones exported through the core that wraps
4545
// the LoggerProvider, meaning that the attributes would be exported twice.
4646
logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core {
47-
fields := []zap.Field{}
47+
var fields []zap.Field
4848
for k, v := range cfg.Resource {
4949
if v != nil {
5050
f := zap.Stringp(k, v)
@@ -56,28 +56,34 @@ func newLogger(set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error
5656
}))
5757

5858
var lp log.LoggerProvider
59-
6059
logger = logger.WithOptions(zap.WrapCore(func(core zapcore.Core) zapcore.Core {
60+
if cfg.Logs.Sampling != nil && cfg.Logs.Sampling.Enabled {
61+
core = newSampledCore(core, cfg.Logs.Sampling)
62+
}
63+
6164
core = componentattribute.NewConsoleCoreWithAttributes(core, attribute.NewSet())
6265

6366
if len(cfg.Logs.Processors) > 0 && set.SDK != nil {
6467
lp = set.SDK.LoggerProvider()
68+
wrapper := func(c zapcore.Core) zapcore.Core {
69+
return c
70+
}
71+
if cfg.Logs.Sampling != nil && cfg.Logs.Sampling.Enabled {
72+
wrapper = func(c zapcore.Core) zapcore.Core {
73+
return newSampledCore(c, cfg.Logs.Sampling)
74+
}
75+
}
6576

6677
core = componentattribute.NewOTelTeeCoreWithAttributes(
6778
core,
6879
lp,
6980
"go.opentelemetry.io/collector/service/telemetry",
7081
cfg.Logs.Level,
7182
attribute.NewSet(),
83+
wrapper,
7284
)
7385
}
7486

75-
if cfg.Logs.Sampling != nil && cfg.Logs.Sampling.Enabled {
76-
core = componentattribute.NewWrapperCoreWithAttributes(core, func(c zapcore.Core) zapcore.Core {
77-
return newSampledCore(c, cfg.Logs.Sampling)
78-
})
79-
}
80-
8187
return core
8288
}))
8389

service/telemetry/logger_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func TestNewLogger(t *testing.T) {
9898
InitialFields: map[string]any(nil),
9999
},
100100
},
101-
wantCoreType: "*componentattribute.wrapperCoreWithAttributes",
101+
wantCoreType: "*componentattribute.consoleCoreWithAttributes",
102102
},
103103
}
104104
for _, tt := range tests {

0 commit comments

Comments
 (0)