Skip to content

Commit f92a576

Browse files
[chore] [exporter/clickhouseexporter] full refactor for ClickHouse component (#40536)
#### Description This PR intends to reorganize the code of the exporter without making any user-facing changes to the behavior or config. The goal is to make it more maintainable and revise all previous changes into one cohesive component. Apologies in advance for the many changes, but I wanted this to be runnable so we can merge it into main. **Let me know if we need to split anything out, or make any changes.** Notable changes: - All `clickhouse-go` database calls now use the library's native Go interface instead of the `database/sql` interface. This is more efficient and easier to configure + test. There was a lot of overhead in converting from `database/sql` to the driver's native structures, so this has a performance benefit as well as more idiomatic code. - Files have been reorganized so that code is in a more logical and easy to find place. (For example, there were some shared functions inside the `exporter_logs.go` file, even though it was unrelated to logs) - SQL is no longer written inside the `.go` file, and is now using `go:embed` to embed it from a directory of files. - Moved generic code to the `internal` package to simplify the top level package's exports. - Optimized insert code for logs/traces to be more CPU efficient (there were some redundant calls for converting attributes to `map`, wasting CPU). See testing section below for notes on test changes. #### Testing - Mocked database tests were replaced with full integration tests for logs, traces, and metrics. - Integration test share the same test container, ensuring quicker testing (no more starting several containers for simple query tests) - Preserved `goleak` checks for integration tests (and unit tests) - Integration tests were split into multiple files (these are in `*_integration_test.go` files) - Unit tests are separate from the integration tests (these are in the usual `_test.go` files) - Added tests for new functions/cases - Removed irrelevant/unhelpful tests
1 parent a80fdb9 commit f92a576

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+3175
-3578
lines changed

exporter/clickhouseexporter/config.go

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,25 @@
44
package clickhouseexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter"
55

66
import (
7-
"database/sql"
87
"errors"
98
"fmt"
109
"net/url"
1110
"time"
1211

1312
"github.com/ClickHouse/clickhouse-go/v2"
13+
"go.opentelemetry.io/collector/component"
1414
"go.opentelemetry.io/collector/config/configopaque"
1515
"go.opentelemetry.io/collector/config/configretry"
1616
"go.opentelemetry.io/collector/exporter/exporterhelper"
1717

1818
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal"
19+
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal/metrics"
1920
)
2021

2122
// Config defines configuration for clickhouse exporter.
2223
type Config struct {
2324
// collectorVersion is the build version of the collector. This is overridden when an exporter is initialized.
2425
collectorVersion string
25-
driverName string // for testing
2626

2727
TimeoutSettings exporterhelper.TimeoutConfig `mapstructure:",squash"`
2828
configretry.BackOffConfig `mapstructure:"retry_on_failure"`
@@ -68,15 +68,15 @@ type Config struct {
6868

6969
type MetricTablesConfig struct {
7070
// Gauge is the table name for gauge metric type. default is `otel_metrics_gauge`.
71-
Gauge internal.MetricTypeConfig `mapstructure:"gauge"`
71+
Gauge metrics.MetricTypeConfig `mapstructure:"gauge"`
7272
// Sum is the table name for sum metric type. default is `otel_metrics_sum`.
73-
Sum internal.MetricTypeConfig `mapstructure:"sum"`
73+
Sum metrics.MetricTypeConfig `mapstructure:"sum"`
7474
// Summary is the table name for summary metric type. default is `otel_metrics_summary`.
75-
Summary internal.MetricTypeConfig `mapstructure:"summary"`
75+
Summary metrics.MetricTypeConfig `mapstructure:"summary"`
7676
// Histogram is the table name for histogram metric type. default is `otel_metrics_histogram`.
77-
Histogram internal.MetricTypeConfig `mapstructure:"histogram"`
77+
Histogram metrics.MetricTypeConfig `mapstructure:"histogram"`
7878
// ExponentialHistogram is the table name for exponential histogram metric type. default is `otel_metrics_exponential_histogram`.
79-
ExponentialHistogram internal.MetricTypeConfig `mapstructure:"exponential_histogram"`
79+
ExponentialHistogram metrics.MetricTypeConfig `mapstructure:"exponential_histogram"`
8080
}
8181

8282
// TableEngine defines the ENGINE string value when creating the table.
@@ -101,11 +101,36 @@ var (
101101
errConfigInvalidEndpoint = errors.New("endpoint must be url format")
102102
)
103103

104+
func createDefaultConfig() component.Config {
105+
return &Config{
106+
collectorVersion: "unknown",
107+
108+
TimeoutSettings: exporterhelper.NewDefaultTimeoutConfig(),
109+
QueueSettings: exporterhelper.NewDefaultQueueConfig(),
110+
BackOffConfig: configretry.NewDefaultBackOffConfig(),
111+
ConnectionParams: map[string]string{},
112+
Database: defaultDatabase,
113+
LogsTableName: "otel_logs",
114+
TracesTableName: "otel_traces",
115+
TTL: 0,
116+
CreateSchema: true,
117+
AsyncInsert: true,
118+
MetricsTables: MetricTablesConfig{
119+
Gauge: metrics.MetricTypeConfig{Name: defaultMetricTableName + defaultGaugeSuffix},
120+
Sum: metrics.MetricTypeConfig{Name: defaultMetricTableName + defaultSumSuffix},
121+
Summary: metrics.MetricTypeConfig{Name: defaultMetricTableName + defaultSummarySuffix},
122+
Histogram: metrics.MetricTypeConfig{Name: defaultMetricTableName + defaultHistogramSuffix},
123+
ExponentialHistogram: metrics.MetricTypeConfig{Name: defaultMetricTableName + defaultExpHistogramSuffix},
124+
},
125+
}
126+
}
127+
104128
// Validate the ClickHouse server configuration.
105129
func (cfg *Config) Validate() (err error) {
106130
if cfg.Endpoint == "" {
107131
err = errors.Join(err, errConfigNoEndpoint)
108132
}
133+
109134
dsn, e := cfg.buildDSN()
110135
if e != nil {
111136
err = errors.Join(err, e)
@@ -160,11 +185,6 @@ func (cfg *Config) buildDSN() (string, error) {
160185
}
161186
queryParams.Set("client_info_product", productInfo)
162187

163-
// Use database from config if not specified in path, or if config is not default.
164-
if dsnURL.Path == "" || cfg.Database != defaultDatabase {
165-
dsnURL.Path = cfg.Database
166-
}
167-
168188
// Override username and password if specified in config.
169189
if cfg.Username != "" {
170190
dsnURL.User = url.UserPassword(cfg.Username, string(cfg.Password))
@@ -175,23 +195,6 @@ func (cfg *Config) buildDSN() (string, error) {
175195
return dsnURL.String(), nil
176196
}
177197

178-
func (cfg *Config) buildDB() (*sql.DB, error) {
179-
dsn, err := cfg.buildDSN()
180-
if err != nil {
181-
return nil, err
182-
}
183-
184-
// ClickHouse sql driver will read clickhouse settings from the DSN string.
185-
// It also ensures defaults.
186-
// See https://github.com/ClickHouse/clickhouse-go/blob/08b27884b899f587eb5c509769cd2bdf74a9e2a1/clickhouse_std.go#L189
187-
conn, err := sql.Open(cfg.driverName, dsn)
188-
if err != nil {
189-
return nil, err
190-
}
191-
192-
return conn, nil
193-
}
194-
195198
// shouldCreateSchema returns true if the exporter should run the DDL for creating database/tables.
196199
func (cfg *Config) shouldCreateSchema() bool {
197200
return cfg.CreateSchema
@@ -242,6 +245,32 @@ func (cfg *Config) tableEngineString() string {
242245
return fmt.Sprintf("%s(%s)", engine, params)
243246
}
244247

248+
// database returns the preferred database for creating tables and inserting data.
249+
// The config option takes precedence over the DSN's settings.
250+
// Falls back to default if neither are set.
251+
// Assumes config has passed Validate.
252+
func (cfg *Config) database() string {
253+
if cfg.Database != "" && cfg.Database != defaultDatabase {
254+
return cfg.Database
255+
}
256+
257+
dsn, err := cfg.buildDSN()
258+
if err != nil {
259+
return ""
260+
}
261+
262+
dsnDB, err := internal.DatabaseFromDSN(dsn)
263+
if err != nil {
264+
return ""
265+
}
266+
267+
if dsnDB != "" && dsnDB != defaultDatabase {
268+
return dsnDB
269+
}
270+
271+
return defaultDatabase
272+
}
273+
245274
// clusterString generates the ON CLUSTER string. Returns empty string if not set.
246275
func (cfg *Config) clusterString() string {
247276
if cfg.ClusterName == "" {

0 commit comments

Comments
 (0)