Skip to content

Commit 8156aed

Browse files
pjanottiatoulme
authored andcommitted
Add datasource, aka "connection string", option to sqlserverreceiver (open-telemetry#39235)
#### Description Adds an option, `datasource`, that allows full control of the connection string used when connecting to the DB. There are many possible variations that are useful that can be specified via this option, e.g.: one could enable Windows authentication and have the collector run with an account configured to access the SQL server. #### Testing Covered the new option in an extended validation test and used it to connect to a SQL server using Windows credentials. #### Documentation Added a note about the new option. --------- Co-authored-by: Antoine Toulme <[email protected]>
1 parent 1112037 commit 8156aed

File tree

7 files changed

+85
-20
lines changed

7 files changed

+85
-20
lines changed
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. filelogreceiver)
7+
component: sqlserverreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Allow full control of the "connection string" via the `datasource` configuration option
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: [39235]
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]

receiver/sqlserverreceiver/README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ Direct connection options (optional, but all must be specified to enable):
3434
- `server`: IP Address or hostname of SQL Server instance to connect to.
3535
- `port`: Port of the SQL Server instance to connect to.
3636

37+
For finer control over the direct connection use the `datasource`, a.k.a. the "connection string", instead.
38+
Note: it can't be used in conjunction with the `username`, `password`, `server` and `port` options.
39+
3740
Windows-specific options:
3841
- `computer_name` (optional): The computer name identifies the SQL Server name or IP address of the computer being monitored.
3942
If specified, `instance_name` is also required to be defined. This option is ignored in non-Windows environments.

receiver/sqlserverreceiver/config.go

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,16 @@ type Config struct {
4545
InstanceName string `mapstructure:"instance_name"`
4646
ComputerName string `mapstructure:"computer_name"`
4747

48-
// The following options currently do nothing. Functionality will be added in a future PR.
48+
DataSource string `mapstructure:"datasource"`
49+
4950
Password configopaque.String `mapstructure:"password"`
5051
Port uint `mapstructure:"port"`
5152
Server string `mapstructure:"server"`
5253
Username string `mapstructure:"username"`
54+
55+
// Flag to check if the connection is direct or not. It should only be
56+
// used after a successful call to the `Validate` method.
57+
isDirectDBConnectionEnabled bool
5358
}
5459

5560
func (cfg *Config) Validate() error {
@@ -66,12 +71,27 @@ func (cfg *Config) Validate() error {
6671
return errors.New("`top_query_count` must be less than or equal to `max_query_sample_count`")
6772
}
6873

69-
if !directDBConnectionEnabled(cfg) {
70-
if cfg.Server != "" || cfg.Username != "" || string(cfg.Password) != "" {
71-
return errors.New("Found one or more of the following configuration options set: [server, port, username, password]. " +
72-
"All of these options must be configured to directly connect to a SQL Server instance.")
73-
}
74+
cfg.isDirectDBConnectionEnabled, err = directDBConnectionEnabled(cfg)
75+
76+
return err
77+
}
78+
79+
func directDBConnectionEnabled(config *Config) (bool, error) {
80+
noneOfServerUserPasswordPortSet := config.Server == "" && config.Username == "" && string(config.Password) == "" && config.Port == 0
81+
if config.DataSource == "" && noneOfServerUserPasswordPortSet {
82+
// If no connection information is provided, we can't connect directly and this is a valid config.
83+
return false, nil
84+
}
85+
86+
anyOfServerUserPasswordPortSet := config.Server != "" || config.Username != "" || string(config.Password) != "" || config.Port != 0
87+
if config.DataSource != "" && anyOfServerUserPasswordPortSet {
88+
return false, errors.New("wrong config: when specifying 'datasource' no other connection parameters ('server', 'username', 'password', or 'port') should be set")
89+
}
90+
91+
if config.DataSource == "" && (config.Server == "" || config.Username == "" || string(config.Password) == "" || config.Port == 0) {
92+
return false, errors.New("wrong config: when specifying either 'server', 'username', 'password', or 'port' all of them need to be specified")
7493
}
7594

76-
return nil
95+
// It is a valid direct connection configuration
96+
return true, nil
7797
}

receiver/sqlserverreceiver/config_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,24 @@ func TestValidate(t *testing.T) {
5353
},
5454
expectedSuccess: false,
5555
},
56+
{
57+
desc: "invalid config with datasource and any direct connect settings",
58+
cfg: &Config{
59+
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
60+
DataSource: "a connection string",
61+
Username: "sa",
62+
Port: 1433,
63+
},
64+
expectedSuccess: false,
65+
},
66+
{
67+
desc: "valid config only datasource and none direct connect settings",
68+
cfg: &Config{
69+
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
70+
DataSource: "a connection string",
71+
},
72+
expectedSuccess: true,
73+
},
5674
{
5775
desc: "valid config with all direct connection settings",
5876
cfg: &Config{
@@ -160,7 +178,7 @@ func TestLoadConfig(t *testing.T) {
160178
require.NoError(t, sub.Unmarshal(cfg))
161179

162180
assert.NoError(t, xconfmap.Validate(cfg))
163-
if diff := cmp.Diff(expected, cfg, cmpopts.IgnoreUnexported(metadata.MetricConfig{}), cmpopts.IgnoreUnexported(metadata.ResourceAttributeConfig{})); diff != "" {
181+
if diff := cmp.Diff(expected, cfg, cmpopts.IgnoreUnexported(Config{}), cmpopts.IgnoreUnexported(metadata.MetricConfig{}), cmpopts.IgnoreUnexported(metadata.ResourceAttributeConfig{})); diff != "" {
164182
t.Errorf("Config mismatch (-expected +actual):\n%s", diff)
165183
}
166184
})

receiver/sqlserverreceiver/factory.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,20 +94,17 @@ func setupLogQueries(cfg *Config) []string {
9494
return queries
9595
}
9696

97-
func directDBConnectionEnabled(config *Config) bool {
98-
return config.Server != "" &&
99-
config.Username != "" &&
100-
string(config.Password) != ""
101-
}
102-
10397
// Assumes config has all information necessary to directly connect to the database
10498
func getDBConnectionString(config *Config) string {
99+
if config.DataSource != "" {
100+
return config.DataSource
101+
}
105102
return fmt.Sprintf("server=%s;user id=%s;password=%s;port=%d", config.Server, config.Username, string(config.Password), config.Port)
106103
}
107104

108105
// SQL Server scraper creation is split out into a separate method for the sake of testing.
109106
func setupSQLServerScrapers(params receiver.Settings, cfg *Config) []*sqlServerScraperHelper {
110-
if !directDBConnectionEnabled(cfg) {
107+
if !cfg.isDirectDBConnectionEnabled {
111108
params.Logger.Info("No direct connection will be made to the SQL Server: Configuration doesn't include some options.")
112109
return nil
113110
}
@@ -147,7 +144,7 @@ func setupSQLServerScrapers(params receiver.Settings, cfg *Config) []*sqlServerS
147144

148145
// SQL Server scraper creation is split out into a separate method for the sake of testing.
149146
func setupSQLServerLogsScrapers(params receiver.Settings, cfg *Config) []*sqlServerScraperHelper {
150-
if !directDBConnectionEnabled(cfg) {
147+
if !cfg.isDirectDBConnectionEnabled {
151148
params.Logger.Info("No direct connection will be made to the SQL Server: Configuration doesn't include some options.")
152149
return nil
153150
}

receiver/sqlserverreceiver/factory_others_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestFactoryOtherOS(t *testing.T) {
3535
cfg.Metrics.SqlserverDatabaseLatency.Enabled = true
3636
require.NoError(t, cfg.Validate())
3737

38-
require.True(t, directDBConnectionEnabled(cfg))
38+
require.True(t, cfg.isDirectDBConnectionEnabled)
3939
require.Equal(t, "server=0.0.0.0;user id=sa;password=password;port=1433", getDBConnectionString(cfg))
4040

4141
params := receivertest.NewNopSettings(metadata.Type)
@@ -78,7 +78,7 @@ func TestFactoryOtherOS(t *testing.T) {
7878
cfg.Port = 1433
7979
require.NoError(t, cfg.Validate())
8080

81-
require.True(t, directDBConnectionEnabled(cfg))
81+
require.True(t, cfg.isDirectDBConnectionEnabled)
8282
require.Equal(t, "server=0.0.0.0;user id=sa;password=password;port=1433", getDBConnectionString(cfg))
8383

8484
params := receivertest.NewNopSettings(metadata.Type)

receiver/sqlserverreceiver/factory_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func TestFactory(t *testing.T) {
102102
require.NoError(t, cfg.Validate())
103103
cfg.Metrics.SqlserverDatabaseLatency.Enabled = true
104104

105-
require.True(t, directDBConnectionEnabled(cfg))
105+
require.True(t, cfg.isDirectDBConnectionEnabled)
106106
require.Equal(t, "server=0.0.0.0;user id=sa;password=password;port=1433", getDBConnectionString(cfg))
107107

108108
params := receivertest.NewNopSettings(metadata.Type)
@@ -190,7 +190,7 @@ func TestFactory(t *testing.T) {
190190
require.NoError(t, cfg.Validate())
191191
cfg.Metrics.SqlserverDatabaseLatency.Enabled = true
192192

193-
require.True(t, directDBConnectionEnabled(cfg))
193+
require.True(t, cfg.isDirectDBConnectionEnabled)
194194
require.Equal(t, "server=0.0.0.0;user id=sa;password=password;port=1433", getDBConnectionString(cfg))
195195

196196
params := receivertest.NewNopSettings(metadata.Type)

0 commit comments

Comments
 (0)