[exporter/clickhouse] change how default database is read from config

.chloggen/clickhouse-change-default-database.yaml

@@ -0,0 +1,31 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: breaking
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: exporter/clickhouse
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Change behavior of how default database is read from the config"
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [33693]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  Changed the default `database` to `default`.
+  The final database will prioritize `endpoint`, unless `database` is set to a value not equal to `default`.
+  If neither are specified then it defaults to the `default` database.
+  Possible breaking change if someone has the DSN configured in combination with `database` config option.
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

exporter/clickhouseexporter/config.go

@@ -67,7 +67,7 @@ func (cfg *Config) Validate() (err error) {
 	if cfg.Endpoint == "" {
 		err = errors.Join(err, errConfigNoEndpoint)
 	}
-	dsn, e := cfg.buildDSN(cfg.Database)
+	dsn, e := cfg.buildDSN()
 	if e != nil {
 		err = errors.Join(err, e)
 	}
@@ -81,7 +81,7 @@ func (cfg *Config) Validate() (err error) {
 	return err
 }
 
-func (cfg *Config) buildDSN(database string) (string, error) {
+func (cfg *Config) buildDSN() (string, error) {
 	dsnURL, err := url.Parse(cfg.Endpoint)
 	if err != nil {
 		return "", fmt.Errorf("%w: %s", errConfigInvalidEndpoint, err.Error())
@@ -99,21 +99,11 @@ func (cfg *Config) buildDSN(database string) (string, error) {
 		queryParams.Set("secure", "true")
 	}
 
-	// Override database if specified in config.
-	if cfg.Database != "" {
+	// Use database from config if not specified in path, or if config is not default.
+	if dsnURL.Path == "" || cfg.Database != defaultDatabase {
 		dsnURL.Path = cfg.Database
 	}
 
-	// Override database if specified in database param.
-	if database != "" {
-		dsnURL.Path = database
-	}
-
-	// Use default database if not specified in any other place.
-	if database == "" && cfg.Database == "" && dsnURL.Path == "" {
-		dsnURL.Path = defaultDatabase
-	}
-
 	// Override username and password if specified in config.
 	if cfg.Username != "" {
 		dsnURL.User = url.UserPassword(cfg.Username, string(cfg.Password))
@@ -124,8 +114,8 @@ func (cfg *Config) buildDSN(database string) (string, error) {
 	return dsnURL.String(), nil
 }
 
-func (cfg *Config) buildDB(database string) (*sql.DB, error) {
-	dsn, err := cfg.buildDSN(database)
+func (cfg *Config) buildDB() (*sql.DB, error) {
+	dsn, err := cfg.buildDSN()
 	if err != nil {
 		return nil, err
 	}

exporter/clickhouseexporter/config_test.go

@@ -110,18 +110,33 @@ func TestConfig_buildDSN(t *testing.T) {
 		Database         string
 		ConnectionParams map[string]string
 	}
-	type args struct {
-		database string
+	mergeConfigWithFields := func(cfg *Config, fields fields) {
+		if fields.Endpoint != "" {
+			cfg.Endpoint = fields.Endpoint
+		}
+		if fields.Username != "" {
+			cfg.Username = fields.Username
+		}
+		if fields.Password != "" {
+			cfg.Password = configopaque.String(fields.Password)
+		}
+		if fields.Database != "" {
+			cfg.Database = fields.Database
+		}
+		if fields.ConnectionParams != nil {
+			cfg.ConnectionParams = fields.ConnectionParams
+		}
 	}
+
 	type ChOptions struct {
 		Secure      bool
 		DialTimeout time.Duration
 		Compress    clickhouse.CompressionMethod
 	}
+
 	tests := []struct {
 		name          string
 		fields        fields
-		args          args
 		want          string
 		wantChOptions ChOptions
 		wantErr       error
@@ -131,7 +146,6 @@ func TestConfig_buildDSN(t *testing.T) {
 			fields: fields{
 				Endpoint: defaultEndpoint,
 			},
-			args: args{},
 			wantChOptions: ChOptions{
 				Secure: false,
 			},
@@ -142,7 +156,6 @@ func TestConfig_buildDSN(t *testing.T) {
 			fields: fields{
 				Endpoint: "tcp://127.0.0.1:9000",
 			},
-			args: args{},
 			wantChOptions: ChOptions{
 				Secure: false,
 			},
@@ -156,9 +169,6 @@ func TestConfig_buildDSN(t *testing.T) {
 				Password: "bar",
 				Database: "otel",
 			},
-			args: args{
-				database: "otel",
-			},
 			wantChOptions: ChOptions{
 				Secure: false,
 			},
@@ -170,10 +180,6 @@ func TestConfig_buildDSN(t *testing.T) {
 				Endpoint: "clickhouse://foo:bar@127.0.0.1:9000/otel",
 				Username: "foo",
 				Password: "bar",
-				Database: "",
-			},
-			args: args{
-				database: "",
 			},
 			wantChOptions: ChOptions{
 				Secure: false,
@@ -198,7 +204,6 @@ func TestConfig_buildDSN(t *testing.T) {
 			wantChOptions: ChOptions{
 				Secure: true,
 			},
-			args: args{},
 			want: "https://127.0.0.1:9000/default?secure=true",
 		},
 		{
@@ -209,7 +214,6 @@ func TestConfig_buildDSN(t *testing.T) {
 			wantChOptions: ChOptions{
 				Secure: true,
 			},
-			args: args{},
 			want: "clickhouse://127.0.0.1:9000/default?foo=bar&secure=true",
 		},
 		{
@@ -222,7 +226,6 @@ func TestConfig_buildDSN(t *testing.T) {
 				DialTimeout: 30 * time.Second,
 				Compress:    clickhouse.CompressionLZ4,
 			},
-			args: args{},
 			want: "https://127.0.0.1:9000/default?compress=lz4&dial_timeout=30s&secure=true",
 		},
 		{
@@ -234,43 +237,35 @@ func TestConfig_buildDSN(t *testing.T) {
 			wantChOptions: ChOptions{
 				Secure: true,
 			},
-			args: args{},
 			want: "clickhouse://127.0.0.1:9000/default?foo=bar&secure=true",
 		},
 		{
-			name: "support replace database in DSN to default database",
+			name: "support replace database in DSN with config to override database",
 			fields: fields{
 				Endpoint: "tcp://127.0.0.1:9000/otel",
+				Database: "override",
 			},
-			args: args{
-				database: defaultDatabase,
-			},
-			want: "tcp://127.0.0.1:9000/default",
+			want: "tcp://127.0.0.1:9000/override",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cfg := &Config{
-				Endpoint:         tt.fields.Endpoint,
-				Username:         tt.fields.Username,
-				Password:         configopaque.String(tt.fields.Password),
-				Database:         tt.fields.Database,
-				ConnectionParams: tt.fields.ConnectionParams,
-			}
-			got, err := cfg.buildDSN(tt.args.database)
+			cfg := createDefaultConfig().(*Config)
+			mergeConfigWithFields(cfg, tt.fields)
+			dsn, err := cfg.buildDSN()
 
 			if tt.wantErr != nil {
-				assert.ErrorIs(t, err, tt.wantErr, "buildDSN(%v)", tt.args.database)
+				assert.ErrorIs(t, err, tt.wantErr, "buildDSN()")
 			} else {
 				// Validate DSN
-				opts, err := clickhouse.ParseDSN(got)
+				opts, err := clickhouse.ParseDSN(dsn)
 				assert.NoError(t, err)
 				assert.Equalf(t, tt.wantChOptions.Secure, opts.TLS != nil, "TLSConfig is not nil")
 				assert.Equalf(t, tt.wantChOptions.DialTimeout, opts.DialTimeout, "DialTimeout is not nil")
 				if tt.wantChOptions.Compress != 0 {
 					assert.Equalf(t, tt.wantChOptions.Compress, opts.Compression.Method, "Compress is not nil")
 				}
-				assert.Equalf(t, tt.want, got, "buildDSN(%v)", tt.args.database)
+				assert.Equalf(t, tt.want, dsn, "buildDSN()")
 			}
 
 		})

exporter/clickhouseexporter/exporter_logs.go

@@ -205,7 +205,7 @@ var driverName = "clickhouse" // for testing
 
 // newClickhouseClient create a clickhouse client.
 func newClickhouseClient(cfg *Config) (*sql.DB, error) {
-	db, err := cfg.buildDB(cfg.Database)
+	db, err := cfg.buildDB()
 	if err != nil {
 		return nil, err
 	}
@@ -218,7 +218,7 @@ func createDatabase(ctx context.Context, cfg *Config) error {
 		return nil
 	}
 
-	db, err := cfg.buildDB(defaultDatabase)
+	db, err := cfg.buildDB()
 	if err != nil {
 		return err
 	}
@@ -228,7 +228,7 @@ func createDatabase(ctx context.Context, cfg *Config) error {
 	query := fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s %s", cfg.Database, cfg.clusterString())
 	_, err = db.ExecContext(ctx, query)
 	if err != nil {
-		return fmt.Errorf("create database:%w", err)
+		return fmt.Errorf("create database: %w", err)
 	}
 	return nil
 }