Skip to content

Commit

Permalink
Correctly handle nil value on the config provider (open-telemetry#434)
Browse files Browse the repository at this point in the history
Handling of nil retrieved values by the config provider manager was causing the string "<nil>" to be used. Fixed the handling on the manager and removed the special case on the env variable.
  • Loading branch information
pjanotti authored Jun 1, 2021
1 parent 9462292 commit 9977788
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 6 deletions.
9 changes: 7 additions & 2 deletions internal/configprovider/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,13 @@ func (m *Manager) expandString(ctx context.Context, s string) (interface{}, erro
return retrieved, nil
}

// Either there was a prefix already or there are still
// characters to be processed.
// Either there was a prefix already or there are still characters to be processed.
if retrieved == nil {
// Since this is going to be concatenated to a string use "" instead of nil,
// otherwise the string will end up with "<nil>".
retrieved = ""
}

buf = append(buf, fmt.Sprintf("%v", retrieved)...)
}

Expand Down
10 changes: 10 additions & 0 deletions internal/configprovider/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ func TestManager_expandString(t *testing.T) {
ValueMap: map[string]valueEntry{
"str_key": {Value: "test_value"},
"int_key": {Value: 1},
"nil_key": {Value: nil},
},
},
"tstcfgsrc/named": &testConfigSource{
Expand Down Expand Up @@ -586,6 +587,15 @@ func TestManager_expandString(t *testing.T) {
input: "$envvar/test:test",
wantErr: &errUnknownConfigSource{},
},
{
name: "retrieved_nil",
input: "${tstcfgsrc:nil_key}",
},
{
name: "retrieved_nil_on_string",
input: "prefix-${tstcfgsrc:nil_key}-suffix",
want: "prefix--suffix",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
3 changes: 0 additions & 3 deletions internal/configsource/envvarconfigsource/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ func (e *envVarSession) Retrieve(_ context.Context, selector string, params inte
if !actualParams.Optional {
return nil, &errMissingRequiredEnvVar{fmt.Errorf("env var %q is required but not defined and not present on defaults", selector)}
}

// To keep with default behavior for env vars not defined set the value to empty string
defaultValue = ""
}

return configprovider.NewRetrieved(defaultValue, configprovider.WatcherNotSupported), nil
Expand Down
2 changes: 1 addition & 1 deletion internal/configsource/envvarconfigsource/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestEnvVarConfigSource_Session(t *testing.T) {
params: map[string]interface{}{
"optional": true,
},
expected: "", // The default behavior for undefined env var is empty string.
expected: nil,
},
{
name: "invalid_param",
Expand Down

0 comments on commit 9977788

Please sign in to comment.