Skip to content

Commit bf0272e

Browse files
authored
Optionally suppress warnings about unreferenced NULL values (#46660)
#### Description This enhancement to NULL value handling allows for cleaner logs when a query cannot be formulated to hide NULL values. For example, the SQL interface to pgBouncer is a handy way to monitor that service, but the minimal SQL parsing of the interface does not allow for a subset of columns to be returned. As a result, NULL values are common in the returned rows, even if they will not be used for monitoring. #### Link to tracking issue Enhances #43985 #### Testing Unit testing showing that the existence of ignore_null_values=true suppresses warnings, unless the null values are in a referenced column. Likewise, tests that show the default value, or explicitly setting the value to false, keeps the warnings around. Errors always remain when a columned holding NULL values is referenced. #### Documentation Documented the new option, and referenced it when talking about NULL values in README.md.
1 parent b61f91e commit bf0272e

8 files changed

Lines changed: 209 additions & 37 deletions

File tree

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: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: receiver/sqlquery
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Add `ignore_null_values` config option to suppress NULL value warning logs
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [43985]
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+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
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: [user]

internal/sqlquery/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ type Query struct {
8888
Logs []LogsCfg `mapstructure:"logs"`
8989
TrackingColumn string `mapstructure:"tracking_column"`
9090
TrackingStartValue string `mapstructure:"tracking_start_value"`
91+
IgnoreNullValues bool `mapstructure:"ignore_null_values"`
9192
}
9293

9394
func (q Query) Validate() error {

internal/sqlquery/config.schema.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ $defs:
8484
query:
8585
type: object
8686
properties:
87+
ignore_null_values:
88+
type: boolean
8789
logs:
8890
type: array
8991
items:

internal/sqlquery/scraper.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ func (s *Scraper) ScrapeMetrics(ctx context.Context) (pmetric.Metrics, error) {
7979
if !errors.Is(err, ErrNullValueWarning) {
8080
return out, fmt.Errorf("scraper: %w", err)
8181
}
82-
s.Logger.Warn("problems encountered getting metric rows", zap.Error(err))
82+
if !s.Query.IgnoreNullValues {
83+
s.Logger.Warn("problems encountered getting metric rows", zap.Error(err))
84+
}
8385
}
8486
ts := pcommon.NewTimestampFromTime(time.Now())
8587
rms := out.ResourceMetrics()

internal/sqlquery/scraper_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"go.opentelemetry.io/collector/pdata/pmetric"
1717
"go.opentelemetry.io/collector/scraper/scrapererror"
1818
"go.uber.org/zap"
19+
"go.uber.org/zap/zaptest/observer"
1920
)
2021

2122
func TestScraper_ErrorOnStart(t *testing.T) {
@@ -362,6 +363,85 @@ func TestScraper_FakeDB_Warnings(t *testing.T) {
362363
require.NoError(t, err)
363364
}
364365

366+
func TestScraper_FakeDB_UnreferencedNullColumnWarning(t *testing.T) {
367+
// An unreferenced NULL column (col_1) should only produce a warning when
368+
// IgnoreNullValues is false (or unset). When true, no warning is logged.
369+
tests := []struct {
370+
name string
371+
ignoreNullValues bool
372+
expectWarning bool
373+
}{
374+
{name: "default", ignoreNullValues: false, expectWarning: true},
375+
{name: "explicit_false", ignoreNullValues: false, expectWarning: true},
376+
{name: "true", ignoreNullValues: true, expectWarning: false},
377+
}
378+
for _, tt := range tests {
379+
t.Run(tt.name, func(t *testing.T) {
380+
db := fakeDB{rowVals: [][]any{{42, nil}}}
381+
core, recorded := observer.New(zap.WarnLevel)
382+
logger := zap.New(core)
383+
scrpr := Scraper{
384+
InstrumentationScope: pcommon.NewInstrumentationScope(),
385+
Client: NewDbClient(db, "", logger, TelemetryConfig{}),
386+
Logger: logger,
387+
Query: Query{
388+
IgnoreNullValues: tt.ignoreNullValues,
389+
Metrics: []MetricCfg{{
390+
MetricName: "my.name",
391+
ValueColumn: "col_0",
392+
Description: "my description",
393+
Unit: "my-unit",
394+
}},
395+
},
396+
}
397+
_, err := scrpr.ScrapeMetrics(t.Context())
398+
require.NoError(t, err)
399+
if tt.expectWarning {
400+
all := recorded.All()
401+
require.Len(t, all, 1)
402+
assert.Equal(t, "problems encountered getting metric rows", all[0].Message)
403+
} else {
404+
assert.Empty(t, recorded.All(), "expected no warnings when IgnoreNullValues is true")
405+
}
406+
})
407+
}
408+
}
409+
410+
func TestScraper_FakeDB_Warnings_ReferencedNullColumnStillErrors(t *testing.T) {
411+
// A PartialScrapeError must be returned when a referenced column
412+
// (ValueColumn) is NULL, regardless of the IgnoreNullValues setting.
413+
tests := []struct {
414+
name string
415+
ignoreNullValues bool
416+
}{
417+
{name: "default", ignoreNullValues: false},
418+
{name: "explicit_false", ignoreNullValues: false},
419+
{name: "true", ignoreNullValues: true},
420+
}
421+
for _, tt := range tests {
422+
t.Run(tt.name, func(t *testing.T) {
423+
db := fakeDB{rowVals: [][]any{{nil, "ok"}}}
424+
scrpr := Scraper{
425+
InstrumentationScope: pcommon.NewInstrumentationScope(),
426+
Client: NewDbClient(db, "", zap.NewNop(), TelemetryConfig{}),
427+
Logger: zap.NewNop(),
428+
Query: Query{
429+
IgnoreNullValues: tt.ignoreNullValues,
430+
Metrics: []MetricCfg{{
431+
MetricName: "my.name",
432+
ValueColumn: "col_0",
433+
AttributeColumns: []string{"col_1"},
434+
}},
435+
},
436+
}
437+
_, err := scrpr.ScrapeMetrics(t.Context())
438+
require.Error(t, err)
439+
assert.True(t, scrapererror.IsPartialScrapeError(err))
440+
assert.ErrorContains(t, err, "value_column 'col_0' not found in result set")
441+
})
442+
}
443+
}
444+
365445
func TestScraper_FakeDB_MultiRows_Warnings(t *testing.T) {
366446
db := fakeDB{rowVals: [][]any{{42, nil}, {43, nil}}}
367447
logger := zap.NewNop()

receiver/sqlqueryreceiver/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ Additionally, each `query` section supports the following properties:
8282
See the below section [Tracking processed results](#tracking-processed-results).
8383
- `tracking_start_value` (optional, default `""`) Applies only to logs. In case of a parameterized query, defines the initial value for the parameter.
8484
See the below section [Tracking processed results](#tracking-processed-results).
85+
- `ignore_null_values` (optional, default `false`) When set to `true`, suppresses warning logs about NULL values encountered in query result columns. This is useful when queries return NULL in columns that are not referenced in the metric or log configuration.
8586
- `attribute_columns`(optional): a list of column names in the returned dataset used to set attributes on the signal.
8687
These attributes may be case-sensitive, depending on the driver (e.g. Oracle DB).
8788

@@ -314,8 +315,8 @@ This produces three separate metrics (`pgbouncer.lists.pools`, `pgbouncer.lists.
314315

315316
#### NULL values
316317

317-
Avoid queries that produce any NULL values. If a query produces a NULL value, a warning will be logged. Furthermore,
318-
if a configuration references the column that produces a NULL value, an additional error will be logged. However, in
318+
If a query produces a NULL value, a warning will be logged unless `ignore_null_values` is set to true. Furthermore,
319+
if a configuration references the column that produces a NULL value, an error will always be logged. However, in
319320
either case, the receiver will continue to operate.
320321

321322
#### Oracle DB Driver Example

receiver/sqlqueryreceiver/logs_receiver.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,9 @@ func (queryReceiver *logsQueryReceiver) collect(ctx context.Context) (plog.Logs,
304304
if !errors.Is(err, sqlquery.ErrNullValueWarning) {
305305
return logs, fmt.Errorf("scraper: %w", err)
306306
}
307-
queryReceiver.logger.Warn("problems encountered getting log rows", zap.Error(err))
307+
if !queryReceiver.query.IgnoreNullValues {
308+
queryReceiver.logger.Warn("problems encountered getting log rows", zap.Error(err))
309+
}
308310
}
309311

310312
var errs []error

receiver/sqlqueryreceiver/logs_receiver_test.go

Lines changed: 90 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -120,44 +120,101 @@ func TestLogsQueryReceiver_BothDatasourceFields(t *testing.T) {
120120
require.NoError(t, receiver.Shutdown(ctx))
121121
}
122122

123-
func TestLogsQueryReceiver_NullValue(t *testing.T) {
124-
col1 := "col1"
125-
col1Value := "42"
126-
fakeClient := &sqlquery.FakeDBClient{
127-
StringMaps: [][]sqlquery.StringMap{
128-
{{col1: col1Value}},
129-
},
130-
// fakeClient.QueryRows will return ErrNullValueWarning on top of the StringMaps
131-
ErrNullValueWarning: true,
123+
func TestLogsQueryReceiver_UnreferencedNullColumnWarning(t *testing.T) {
124+
// An unreferenced NULL column should only produce a warning when
125+
// IgnoreNullValues is false (or unset). When true, no warning is logged.
126+
// In all cases the log record itself is collected successfully.
127+
tests := []struct {
128+
name string
129+
ignoreNullValues bool
130+
expectWarning bool
131+
}{
132+
{name: "default", ignoreNullValues: false, expectWarning: true},
133+
{name: "explicit_false", ignoreNullValues: false, expectWarning: true},
134+
{name: "true", ignoreNullValues: true, expectWarning: false},
132135
}
136+
for _, tt := range tests {
137+
t.Run(tt.name, func(t *testing.T) {
138+
col1 := "col1"
139+
col1Value := "42"
140+
fakeClient := &sqlquery.FakeDBClient{
141+
StringMaps: [][]sqlquery.StringMap{
142+
{{col1: col1Value}},
143+
},
144+
// fakeClient.QueryRows will return ErrNullValueWarning on top of the StringMaps
145+
ErrNullValueWarning: true,
146+
}
133147

134-
core, recorded := observer.New(zap.InfoLevel)
135-
logger := zap.New(core)
148+
core, recorded := observer.New(zap.WarnLevel)
149+
logger := zap.New(core)
136150

137-
queryReceiver := logsQueryReceiver{
138-
client: fakeClient,
139-
query: sqlquery.Query{
140-
Logs: []sqlquery.LogsCfg{
141-
{
142-
BodyColumn: col1,
143-
AttributeColumns: []string{col1},
151+
queryReceiver := logsQueryReceiver{
152+
client: fakeClient,
153+
query: sqlquery.Query{
154+
IgnoreNullValues: tt.ignoreNullValues,
155+
Logs: []sqlquery.LogsCfg{
156+
{
157+
BodyColumn: col1,
158+
AttributeColumns: []string{col1},
159+
},
160+
},
144161
},
145-
},
146-
},
147-
logger: logger,
162+
logger: logger,
163+
}
164+
logs, err := queryReceiver.collect(t.Context())
165+
assert.NoError(t, err)
166+
assert.Equal(t, 1, logs.LogRecordCount())
167+
assert.Equal(t, col1Value, logs.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().At(0).Body().Str())
168+
169+
if tt.expectWarning {
170+
all := recorded.All()
171+
require.Len(t, all, 1)
172+
assert.Equal(t, "problems encountered getting log rows", all[0].Message)
173+
assert.Equal(t, sqlquery.ErrNullValueWarning.Error(), all[0].ContextMap()["error"])
174+
} else {
175+
assert.Empty(t, recorded.All(), "expected no warnings when IgnoreNullValues is true")
176+
}
177+
})
178+
}
179+
}
180+
181+
func TestLogsQueryReceiver_NullValue_ReferencedNullColumnStillErrors(t *testing.T) {
182+
// An error must be returned when a referenced column (BodyColumn) is NULL,
183+
// regardless of the IgnoreNullValues setting.
184+
tests := []struct {
185+
name string
186+
ignoreNullValues bool
187+
}{
188+
{name: "default", ignoreNullValues: false},
189+
{name: "explicit_false", ignoreNullValues: false},
190+
{name: "true", ignoreNullValues: true},
191+
}
192+
for _, tt := range tests {
193+
t.Run(tt.name, func(t *testing.T) {
194+
fakeClient := &sqlquery.FakeDBClient{
195+
StringMaps: [][]sqlquery.StringMap{
196+
{{"other_col": "val"}}, // BodyColumn "missing_col" is absent
197+
},
198+
ErrNullValueWarning: true,
199+
}
200+
201+
queryReceiver := logsQueryReceiver{
202+
client: fakeClient,
203+
query: sqlquery.Query{
204+
IgnoreNullValues: tt.ignoreNullValues,
205+
Logs: []sqlquery.LogsCfg{
206+
{
207+
BodyColumn: "missing_col",
208+
},
209+
},
210+
},
211+
logger: zap.NewNop(),
212+
}
213+
_, err := queryReceiver.collect(t.Context())
214+
require.Error(t, err)
215+
assert.ErrorContains(t, err, "body_column 'missing_col' not found in result set")
216+
})
148217
}
149-
// ensure that the logs are collected successfully
150-
logs, err := queryReceiver.collect(t.Context())
151-
assert.NoError(t, err)
152-
assert.Equal(t, 1, logs.LogRecordCount())
153-
assert.Equal(t, col1Value, logs.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().At(0).Body().Str())
154-
155-
// ensure that the warning is logged
156-
all := recorded.All()
157-
require.Len(t, all, 1)
158-
entry := all[0]
159-
require.Equal(t, "problems encountered getting log rows", entry.Message)
160-
require.Equal(t, sqlquery.ErrNullValueWarning.Error(), entry.ContextMap()["error"])
161218
}
162219

163220
func TestLogsReceiver_InitialDelay(t *testing.T) {

0 commit comments

Comments
 (0)