From ad09d57e450311dba91267e547dd7f7750dcd6f0 Mon Sep 17 00:00:00 2001 From: Dmitrii Anoshin Date: Wed, 19 Jun 2024 08:22:01 -0700 Subject: [PATCH] [exporter/clickhouse] Unexport extra Config methods (#33647) We need to keep the public Go API lean and don't export API when it's not required --- ...xporter-enexport-extra-config-methods.yaml | 22 +++++++++++++++++++ exporter/clickhouseexporter/config.go | 12 +++++----- exporter/clickhouseexporter/config_test.go | 10 ++++----- exporter/clickhouseexporter/exporter_logs.go | 6 ++--- .../clickhouseexporter/exporter_metrics.go | 4 ++-- .../clickhouseexporter/exporter_sql_test.go | 4 ++-- .../clickhouseexporter/exporter_traces.go | 8 +++---- 7 files changed, 44 insertions(+), 22 deletions(-) create mode 100644 .chloggen/clickhouse-exporter-enexport-extra-config-methods.yaml diff --git a/.chloggen/clickhouse-exporter-enexport-extra-config-methods.yaml b/.chloggen/clickhouse-exporter-enexport-extra-config-methods.yaml new file mode 100644 index 000000000000..f6fcb794e16e --- /dev/null +++ b/.chloggen/clickhouse-exporter-enexport-extra-config-methods.yaml @@ -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] diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 06eb80aae4a0..0f16ba7827f2 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -149,8 +149,8 @@ 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 } @@ -158,8 +158,8 @@ func (cfg *Config) ShouldCreateSchema() bool { 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 @@ -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 "" } diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index 3f3b27d6ec3a..6e3239214a0c 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -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()) }) } } @@ -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()) }) } } @@ -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()) }) } } diff --git a/exporter/clickhouseexporter/exporter_logs.go b/exporter/clickhouseexporter/exporter_logs.go index 69707ca80fe2..700c93e9ba92 100644 --- a/exporter/clickhouseexporter/exporter_logs.go +++ b/exporter/clickhouseexporter/exporter_logs.go @@ -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 } @@ -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) @@ -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 { diff --git a/exporter/clickhouseexporter/exporter_metrics.go b/exporter/clickhouseexporter/exporter_metrics.go index b4825565100f..7e5043de8e72 100644 --- a/exporter/clickhouseexporter/exporter_metrics.go +++ b/exporter/clickhouseexporter/exporter_metrics.go @@ -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 } @@ -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. diff --git a/exporter/clickhouseexporter/exporter_sql_test.go b/exporter/clickhouseexporter/exporter_sql_test.go index 17f52d4ba8be..0a01b1c284ef 100644 --- a/exporter/clickhouseexporter/exporter_sql_test.go +++ b/exporter/clickhouseexporter/exporter_sql_test.go @@ -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()) } } diff --git a/exporter/clickhouseexporter/exporter_traces.go b/exporter/clickhouseexporter/exporter_traces.go index 72e8e6219ca5..3b8fe7db808b 100644 --- a/exporter/clickhouseexporter/exporter_traces.go +++ b/exporter/clickhouseexporter/exporter_traces.go @@ -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 } @@ -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) }