Skip to content

Commit 213facd

Browse files
gnak-yarChrsMark
authored and
mashhurs
committed
[chore][pkg/stanza] Issue better error messages in case of misconfiguration (open-telemetry#42924)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The container parser's `add_metadata_from_filepath` feature requires `include_file_path` to be enabled([docs](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/stanza/docs/operators/container.md#add-metadata-from-file-path)). When `include_file_path` is disabled, users encounter an error message, `type '<nil>' cannot be parsed as log path field`, which doesn't clearly indicate the misconfiguration. This change provides users a better explanation of the error. Ideally, this misconfiguration should be detected during initialization to fail fast. However, it may require significant changes since operator configurations (file input, container parser, etc.) are separated and highly abstracted. It seems like this approach is the simplest solution for now. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue open-telemetry#42859 <!--Describe what testing was performed and which tests were added.--> #### Testing Added a unit test for the case. --------- Co-authored-by: Christos Markou <[email protected]>
1 parent e517585 commit 213facd

File tree

2 files changed

+127
-83
lines changed

2 files changed

+127
-83
lines changed

‎pkg/stanza/operator/parser/container/parser.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,14 @@ func (p *Parser) extractk8sMetaFromFilePath(e *entry.Entry) error {
272272
return nil
273273
}
274274

275-
logPath := e.Attributes[logPathField]
275+
logPath, ok := e.Attributes[logPathField]
276+
if !ok {
277+
return fmt.Errorf(
278+
"operator '%s' has 'add_metadata_from_filepath' enabled, but the log record attribute '%s' is missing. Perhaps enable the 'include_file_path' option?",
279+
p.OperatorID,
280+
logPathField)
281+
}
282+
276283
rawLogPath, ok := logPath.(string)
277284
if !ok {
278285
return fmt.Errorf("type '%T' cannot be parsed as log path field", logPath)

‎pkg/stanza/operator/parser/container/parser_test.go‎

Lines changed: 119 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -88,95 +88,132 @@ func TestInternalRecombineCfg(t *testing.T) {
8888
}
8989

9090
func TestProcess(t *testing.T) {
91-
cases := []struct {
92-
name string
93-
op func() (operator.Operator, error)
94-
input *entry.Entry
95-
expect *entry.Entry
96-
}{
97-
{
98-
"docker",
99-
func() (operator.Operator, error) {
100-
cfg := NewConfigWithID("test_id")
101-
cfg.AddMetadataFromFilePath = false
102-
cfg.Format = "docker"
103-
set := componenttest.NewNopTelemetrySettings()
104-
return cfg.Build(set)
105-
},
106-
&entry.Entry{
107-
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
108-
},
109-
&entry.Entry{
110-
Attributes: map[string]any{
111-
"log.iostream": "stdout",
91+
t.Run("Success", func(t *testing.T) {
92+
cases := []struct {
93+
name string
94+
op func() (operator.Operator, error)
95+
input *entry.Entry
96+
expect *entry.Entry
97+
}{
98+
{
99+
"docker",
100+
func() (operator.Operator, error) {
101+
cfg := NewConfigWithID("test_id")
102+
cfg.AddMetadataFromFilePath = false
103+
cfg.Format = "docker"
104+
set := componenttest.NewNopTelemetrySettings()
105+
return cfg.Build(set)
106+
},
107+
&entry.Entry{
108+
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
109+
},
110+
&entry.Entry{
111+
Attributes: map[string]any{
112+
"log.iostream": "stdout",
113+
},
114+
Body: "INFO: log line here",
115+
Timestamp: time.Date(2029, time.March, 30, 8, 31, 20, 545192187, time.UTC),
112116
},
113-
Body: "INFO: log line here",
114-
Timestamp: time.Date(2029, time.March, 30, 8, 31, 20, 545192187, time.UTC),
115-
},
116-
},
117-
{
118-
"docker_with_auto_detection",
119-
func() (operator.Operator, error) {
120-
cfg := NewConfigWithID("test_id")
121-
cfg.AddMetadataFromFilePath = false
122-
set := componenttest.NewNopTelemetrySettings()
123-
return cfg.Build(set)
124-
},
125-
&entry.Entry{
126-
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
127117
},
128-
&entry.Entry{
129-
Attributes: map[string]any{
130-
"log.iostream": "stdout",
118+
{
119+
"docker_with_auto_detection",
120+
func() (operator.Operator, error) {
121+
cfg := NewConfigWithID("test_id")
122+
cfg.AddMetadataFromFilePath = false
123+
set := componenttest.NewNopTelemetrySettings()
124+
return cfg.Build(set)
131125
},
132-
Body: "INFO: log line here",
133-
Timestamp: time.Date(2029, time.March, 30, 8, 31, 20, 545192187, time.UTC),
134-
},
135-
},
136-
{
137-
"docker_with_auto_detection_and_metadata_from_file_path",
138-
func() (operator.Operator, error) {
139-
cfg := NewConfigWithID("test_id")
140-
cfg.AddMetadataFromFilePath = true
141-
set := componenttest.NewNopTelemetrySettings()
142-
return cfg.Build(set)
143-
},
144-
&entry.Entry{
145-
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
146-
Attributes: map[string]any{
147-
attrs.LogFilePath: "/var/log/pods/some_kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log",
126+
&entry.Entry{
127+
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
128+
},
129+
&entry.Entry{
130+
Attributes: map[string]any{
131+
"log.iostream": "stdout",
132+
},
133+
Body: "INFO: log line here",
134+
Timestamp: time.Date(2029, time.March, 30, 8, 31, 20, 545192187, time.UTC),
148135
},
149136
},
150-
&entry.Entry{
151-
Attributes: map[string]any{
152-
"log.iostream": "stdout",
153-
attrs.LogFilePath: "/var/log/pods/some_kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log",
137+
{
138+
"docker_with_auto_detection_and_metadata_from_file_path",
139+
func() (operator.Operator, error) {
140+
cfg := NewConfigWithID("test_id")
141+
cfg.AddMetadataFromFilePath = true
142+
set := componenttest.NewNopTelemetrySettings()
143+
return cfg.Build(set)
154144
},
155-
Body: "INFO: log line here",
156-
Resource: map[string]any{
157-
"k8s.pod.name": "kube-scheduler-kind-control-plane",
158-
"k8s.pod.uid": "49cc7c1fd3702c40b2686ea7486091d3",
159-
"k8s.container.name": "kube-scheduler44",
160-
"k8s.container.restart_count": "1",
161-
"k8s.namespace.name": "some",
145+
&entry.Entry{
146+
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
147+
Attributes: map[string]any{
148+
attrs.LogFilePath: "/var/log/pods/some_kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log",
149+
},
162150
},
163-
Timestamp: time.Date(2029, time.March, 30, 8, 31, 20, 545192187, time.UTC),
164-
},
165-
},
166-
}
167-
168-
for _, tc := range cases {
169-
t.Run(tc.name, func(t *testing.T) {
170-
op, err := tc.op()
171-
require.NoError(t, err, "did not expect operator function to return an error, this is a bug with the test case")
172-
173-
err = op.Process(t.Context(), tc.input)
174-
require.NoError(t, err)
175-
require.Equal(t, tc.expect, tc.input)
176-
// Stop the operator
177-
require.NoError(t, op.Stop())
178-
})
179-
}
151+
&entry.Entry{
152+
Attributes: map[string]any{
153+
"log.iostream": "stdout",
154+
attrs.LogFilePath: "/var/log/pods/some_kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log",
155+
},
156+
Body: "INFO: log line here",
157+
Resource: map[string]any{
158+
"k8s.pod.name": "kube-scheduler-kind-control-plane",
159+
"k8s.pod.uid": "49cc7c1fd3702c40b2686ea7486091d3",
160+
"k8s.container.name": "kube-scheduler44",
161+
"k8s.container.restart_count": "1",
162+
"k8s.namespace.name": "some",
163+
},
164+
Timestamp: time.Date(2029, time.March, 30, 8, 31, 20, 545192187, time.UTC),
165+
},
166+
},
167+
}
168+
169+
for _, tc := range cases {
170+
t.Run(tc.name, func(t *testing.T) {
171+
op, err := tc.op()
172+
require.NoError(t, err, "did not expect operator function to return an error, this is a bug with the test case")
173+
174+
err = op.Process(t.Context(), tc.input)
175+
require.NoError(t, err)
176+
require.Equal(t, tc.expect, tc.input)
177+
// Stop the operator
178+
require.NoError(t, op.Stop())
179+
})
180+
}
181+
})
182+
183+
t.Run("Failure", func(t *testing.T) {
184+
cases := []struct {
185+
name string
186+
op func() (operator.Operator, error)
187+
input *entry.Entry
188+
expectedErrMsg string
189+
}{
190+
{
191+
"docker_with_add_metadata_from_filepath_but_not_included",
192+
func() (operator.Operator, error) {
193+
cfg := NewConfigWithID("test_id")
194+
cfg.AddMetadataFromFilePath = true
195+
cfg.Format = "docker"
196+
set := componenttest.NewNopTelemetrySettings()
197+
return cfg.Build(set)
198+
},
199+
&entry.Entry{
200+
Body: `{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}`,
201+
},
202+
"operator 'test_id' has 'add_metadata_from_filepath' enabled, but the log record attribute 'log.file.path' is missing. Perhaps enable the 'include_file_path' option?",
203+
},
204+
}
205+
206+
for _, tc := range cases {
207+
t.Run(tc.name, func(t *testing.T) {
208+
op, err := tc.op()
209+
require.NoError(t, err)
210+
211+
err = op.Process(t.Context(), tc.input)
212+
require.ErrorContains(t, err, tc.expectedErrMsg)
213+
require.NoError(t, op.Stop())
214+
})
215+
}
216+
})
180217
}
181218

182219
func TestRecombineProcess(t *testing.T) {

0 commit comments

Comments
 (0)