Skip to content

Commit e1bbe24

Browse files
justinianvoss22justinianvoss22
committed
Feedback. chloggen. refactor string building into single function
1 parent 27acba3 commit e1bbe24

File tree

6 files changed

+79
-131
lines changed

6 files changed

+79
-131
lines changed

‎.chloggen/sqlqueryreceiver-break-up-datasource-into-struct.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2-
change_type: breaking
2+
change_type: enhancement
33

44
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
55
component: sqlqueryreceiver

‎internal/sqlquery/config.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,8 @@ func (c Config) Validate() error {
4040
return errors.New("'datasource_config.host' or 'datasource' must be specified")
4141
}
4242
// For sqlserver, port is optional
43-
if c.Driver != "sqlserver" {
44-
if c.DataSourceConfig.Port == 0 {
45-
return errors.New("'datasource_config.port' or 'datasource' must be specified")
46-
}
43+
if c.Driver != DriverSQLServer && c.DataSourceConfig.Port == 0 {
44+
return errors.New("'datasource_config.port' or 'datasource' must be specified")
4745
}
4846
if c.DataSourceConfig.Database == "" {
4947
return errors.New("'datasource_config.database' or 'datasource' must be specified")

‎internal/sqlquery/driver.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package sqlquery
2+
3+
const (
4+
DriverHDB = "hdb"
5+
DriverMySQL = "mysql"
6+
DriverOracle = "oracle"
7+
DriverPostgres = "postgres"
8+
DriverSnowflake = "snowflake"
9+
DriverSQLServer = "sqlserver"
10+
DriverTDS = "tds"
11+
)
12+
13+
// IsValidDriver checks if the given driver name is supported
14+
func IsValidDriver(driver string) bool {
15+
switch driver {
16+
case DriverHDB, DriverMySQL, DriverOracle, DriverPostgres, DriverSnowflake, DriverSQLServer, DriverTDS:
17+
return true
18+
default:
19+
return false
20+
}
21+
}

‎internal/sqlquery/scraper.go

Lines changed: 48 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -111,137 +111,64 @@ func (s *Scraper) Shutdown(_ context.Context) error {
111111
}
112112

113113
func BuildDataSourceString(driver string, dataSource DataSourceConfig) (string, error) {
114-
switch driver {
115-
case "postgres":
116-
return buildPostgreSQLString(dataSource)
117-
case "mysql":
118-
return buildMySQLString(dataSource)
119-
case "snowflake":
120-
return buildSnowflakeString(dataSource)
121-
case "sqlserver":
122-
return buildSQLServerString(dataSource)
123-
case "oracle":
124-
return buildOracleString(dataSource)
125-
default:
126-
return "", fmt.Errorf("unsupported driver: %s", driver)
127-
}
128-
}
129-
130-
func buildPostgreSQLString(conn DataSourceConfig) (string, error) {
131-
// PostgreSQL connection string format: postgresql://user:pass@host:port/db?param1=value1&param2=value2
132-
var auth string
133-
if conn.Username != "" {
134-
auth = fmt.Sprintf("%s:%s@", url.QueryEscape(conn.Username), url.QueryEscape(string(conn.Password)))
135-
}
136-
137-
query := url.Values{}
138-
for k, v := range conn.AdditionalParams {
139-
query.Set(k, fmt.Sprintf("%v", v))
140-
}
141-
142-
connStr := fmt.Sprintf("postgresql://%s%s:%d/%s", auth, conn.Host, conn.Port, conn.Database)
143-
if len(query) > 0 {
144-
connStr += "?" + query.Encode()
145-
}
146-
147-
return connStr, nil
148-
}
149-
150-
func buildMySQLString(conn DataSourceConfig) (string, error) {
151-
// MySQL connection string format: user:pass@tcp(host:port)/db?param1=value1&param2=value2
152-
var auth string
153-
154-
// MySQL requires no escaping of username and password
155-
if conn.Username != "" {
156-
username := conn.Username
157-
password := string(conn.Password)
158-
auth = fmt.Sprintf("%s:%s@", username, password)
159-
}
160-
161-
query := url.Values{}
162-
for k, v := range conn.AdditionalParams {
163-
query.Set(k, fmt.Sprintf("%v", v))
164-
}
165-
166-
connStr := fmt.Sprintf("%stcp(%s:%d)/%s", auth, conn.Host, conn.Port, conn.Database)
167-
if len(query) > 0 {
168-
connStr += "?" + query.Encode()
169-
}
170-
171-
return connStr, nil
172-
}
173-
174-
func buildSnowflakeString(conn DataSourceConfig) (string, error) {
175-
// Snowflake connection string format: user:pass@host:port/database?param1=value1&param2=value2
176-
var auth string
177-
if conn.Username != "" {
178-
auth = fmt.Sprintf("%s:%s@", url.QueryEscape(conn.Username), url.QueryEscape(string(conn.Password)))
179-
}
180-
181-
query := url.Values{}
182-
for k, v := range conn.AdditionalParams {
183-
query.Set(k, fmt.Sprintf("%v", v))
184-
}
185-
186-
connStr := fmt.Sprintf("%s%s:%d/%s", auth, conn.Host, conn.Port, conn.Database)
187-
if len(query) > 0 {
188-
connStr += "?" + query.Encode()
189-
}
190-
191-
return connStr, nil
192-
}
193-
194-
func buildSQLServerString(conn DataSourceConfig) (string, error) {
195-
// SQL Server connection string format: sqlserver://username:password@host:port/instance
196114
var auth string
197-
198-
if conn.Username != "" {
199-
auth = fmt.Sprintf("%s:%s@", url.QueryEscape(conn.Username), url.QueryEscape(string(conn.Password)))
200-
}
201-
202-
// replace all backslashes with forward slashes
203-
host := strings.ReplaceAll(conn.Host, "\\", "/")
204-
205-
// if host contains a "/", split it into hostname and instance
206-
parts := strings.SplitN(host, "/", 2)
207-
hostname := parts[0]
208-
var instance string
209-
if len(parts) > 1 {
210-
instance = parts[1]
115+
if dataSource.Username != "" {
116+
// MySQL doesn't need URL escaping
117+
if driver == DriverMySQL {
118+
auth = fmt.Sprintf("%s:%s@", dataSource.Username, string(dataSource.Password))
119+
} else {
120+
auth = fmt.Sprintf("%s:%s@", url.QueryEscape(dataSource.Username), url.QueryEscape(string(dataSource.Password)))
121+
}
211122
}
212123

213124
query := url.Values{}
214-
query.Set("database", conn.Database)
215-
for k, v := range conn.AdditionalParams {
125+
for k, v := range dataSource.AdditionalParams {
216126
query.Set(k, fmt.Sprintf("%v", v))
217127
}
218128

219129
var connStr string
220-
if instance != "" {
221-
connStr = fmt.Sprintf("sqlserver://%s%s:%d/%s", auth, hostname, conn.Port, instance)
222-
} else {
223-
connStr = fmt.Sprintf("sqlserver://%s%s:%d", auth, hostname, conn.Port)
224-
}
225-
if len(query) > 0 {
226-
connStr += "?" + query.Encode()
227-
}
228-
229-
return connStr, nil
230-
}
231-
232-
func buildOracleString(conn DataSourceConfig) (string, error) {
233-
// Oracle connection string format: oracle://user:pass@host:port/service_name?param1=value1&param2=value2
234-
var auth string
235-
if conn.Username != "" {
236-
auth = fmt.Sprintf("%s:%s@", url.QueryEscape(conn.Username), url.QueryEscape(string(conn.Password)))
237-
}
238-
239-
query := url.Values{}
240-
for k, v := range conn.AdditionalParams {
241-
query.Set(k, fmt.Sprintf("%v", v))
130+
switch driver {
131+
case DriverHDB:
132+
// HDB connection string format: hdb://user:pass@host:port?param1=value1
133+
connStr = fmt.Sprintf("hdb://%s%s:%d", auth, dataSource.Host, dataSource.Port)
134+
case DriverMySQL:
135+
// MySQL connection string format: user:pass@tcp(host:port)/db?param1=value1&param2=value2
136+
connStr = fmt.Sprintf("%stcp(%s:%d)/%s", auth, dataSource.Host, dataSource.Port, dataSource.Database)
137+
case DriverOracle:
138+
// Oracle connection string format: oracle://user:pass@host:port/service_name?param1=value1&param2=value2
139+
connStr = fmt.Sprintf("oracle://%s%s:%d/%s", auth, dataSource.Host, dataSource.Port, dataSource.Database)
140+
case DriverPostgres:
141+
// PostgreSQL connection string format: postgresql://user:pass@host:port/db?param1=value1&param2=value2
142+
connStr = fmt.Sprintf("postgresql://%s%s:%d/%s", auth, dataSource.Host, dataSource.Port, dataSource.Database)
143+
case DriverSnowflake:
144+
// Snowflake connection string format: user:pass@host:port/database?param1=value1&param2=value2
145+
connStr = fmt.Sprintf("%s%s:%d/%s", auth, dataSource.Host, dataSource.Port, dataSource.Database)
146+
case DriverSQLServer:
147+
// SQL Server connection string format: sqlserver://username:password@host:port/instance
148+
149+
// replace all backslashes with forward slashes
150+
host := strings.ReplaceAll(dataSource.Host, "\\", "/")
151+
// if host contains a "/", split it into hostname and instance
152+
parts := strings.SplitN(host, "/", 2)
153+
hostname := parts[0]
154+
var instance string
155+
if len(parts) > 1 {
156+
instance = parts[1]
157+
}
158+
query.Set("database", dataSource.Database)
159+
if instance != "" {
160+
connStr = fmt.Sprintf("sqlserver://%s%s:%d/%s", auth, hostname, dataSource.Port, instance)
161+
} else {
162+
connStr = fmt.Sprintf("sqlserver://%s%s:%d", auth, hostname, dataSource.Port)
163+
}
164+
case DriverTDS:
165+
// TDS connection string format: tds://user:pass@host:port/database
166+
connStr = fmt.Sprintf("tds://%s%s:%d/%s", auth, dataSource.Host, dataSource.Port, dataSource.Database)
167+
default:
168+
return "", fmt.Errorf("unsupported driver: %s", driver)
242169
}
243170

244-
connStr := fmt.Sprintf("oracle://%s%s:%d/%s", auth, conn.Host, conn.Port, conn.Database)
171+
// Append query parameters if any exist
245172
if len(query) > 0 {
246173
connStr += "?" + query.Encode()
247174
}

‎internal/sqlquery/scraper_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ func TestScraper_StartAndTS_ErrorOnParse(t *testing.T) {
530530
assert.Error(t, err)
531531
}
532532

533-
func TestBpConnectionStringBuilder(t *testing.T) {
533+
func TestBuildDataSourceString(t *testing.T) {
534534
t.Parallel()
535535
tests := []struct {
536536
name string
@@ -769,7 +769,7 @@ func TestBpConnectionStringBuilder(t *testing.T) {
769769
}
770770
}
771771

772-
func TestBuildSQLServerString(t *testing.T) {
772+
func TestBuildDataSourceString_SQLServer(t *testing.T) {
773773
tests := []struct {
774774
name string
775775
config DataSourceConfig
@@ -819,7 +819,7 @@ func TestBuildSQLServerString(t *testing.T) {
819819

820820
for _, tt := range tests {
821821
t.Run(tt.name, func(t *testing.T) {
822-
got, err := buildSQLServerString(tt.config)
822+
got, err := BuildDataSourceString(DriverSQLServer, tt.config)
823823
if tt.wantErr {
824824
assert.Error(t, err)
825825
return

‎receiver/sqlqueryreceiver/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
[development]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#development
1515
[alpha]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#alpha
1616
[contrib]: https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol-contrib
17+
1718
<!-- end autogenerated section -->
1819

1920
The SQL Query Receiver uses custom SQL queries to generate metrics from a database connection.
@@ -26,7 +27,7 @@ The SQL Query Receiver uses custom SQL queries to generate metrics from a databa
2627
The configuration supports the following top-level fields:
2728

2829
- `driver`(required): The name of the database driver: one of _postgres_, _mysql_, _snowflake_, _sqlserver_, _hdb_ (SAP
29-
HANA), _oracle_ (Oracle DB), _tds_ (SapASE/Sybase).
30+
HANA), _oracle_ (Oracle DB), _tds_ (SapASE/Sybase).
3031
- `datasource`(required): The datasource value passed to [sql.Open](https://pkg.go.dev/database/sql#Open). This is
3132
a driver-specific string usually consisting of at least a database name and connection information. This is sometimes
3233
referred to as the "connection string" in driver documentation. Examples:
@@ -37,7 +38,7 @@ The configuration supports the following top-level fields:
3738
- [snowflake](https://github.com/snowflakedb/gosnowflake) - `username[:password]@<account_identifier>/dbname/schemaname[?param1=value&...&paramN=valueN]`
3839
- [sqlserver](https://github.com/microsoft/go-mssqldb) - `sqlserver://username:user_password@localhost:1433?database=db_name`
3940
- [tds](https://github.com/thda/tds) - `tds://username:user_password@localhost:5000/db_name`
40-
- `datasource_config` (optional): Used in place of the `datasource` string. Contains separate values that are processed separately, and then concatenated into the connection string.
41+
- `datasource_config` (optional): Used as an alternative connection option to the `datasource` string.
4142
```yaml
4243
host: localhost
4344
port: 5432
@@ -225,6 +226,7 @@ either case, the receiver will continue to operate.
225226
Refer to the config file [provided](./testdata/oracledb-receiver-config.yaml) for an example of using the
226227
Oracle DB driver to connect and query the same table schema and contents as the example above.
227228
The Oracle DB driver documentation can be found [here.](https://github.com/sijms/go-ora)
229+
228230
<!-- This link below causes the check-links check to fail. -->
229231
<!-- Another usage example is the `go_ora` example here: https://blogs.oracle.com/developers/post/connecting-a-go-application-to-oracle-database -->
230232

0 commit comments

Comments
 (0)