Skip to content

Commit 698bf40

Browse files
authored
Print generated resource attributes for internal logs (#13111)
#### Link to tracking issue Fixes #13110 #### Testing - Improved the unit tests. Signed-off-by: Israel Blancas <[email protected]>
1 parent 8377ee7 commit 698bf40

File tree

5 files changed

+163
-39
lines changed

5 files changed

+163
-39
lines changed

.chloggen/13110.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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: Add generated resource attributes to the printed log messages.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13110]
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+
If service.name, service.version, or service.instance.id are not specified in the config, they will be generated automatically.
20+
This change ensures that these attributes are also included in the printed log messages.
21+
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

service/service.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
181181
BuildInfo: set.BuildInfo,
182182
ZapOptions: set.LoggingOptions,
183183
SDK: &sdk,
184+
Resource: res,
184185
}
185186

186187
logger, lp, err := telFactory.CreateLogger(ctx, telset, &cfg.Telemetry)

service/telemetry/factory.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
config "go.opentelemetry.io/contrib/otelconf/v0.3.0"
1111
"go.opentelemetry.io/otel/log"
1212
"go.opentelemetry.io/otel/metric"
13+
"go.opentelemetry.io/otel/sdk/resource"
1314
"go.opentelemetry.io/otel/trace"
1415
"go.uber.org/zap"
1516
"go.uber.org/zap/zapcore"
@@ -32,6 +33,7 @@ type Settings struct {
3233
AsyncErrorChannel chan error
3334
ZapOptions []zap.Option
3435
SDK *config.SDK
36+
Resource *resource.Resource
3537
}
3638

3739
// Factory is factory interface for telemetry.

service/telemetry/logger.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,24 @@ func newLogger(set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error
3939
return nil, nil, err
4040
}
4141

42-
// The attributes in cfg.Resource are added as resource attributes for logs exported through the LoggerProvider instantiated below.
43-
// To make sure they are also exposed in logs written to stdout, we add them as fields to the Zap core created above using WrapCore.
44-
// 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
45-
// the LoggerProvider, meaning that the attributes would be exported twice.
46-
logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core {
47-
var fields []zap.Field
48-
for k, v := range cfg.Resource {
49-
if v != nil {
50-
f := zap.Stringp(k, v)
51-
fields = append(fields, f)
42+
// The attributes in set.Resource.Attributes(), which are generated in service.go, are added
43+
// as resource attributes for logs exported through the LoggerProvider instantiated below.
44+
// To make sure they are also exposed in logs written to stdout, we add them as fields to the
45+
// Zap core created above using WrapCore.
46+
// We do NOT add them to the logger using With, because that would apply to all logs, even ones
47+
// exported through the core that wraps the LoggerProvider, meaning that the attributes would
48+
// be exported twice.
49+
if set.Resource != nil && len(set.Resource.Attributes()) > 0 {
50+
logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core {
51+
var fields []zap.Field
52+
for _, attr := range set.Resource.Attributes() {
53+
fields = append(fields, zap.String(string(attr.Key), attr.Value.Emit()))
5254
}
53-
}
54-
r := zap.Dict("resource", fields...)
55-
return c.With([]zapcore.Field{r})
56-
}))
55+
56+
r := zap.Dict("resource", fields...)
57+
return c.With([]zapcore.Field{r})
58+
}))
59+
}
5760

5861
var lp log.LoggerProvider
5962
logger = logger.WithOptions(zap.WrapCore(func(core zapcore.Core) zapcore.Core {

service/telemetry/logger_test.go

Lines changed: 116 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import (
2121
"go.uber.org/zap/zapcore"
2222
"go.uber.org/zap/zaptest/observer"
2323

24+
"go.opentelemetry.io/collector/component"
2425
"go.opentelemetry.io/collector/pdata/plog/plogotlp"
26+
"go.opentelemetry.io/collector/service/internal/resource"
2527
)
2628

2729
type shutdownable interface {
@@ -126,38 +128,127 @@ func TestNewLogger(t *testing.T) {
126128
}
127129

128130
func TestNewLoggerWithResource(t *testing.T) {
129-
observerCore, observedLogs := observer.New(zap.InfoLevel)
130-
131-
set := Settings{
132-
ZapOptions: []zap.Option{
133-
zap.WrapCore(func(core zapcore.Core) zapcore.Core {
134-
// Combine original core and observer core to capture everything
135-
return zapcore.NewTee(core, observerCore)
136-
}),
131+
tests := []struct {
132+
name string
133+
buildInfo component.BuildInfo
134+
resourceConfig map[string]*string
135+
wantFields map[string]string
136+
}{
137+
{
138+
name: "auto-populated fields only",
139+
buildInfo: component.BuildInfo{
140+
Command: "mycommand",
141+
Version: "1.0.0",
142+
},
143+
resourceConfig: map[string]*string{},
144+
wantFields: map[string]string{
145+
string(semconv.ServiceNameKey): "mycommand",
146+
string(semconv.ServiceVersionKey): "1.0.0",
147+
string(semconv.ServiceInstanceIDKey): "",
148+
},
137149
},
138-
}
139-
140-
cfg := Config{
141-
Logs: LogsConfig{
142-
Level: zapcore.InfoLevel,
143-
Encoding: "json",
150+
{
151+
name: "override service.name",
152+
buildInfo: component.BuildInfo{
153+
Command: "mycommand",
154+
Version: "1.0.0",
155+
},
156+
resourceConfig: map[string]*string{
157+
string(semconv.ServiceNameKey): ptr("custom-service"),
158+
},
159+
wantFields: map[string]string{
160+
string(semconv.ServiceNameKey): "custom-service",
161+
string(semconv.ServiceVersionKey): "1.0.0",
162+
string(semconv.ServiceInstanceIDKey): "",
163+
},
144164
},
145-
Resource: map[string]*string{
146-
"myfield": ptr("myvalue"),
165+
{
166+
name: "override service.version",
167+
buildInfo: component.BuildInfo{
168+
Command: "mycommand",
169+
Version: "1.0.0",
170+
},
171+
resourceConfig: map[string]*string{
172+
string(semconv.ServiceVersionKey): ptr("2.0.0"),
173+
},
174+
wantFields: map[string]string{
175+
string(semconv.ServiceNameKey): "mycommand",
176+
string(semconv.ServiceVersionKey): "2.0.0",
177+
string(semconv.ServiceInstanceIDKey): "",
178+
},
179+
},
180+
{
181+
name: "custom field with auto-populated",
182+
buildInfo: component.BuildInfo{
183+
Command: "mycommand",
184+
Version: "1.0.0",
185+
},
186+
resourceConfig: map[string]*string{
187+
"custom.field": ptr("custom-value"),
188+
},
189+
wantFields: map[string]string{
190+
string(semconv.ServiceNameKey): "mycommand",
191+
string(semconv.ServiceVersionKey): "1.0.0",
192+
string(semconv.ServiceInstanceIDKey): "", // Just check presence
193+
"custom.field": "custom-value",
194+
},
195+
},
196+
{
197+
name: "resource with no attributes",
198+
buildInfo: component.BuildInfo{},
199+
resourceConfig: nil,
200+
wantFields: nil,
147201
},
148202
}
149203

150-
mylogger, _, _ := newLogger(set, cfg)
204+
for _, tt := range tests {
205+
t.Run(tt.name, func(t *testing.T) {
206+
observerCore, observedLogs := observer.New(zap.InfoLevel)
151207

152-
mylogger.Info("Test log message")
153-
require.Len(t, observedLogs.All(), 1)
208+
set := Settings{
209+
ZapOptions: []zap.Option{
210+
zap.WrapCore(func(core zapcore.Core) zapcore.Core {
211+
return zapcore.NewTee(core, observerCore)
212+
}),
213+
},
214+
}
215+
if tt.wantFields != nil {
216+
set.Resource = resource.New(tt.buildInfo, tt.resourceConfig)
217+
}
154218

155-
entry := observedLogs.All()[0]
156-
assert.Equal(t, "resource", entry.Context[0].Key)
157-
dict := entry.Context[0].Interface.(zapcore.ObjectMarshaler)
158-
enc := zapcore.NewMapObjectEncoder()
159-
require.NoError(t, dict.MarshalLogObject(enc))
160-
require.Equal(t, "myvalue", enc.Fields["myfield"])
219+
cfg := Config{
220+
Logs: LogsConfig{
221+
Level: zapcore.InfoLevel,
222+
Encoding: "json",
223+
},
224+
}
225+
226+
mylogger, _, _ := newLogger(set, cfg)
227+
mylogger.Info("Test log message")
228+
require.Len(t, observedLogs.All(), 1)
229+
230+
entry := observedLogs.All()[0]
231+
if tt.wantFields == nil {
232+
assert.Empty(t, entry.Context)
233+
return
234+
}
235+
236+
assert.Equal(t, "resource", entry.Context[0].Key)
237+
dict := entry.Context[0].Interface.(zapcore.ObjectMarshaler)
238+
enc := zapcore.NewMapObjectEncoder()
239+
require.NoError(t, dict.MarshalLogObject(enc))
240+
241+
// Verify all expected fields
242+
for k, v := range tt.wantFields {
243+
if k == string(semconv.ServiceInstanceIDKey) {
244+
// For service.instance.id just verify it exists since it's auto-generated
245+
assert.Contains(t, enc.Fields, k)
246+
} else {
247+
assert.Equal(t, v, enc.Fields[k])
248+
}
249+
}
250+
})
251+
}
161252
}
162253

163254
func TestOTLPLogExport(t *testing.T) {

0 commit comments

Comments
 (0)