Skip to content

Commit

Permalink
[exporter/clickhouse] Unexport extra Config methods (open-telemetry#3…
Browse files Browse the repository at this point in the history
…3647)

We need to keep the public Go API lean and don't export API when it's
not required
  • Loading branch information
dmitryax authored Jun 19, 2024
1 parent 97f46cc commit ad09d57
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 22 deletions.
22 changes: 22 additions & 0 deletions .chloggen/clickhouse-exporter-enexport-extra-config-methods.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# 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: Unexport extra configuration methods.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [33647]

# 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: [api]
12 changes: 6 additions & 6 deletions exporter/clickhouseexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,17 @@ func (cfg *Config) buildDB(database string) (*sql.DB, error) {
return conn, nil
}

// ShouldCreateSchema returns true if the exporter should run the DDL for creating database/tables.
func (cfg *Config) ShouldCreateSchema() bool {
// shouldCreateSchema returns true if the exporter should run the DDL for creating database/tables.
func (cfg *Config) shouldCreateSchema() bool {
if cfg.CreateSchema == nil {
return true // default to true
}

return *cfg.CreateSchema
}

// TableEngineString generates the ENGINE string.
func (cfg *Config) TableEngineString() string {
// tableEngineString generates the ENGINE string.
func (cfg *Config) tableEngineString() string {
engine := cfg.TableEngine.Name
params := cfg.TableEngine.Params

Expand All @@ -171,8 +171,8 @@ func (cfg *Config) TableEngineString() string {
return fmt.Sprintf("%s(%s)", engine, params)
}

// ClusterString generates the ON CLUSTER string. Returns empty string if not set.
func (cfg *Config) ClusterString() string {
// clusterString generates the ON CLUSTER string. Returns empty string if not set.
func (cfg *Config) clusterString() string {
if cfg.ClusterName == "" {
return ""
}
Expand Down
10 changes: 5 additions & 5 deletions exporter/clickhouseexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,9 @@ func TestShouldCreateSchema(t *testing.T) {
}

for _, tt := range tests {
t.Run(fmt.Sprintf("ShouldCreateSchema case %s", tt.name), func(t *testing.T) {
t.Run(fmt.Sprintf("shouldCreateSchema case %s", tt.name), func(t *testing.T) {
assert.NoError(t, component.ValidateConfig(tt))
assert.Equal(t, tt.expected, tt.input.ShouldCreateSchema())
assert.Equal(t, tt.expected, tt.input.shouldCreateSchema())
})
}
}
Expand Down Expand Up @@ -356,7 +356,7 @@ func TestTableEngineConfigParsing(t *testing.T) {
require.NoError(t, sub.Unmarshal(cfg))

assert.NoError(t, component.ValidateConfig(cfg))
assert.Equal(t, tt.expected, cfg.(*Config).TableEngineString())
assert.Equal(t, tt.expected, cfg.(*Config).tableEngineString())
})
}
}
Expand All @@ -383,13 +383,13 @@ func TestClusterString(t *testing.T) {
}

for i, tt := range tests {
t.Run(fmt.Sprintf("ClusterString case %d", i), func(t *testing.T) {
t.Run(fmt.Sprintf("clusterString case %d", i), func(t *testing.T) {
cfg := createDefaultConfig()
cfg.(*Config).Endpoint = defaultEndpoint
cfg.(*Config).ClusterName = tt.input

assert.NoError(t, component.ValidateConfig(cfg))
assert.Equal(t, tt.expected, cfg.(*Config).ClusterString())
assert.Equal(t, tt.expected, cfg.(*Config).clusterString())
})
}
}
6 changes: 3 additions & 3 deletions exporter/clickhouseexporter/exporter_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func newLogsExporter(logger *zap.Logger, cfg *Config) (*logsExporter, error) {
}

func (e *logsExporter) start(ctx context.Context, _ component.Host) error {
if !e.cfg.ShouldCreateSchema() {
if !e.cfg.shouldCreateSchema() {
return nil
}

Expand Down Expand Up @@ -225,7 +225,7 @@ func createDatabase(ctx context.Context, cfg *Config) error {
defer func() {
_ = db.Close()
}()
query := fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s %s", cfg.Database, cfg.ClusterString())
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)
Expand All @@ -242,7 +242,7 @@ func createLogsTable(ctx context.Context, cfg *Config, db *sql.DB) error {

func renderCreateLogsTableSQL(cfg *Config) string {
ttlExpr := generateTTLExpr(cfg.TTLDays, cfg.TTL, "TimestampTime")
return fmt.Sprintf(createLogsTableSQL, cfg.LogsTableName, cfg.ClusterString(), cfg.TableEngineString(), ttlExpr)
return fmt.Sprintf(createLogsTableSQL, cfg.LogsTableName, cfg.clusterString(), cfg.tableEngineString(), ttlExpr)
}

func renderInsertLogsSQL(cfg *Config) string {
Expand Down
4 changes: 2 additions & 2 deletions exporter/clickhouseexporter/exporter_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func newMetricsExporter(logger *zap.Logger, cfg *Config) (*metricsExporter, erro
func (e *metricsExporter) start(ctx context.Context, _ component.Host) error {
internal.SetLogger(e.logger)

if !e.cfg.ShouldCreateSchema() {
if !e.cfg.shouldCreateSchema() {
return nil
}

Expand All @@ -48,7 +48,7 @@ func (e *metricsExporter) start(ctx context.Context, _ component.Host) error {
}

ttlExpr := generateTTLExpr(e.cfg.TTLDays, e.cfg.TTL, "toDateTime(TimeUnix)")
return internal.NewMetricsTable(ctx, e.cfg.MetricsTableName, e.cfg.ClusterString(), e.cfg.TableEngineString(), ttlExpr, e.client)
return internal.NewMetricsTable(ctx, e.cfg.MetricsTableName, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client)
}

// shutdown will shut down the exporter.
Expand Down
4 changes: 2 additions & 2 deletions exporter/clickhouseexporter/exporter_sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ type clusterTestConfig struct {

func (test clusterTestConfig) verifyConfig(t *testing.T, cfg *Config) {
if test.cluster == "" {
require.Empty(t, cfg.ClusterString())
require.Empty(t, cfg.clusterString())
} else {
require.NotEmpty(t, cfg.ClusterString())
require.NotEmpty(t, cfg.clusterString())
}
}

Expand Down
8 changes: 4 additions & 4 deletions exporter/clickhouseexporter/exporter_traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func newTracesExporter(logger *zap.Logger, cfg *Config) (*tracesExporter, error)
}

func (e *tracesExporter) start(ctx context.Context, _ component.Host) error {
if !e.cfg.ShouldCreateSchema() {
if !e.cfg.shouldCreateSchema() {
return nil
}

Expand Down Expand Up @@ -296,15 +296,15 @@ func renderInsertTracesSQL(cfg *Config) string {

func renderCreateTracesTableSQL(cfg *Config) string {
ttlExpr := generateTTLExpr(cfg.TTLDays, cfg.TTL, "toDateTime(Timestamp)")
return fmt.Sprintf(createTracesTableSQL, cfg.TracesTableName, cfg.ClusterString(), cfg.TableEngineString(), ttlExpr)
return fmt.Sprintf(createTracesTableSQL, cfg.TracesTableName, cfg.clusterString(), cfg.tableEngineString(), ttlExpr)
}

func renderCreateTraceIDTsTableSQL(cfg *Config) string {
ttlExpr := generateTTLExpr(cfg.TTLDays, cfg.TTL, "toDateTime(Start)")
return fmt.Sprintf(createTraceIDTsTableSQL, cfg.TracesTableName, cfg.ClusterString(), cfg.TableEngineString(), ttlExpr)
return fmt.Sprintf(createTraceIDTsTableSQL, cfg.TracesTableName, cfg.clusterString(), cfg.tableEngineString(), ttlExpr)
}

func renderTraceIDTsMaterializedViewSQL(cfg *Config) string {
return fmt.Sprintf(createTraceIDTsMaterializedViewSQL, cfg.TracesTableName,
cfg.ClusterString(), cfg.Database, cfg.TracesTableName, cfg.Database, cfg.TracesTableName)
cfg.clusterString(), cfg.Database, cfg.TracesTableName, cfg.Database, cfg.TracesTableName)
}

0 comments on commit ad09d57

Please sign in to comment.