From bcf8487e1a2fade2ce4d67568f92b3eda8873bef Mon Sep 17 00:00:00 2001 From: odubajDT Date: Tue, 30 Jul 2024 12:00:11 +0200 Subject: [PATCH 01/12] [exporter/clickhouse]: Add the ability to override default table names for all metric types Signed-off-by: odubajDT --- .chloggen/metrics-tables-clickhouse.yaml | 27 +++ exporter/clickhouseexporter/config.go | 69 ++++++++ exporter/clickhouseexporter/config_test.go | 160 ++++++++++++++++++ .../clickhouseexporter/exporter_metrics.go | 16 +- .../exporter_metrics_test.go | 25 +++ exporter/clickhouseexporter/factory.go | 8 +- .../internal/exponential_histogram_metrics.go | 4 +- .../internal/gauge_metrics.go | 4 +- .../internal/histogram_metrics.go | 4 +- .../internal/metrics_model.go | 32 ++-- .../internal/sum_metrics.go | 4 +- .../internal/summary_metrics.go | 4 +- .../clickhouseexporter/testdata/config.yaml | 6 + 13 files changed, 335 insertions(+), 28 deletions(-) create mode 100644 .chloggen/metrics-tables-clickhouse.yaml diff --git a/.chloggen/metrics-tables-clickhouse.yaml b/.chloggen/metrics-tables-clickhouse.yaml new file mode 100644 index 000000000000..19d55a8cef7b --- /dev/null +++ b/.chloggen/metrics-tables-clickhouse.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# 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: Add the ability to override default table names for all metric types. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34225] + +# (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: + +# 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: [] diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 15c6cf90c9f5..883f1ba75d1f 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -52,6 +52,21 @@ type Config struct { // Ignored if async inserts are configured in the `endpoint` or `connection_params`. // Async inserts may still be overridden server-side. AsyncInsert bool `mapstructure:"async_insert"` + // MetricsTables defines the table names for metric types. + MetricsTables TableNames `mapstructure:"metrics_tables"` +} + +type TableNames struct { + // Gauge is the table name for gauge metric type. default is `otel_metrics_gauge`. + Gauge string `mapstructure:"gauge"` + // Sum is the table name for sum metric type. default is `otel_metrics_sum`. + Sum string `mapstructure:"sum"` + // Summary is the table name for summary metric type. default is `otel_metrics_summary`. + Summary string `mapstructure:"summary"` + // Histogram is the table name for histogram metric type. default is `otel_metrics_histogram`. + Histogram string `mapstructure:"histogram"` + // ExponentialHistogram is the table name for exponential histogram metric type. default is `otel_metrics_exponential_histogram`. + ExponentialHistogram string `mapstructure:"exponential_histogram"` } // TableEngine defines the ENGINE string value when creating the table. @@ -62,6 +77,12 @@ type TableEngine struct { const defaultDatabase = "default" const defaultTableEngineName = "MergeTree" +const defaultMetricTableName = "otel_metrics" +const defaultGaugeSuffix = "_gauge" +const defaultSumSuffix = "_sum" +const defaultSummarySuffix = "_summary" +const defaultHistogramSuffix = "_histogram" +const defaultExpHistogramSuffix = "_exponential_histogram" var ( errConfigNoEndpoint = errors.New("endpoint must be specified") @@ -78,6 +99,12 @@ func (cfg *Config) Validate() (err error) { err = errors.Join(err, e) } + if e := cfg.ValidateMetricTableNames(); err != nil { + err = errors.Join(err, e) + } else { + cfg.buildTableNames() + } + // Validate DSN with clickhouse driver. // Last chance to catch invalid config. if _, e := clickhouse.ParseDSN(dsn); e != nil { @@ -153,6 +180,48 @@ func (cfg *Config) shouldCreateSchema() bool { return cfg.CreateSchema } +func (cfg *Config) ValidateMetricTableNames() error { + if len(cfg.MetricsTableName) == 0 { + return nil + } + if cfg.areTableNamesSet() { + return fmt.Errorf("Warning: 'metrics_table_name' is deprecated, use 'metrics_tables' instead") + } + return nil +} + +func (cfg *Config) buildTableNames() { + tableName := defaultMetricTableName + + if len(cfg.MetricsTableName) != 0 && !cfg.areTableNamesSet() { + tableName = cfg.MetricsTableName + } + + if len(cfg.MetricsTables.Gauge) == 0 { + cfg.MetricsTables.Gauge = tableName + defaultGaugeSuffix + } + if len(cfg.MetricsTables.Sum) == 0 { + cfg.MetricsTables.Sum = tableName + defaultSumSuffix + } + if len(cfg.MetricsTables.Summary) == 0 { + cfg.MetricsTables.Summary = tableName + defaultSummarySuffix + } + if len(cfg.MetricsTables.Histogram) == 0 { + cfg.MetricsTables.Histogram = tableName + defaultHistogramSuffix + } + if len(cfg.MetricsTables.ExponentialHistogram) == 0 { + cfg.MetricsTables.ExponentialHistogram = tableName + defaultExpHistogramSuffix + } +} + +func (cfg *Config) areTableNamesSet() bool { + return len(cfg.MetricsTables.Gauge) != 0 || + len(cfg.MetricsTables.Sum) != 0 || + len(cfg.MetricsTables.Summary) != 0 || + len(cfg.MetricsTables.Histogram) != 0 || + len(cfg.MetricsTables.ExponentialHistogram) != 0 +} + // tableEngineString generates the ENGINE string. func (cfg *Config) tableEngineString() string { engine := cfg.TableEngine.Name diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index b97ba553e41a..99d41dd0557f 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -67,6 +67,13 @@ func TestLoadConfig(t *testing.T) { RandomizationFactor: backoff.DefaultRandomizationFactor, Multiplier: backoff.DefaultMultiplier, }, + MetricsTables: TableNames{ + Gauge: "otel_metrics_custom_gauge", + Sum: "otel_metrics_custom_sum", + Summary: "otel_metrics_custom_summary", + Histogram: "otel_metrics_custom_histogram", + ExponentialHistogram: "otel_metrics_custom_exp_histogram", + }, ConnectionParams: map[string]string{}, QueueSettings: exporterhelper.QueueConfig{ Enabled: true, @@ -102,6 +109,159 @@ func withDefaultConfig(fns ...func(*Config)) *Config { return cfg } +func TestValidateMetricTableNames(t *testing.T) { + tests := []struct { + name string + cfg Config + wantErr error + }{ + { + name: "metric_table_name not set", + cfg: Config{}, + wantErr: nil, + }, + { + name: "metric_table_name set", + cfg: Config{ + MetricsTableName: "table", + }, + wantErr: nil, + }, + { + name: "metric_table_name set with metric_tables", + cfg: Config{ + MetricsTableName: "table", + MetricsTables: TableNames{ + Gauge: "gauge", + }, + }, + wantErr: fmt.Errorf("Warning: 'metrics_table_name' is deprecated, use 'metrics_tables' instead"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.wantErr, tt.cfg.ValidateMetricTableNames()) + }) + } +} + +func TestBuildMetricTableNames(t *testing.T) { + tests := []struct { + name string + cfg Config + want Config + }{ + { + name: "nothing set", + cfg: Config{}, + want: Config{ + MetricsTables: TableNames{ + Gauge: "otel_metrics_gauge", + Sum: "otel_metrics_sum", + Summary: "otel_metrics_summary", + Histogram: "otel_metrics_histogram", + ExponentialHistogram: "otel_metrics_exponential_histogram", + }, + }, + }, + { + name: "only metric_table_name set", + cfg: Config{ + MetricsTableName: "table_name", + }, + want: Config{ + MetricsTableName: "table_name", + MetricsTables: TableNames{ + Gauge: "table_name_gauge", + Sum: "table_name_sum", + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exponential_histogram", + }, + }, + }, + { + name: "only metric_tables set fully", + cfg: Config{ + MetricsTables: TableNames{ + Gauge: "table_name_gauge", + Sum: "table_name_sum", + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exponential_histogram", + }, + }, + want: Config{ + MetricsTables: TableNames{ + Gauge: "table_name_gauge", + Sum: "table_name_sum", + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exponential_histogram", + }, + }, + }, + { + name: "only metric_tables set partially", + cfg: Config{ + MetricsTables: TableNames{ + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exp_histogram", + }, + }, + want: Config{ + MetricsTables: TableNames{ + Gauge: "otel_metrics_gauge", + Sum: "otel_metrics_sum", + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exp_histogram", + }, + }, + }, + { + name: "only metric_tables set partially with metric_table_name", + cfg: Config{ + MetricsTableName: "custom_name", + MetricsTables: TableNames{ + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exp_histogram", + }, + }, + want: Config{ + MetricsTableName: "custom_name", + MetricsTables: TableNames{ + Gauge: "otel_metrics_gauge", + Sum: "otel_metrics_sum", + Summary: "table_name_summary", + Histogram: "table_name_histogram", + ExponentialHistogram: "table_name_exp_histogram", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.cfg.buildTableNames() + require.Equal(t, tt.want, tt.cfg) + }) + } +} + +func TestAreTableNamesSet(t *testing.T) { + cfg := Config{} + require.False(t, cfg.areTableNamesSet()) + + cfg = Config{ + MetricsTables: TableNames{ + Gauge: "gauge", + }, + } + require.True(t, cfg.areTableNamesSet()) +} + func TestConfig_buildDSN(t *testing.T) { type fields struct { Endpoint string diff --git a/exporter/clickhouseexporter/exporter_metrics.go b/exporter/clickhouseexporter/exporter_metrics.go index 6f11ba940d57..7c924485354f 100644 --- a/exporter/clickhouseexporter/exporter_metrics.go +++ b/exporter/clickhouseexporter/exporter_metrics.go @@ -48,7 +48,18 @@ func (e *metricsExporter) start(ctx context.Context, _ component.Host) error { } ttlExpr := generateTTLExpr(e.cfg.TTL, "toDateTime(TimeUnix)") - return internal.NewMetricsTable(ctx, e.cfg.MetricsTableName, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client) + tableNames := e.generateTableNames() + return internal.NewMetricsTable(ctx, tableNames, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client) +} + +func (e *metricsExporter) generateTableNames() internal.MetricTableNames { + return internal.MetricTableNames{ + pmetric.MetricTypeGauge: e.cfg.MetricsTables.Gauge, + pmetric.MetricTypeSum: e.cfg.MetricsTables.Sum, + pmetric.MetricTypeSummary: e.cfg.MetricsTables.Summary, + pmetric.MetricTypeHistogram: e.cfg.MetricsTables.Histogram, + pmetric.MetricTypeExponentialHistogram: e.cfg.MetricsTables.ExponentialHistogram, + } } // shutdown will shut down the exporter. @@ -60,7 +71,8 @@ func (e *metricsExporter) shutdown(_ context.Context) error { } func (e *metricsExporter) pushMetricsData(ctx context.Context, md pmetric.Metrics) error { - metricsMap := internal.NewMetricsModel(e.cfg.MetricsTableName) + tableNames := e.generateTableNames() + metricsMap := internal.NewMetricsModel(tableNames) for i := 0; i < md.ResourceMetrics().Len(); i++ { metrics := md.ResourceMetrics().At(i) resAttr := attributesToMap(metrics.Resource().Attributes()) diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index 5a705c3b6862..9c193fc2c789 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -17,6 +17,8 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" "go.uber.org/zap/zaptest" + + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal" ) func TestMetricsClusterConfig(t *testing.T) { @@ -33,6 +35,29 @@ func TestMetricsTableEngineConfig(t *testing.T) { }) } +func Test_generateMetricTableNames(t *testing.T) { + cfg := Config{ + MetricsTables: TableNames{ + Gauge: "otel_metrics_custom_gauge", + Sum: "otel_metrics_custom_sum", + Summary: "otel_metrics_custom_summary", + Histogram: "otel_metrics_custom_histogram", + ExponentialHistogram: "otel_metrics_custom_exp_histogram", + }, + } + exporter := metricsExporter{ + cfg: &cfg, + } + + require.Equal(t, internal.MetricTableNames{ + pmetric.MetricTypeGauge: cfg.MetricsTables.Gauge, + pmetric.MetricTypeSum: cfg.MetricsTables.Sum, + pmetric.MetricTypeSummary: cfg.MetricsTables.Summary, + pmetric.MetricTypeHistogram: cfg.MetricsTables.Histogram, + pmetric.MetricTypeExponentialHistogram: cfg.MetricsTables.ExponentialHistogram, + }, exporter.generateTableNames()) +} + func TestExporter_pushMetricsData(t *testing.T) { t.Parallel() t.Run("push success", func(t *testing.T) { diff --git a/exporter/clickhouseexporter/factory.go b/exporter/clickhouseexporter/factory.go index c4f632cab2f8..0bb84633d747 100644 --- a/exporter/clickhouseexporter/factory.go +++ b/exporter/clickhouseexporter/factory.go @@ -38,10 +38,16 @@ func createDefaultConfig() component.Config { Database: defaultDatabase, LogsTableName: "otel_logs", TracesTableName: "otel_traces", - MetricsTableName: "otel_metrics", TTL: 0, CreateSchema: true, AsyncInsert: true, + MetricsTables: TableNames{ + Gauge: defaultMetricTableName + defaultGaugeSuffix, + Sum: defaultMetricTableName + defaultSumSuffix, + Summary: defaultMetricTableName + defaultSummarySuffix, + Histogram: defaultMetricTableName + defaultHistogramSuffix, + ExponentialHistogram: defaultMetricTableName + defaultExpHistogramSuffix, + }, } } diff --git a/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go b/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go index d23c37d2e2fd..d254dceada86 100644 --- a/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go +++ b/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go @@ -18,7 +18,7 @@ import ( const ( // language=ClickHouse SQL createExpHistogramTableSQL = ` -CREATE TABLE IF NOT EXISTS %s_exponential_histogram %s ( +CREATE TABLE IF NOT EXISTS %s %s ( ResourceAttributes Map(LowCardinality(String), String) CODEC(ZSTD(1)), ResourceSchemaUrl String CODEC(ZSTD(1)), ScopeName String CODEC(ZSTD(1)), @@ -65,7 +65,7 @@ ORDER BY (ServiceName, MetricName, Attributes, toUnixTimestamp64Nano(TimeUnix)) SETTINGS index_granularity=8192, ttl_only_drop_parts = 1; ` // language=ClickHouse SQL - insertExpHistogramTableSQL = `INSERT INTO %s_exponential_histogram ( + insertExpHistogramTableSQL = `INSERT INTO %s ( ResourceAttributes, ResourceSchemaUrl, ScopeName, diff --git a/exporter/clickhouseexporter/internal/gauge_metrics.go b/exporter/clickhouseexporter/internal/gauge_metrics.go index a45121ccaa95..d2a1e5beb939 100644 --- a/exporter/clickhouseexporter/internal/gauge_metrics.go +++ b/exporter/clickhouseexporter/internal/gauge_metrics.go @@ -18,7 +18,7 @@ import ( const ( // language=ClickHouse SQL createGaugeTableSQL = ` -CREATE TABLE IF NOT EXISTS %s_gauge %s ( +CREATE TABLE IF NOT EXISTS %s %s ( ResourceAttributes Map(LowCardinality(String), String) CODEC(ZSTD(1)), ResourceSchemaUrl String CODEC(ZSTD(1)), ScopeName String CODEC(ZSTD(1)), @@ -55,7 +55,7 @@ ORDER BY (ServiceName, MetricName, Attributes, toUnixTimestamp64Nano(TimeUnix)) SETTINGS index_granularity=8192, ttl_only_drop_parts = 1; ` // language=ClickHouse SQL - insertGaugeTableSQL = `INSERT INTO %s_gauge ( + insertGaugeTableSQL = `INSERT INTO %s ( ResourceAttributes, ResourceSchemaUrl, ScopeName, diff --git a/exporter/clickhouseexporter/internal/histogram_metrics.go b/exporter/clickhouseexporter/internal/histogram_metrics.go index 0f75d83c93e8..a2975328b389 100644 --- a/exporter/clickhouseexporter/internal/histogram_metrics.go +++ b/exporter/clickhouseexporter/internal/histogram_metrics.go @@ -18,7 +18,7 @@ import ( const ( // language=ClickHouse SQL createHistogramTableSQL = ` -CREATE TABLE IF NOT EXISTS %s_histogram %s ( +CREATE TABLE IF NOT EXISTS %s %s ( ResourceAttributes Map(LowCardinality(String), String) CODEC(ZSTD(1)), ResourceSchemaUrl String CODEC(ZSTD(1)), ScopeName String CODEC(ZSTD(1)), @@ -61,7 +61,7 @@ ORDER BY (ServiceName, MetricName, Attributes, toUnixTimestamp64Nano(TimeUnix)) SETTINGS index_granularity=8192, ttl_only_drop_parts = 1; ` // language=ClickHouse SQL - insertHistogramTableSQL = `INSERT INTO %s_histogram ( + insertHistogramTableSQL = `INSERT INTO %s ( ResourceAttributes, ResourceSchemaUrl, ScopeName, diff --git a/exporter/clickhouseexporter/internal/metrics_model.go b/exporter/clickhouseexporter/internal/metrics_model.go index b4a1e6a3ab71..a7664756c8a4 100644 --- a/exporter/clickhouseexporter/internal/metrics_model.go +++ b/exporter/clickhouseexporter/internal/metrics_model.go @@ -18,16 +18,18 @@ import ( "go.uber.org/zap" ) -var supportedMetricTypes = map[string]struct{}{ - createGaugeTableSQL: {}, - createSumTableSQL: {}, - createHistogramTableSQL: {}, - createExpHistogramTableSQL: {}, - createSummaryTableSQL: {}, +var supportedMetricTypes = MetricTableNames{ + pmetric.MetricTypeGauge: createGaugeTableSQL, + pmetric.MetricTypeSum: createSumTableSQL, + pmetric.MetricTypeHistogram: createHistogramTableSQL, + pmetric.MetricTypeExponentialHistogram: createExpHistogramTableSQL, + pmetric.MetricTypeSummary: createSummaryTableSQL, } var logger *zap.Logger +type MetricTableNames map[pmetric.MetricType]string + // MetricsModel is used to group metric data and insert into clickhouse // any type of metrics need implement it. type MetricsModel interface { @@ -51,9 +53,9 @@ func SetLogger(l *zap.Logger) { } // NewMetricsTable create metric tables with an expiry time to storage metric telemetry data -func NewMetricsTable(ctx context.Context, tableName, cluster, engine, ttlExpr string, db *sql.DB) error { - for table := range supportedMetricTypes { - query := fmt.Sprintf(table, tableName, cluster, engine, ttlExpr) +func NewMetricsTable(ctx context.Context, tableNames MetricTableNames, cluster, engine, ttlExpr string, db *sql.DB) error { + for key, table := range supportedMetricTypes { + query := fmt.Sprintf(table, tableNames[key], cluster, engine, ttlExpr) if _, err := db.ExecContext(ctx, query); err != nil { return fmt.Errorf("exec create metrics table sql: %w", err) } @@ -62,22 +64,22 @@ func NewMetricsTable(ctx context.Context, tableName, cluster, engine, ttlExpr st } // NewMetricsModel create a model for contain different metric data -func NewMetricsModel(tableName string) map[pmetric.MetricType]MetricsModel { +func NewMetricsModel(tableNames MetricTableNames) map[pmetric.MetricType]MetricsModel { return map[pmetric.MetricType]MetricsModel{ pmetric.MetricTypeGauge: &gaugeMetrics{ - insertSQL: fmt.Sprintf(insertGaugeTableSQL, tableName), + insertSQL: fmt.Sprintf(insertGaugeTableSQL, tableNames[pmetric.MetricTypeGauge]), }, pmetric.MetricTypeSum: &sumMetrics{ - insertSQL: fmt.Sprintf(insertSumTableSQL, tableName), + insertSQL: fmt.Sprintf(insertSumTableSQL, tableNames[pmetric.MetricTypeSum]), }, pmetric.MetricTypeHistogram: &histogramMetrics{ - insertSQL: fmt.Sprintf(insertHistogramTableSQL, tableName), + insertSQL: fmt.Sprintf(insertHistogramTableSQL, tableNames[pmetric.MetricTypeHistogram]), }, pmetric.MetricTypeExponentialHistogram: &expHistogramMetrics{ - insertSQL: fmt.Sprintf(insertExpHistogramTableSQL, tableName), + insertSQL: fmt.Sprintf(insertExpHistogramTableSQL, tableNames[pmetric.MetricTypeExponentialHistogram]), }, pmetric.MetricTypeSummary: &summaryMetrics{ - insertSQL: fmt.Sprintf(insertSummaryTableSQL, tableName), + insertSQL: fmt.Sprintf(insertSummaryTableSQL, tableNames[pmetric.MetricTypeSummary]), }, } } diff --git a/exporter/clickhouseexporter/internal/sum_metrics.go b/exporter/clickhouseexporter/internal/sum_metrics.go index c784dfdf7325..86f3e5553e67 100644 --- a/exporter/clickhouseexporter/internal/sum_metrics.go +++ b/exporter/clickhouseexporter/internal/sum_metrics.go @@ -18,7 +18,7 @@ import ( const ( // language=ClickHouse SQL createSumTableSQL = ` -CREATE TABLE IF NOT EXISTS %s_sum %s ( +CREATE TABLE IF NOT EXISTS %s %s ( ResourceAttributes Map(LowCardinality(String), String) CODEC(ZSTD(1)), ResourceSchemaUrl String CODEC(ZSTD(1)), ScopeName String CODEC(ZSTD(1)), @@ -57,7 +57,7 @@ ORDER BY (ServiceName, MetricName, Attributes, toUnixTimestamp64Nano(TimeUnix)) SETTINGS index_granularity=8192, ttl_only_drop_parts = 1; ` // language=ClickHouse SQL - insertSumTableSQL = `INSERT INTO %s_sum ( + insertSumTableSQL = `INSERT INTO %s ( ResourceAttributes, ResourceSchemaUrl, ScopeName, diff --git a/exporter/clickhouseexporter/internal/summary_metrics.go b/exporter/clickhouseexporter/internal/summary_metrics.go index 5f3ca7beab8e..6fa5560c3e36 100644 --- a/exporter/clickhouseexporter/internal/summary_metrics.go +++ b/exporter/clickhouseexporter/internal/summary_metrics.go @@ -18,7 +18,7 @@ import ( const ( // language=ClickHouse SQL createSummaryTableSQL = ` -CREATE TABLE IF NOT EXISTS %s_summary %s ( +CREATE TABLE IF NOT EXISTS %s %s ( ResourceAttributes Map(LowCardinality(String), String) CODEC(ZSTD(1)), ResourceSchemaUrl String CODEC(ZSTD(1)), ScopeName String CODEC(ZSTD(1)), @@ -53,7 +53,7 @@ ORDER BY (ServiceName, MetricName, Attributes, toUnixTimestamp64Nano(TimeUnix)) SETTINGS index_granularity=8192, ttl_only_drop_parts = 1; ` // language=ClickHouse SQL - insertSummaryTableSQL = `INSERT INTO %s_summary ( + insertSummaryTableSQL = `INSERT INTO %s ( ResourceAttributes, ResourceSchemaUrl, ScopeName, diff --git a/exporter/clickhouseexporter/testdata/config.yaml b/exporter/clickhouseexporter/testdata/config.yaml index 1531e4578782..87c006359fe9 100644 --- a/exporter/clickhouseexporter/testdata/config.yaml +++ b/exporter/clickhouseexporter/testdata/config.yaml @@ -17,6 +17,12 @@ clickhouse/full: sending_queue: queue_size: 100 storage: file_storage/clickhouse + metrics_tables: + gauge: "otel_metrics_custom_gauge" + sum: "otel_metrics_custom_sum" + summary: "otel_metrics_custom_summary" + histogram: "otel_metrics_custom_histogram" + exponential_histogram: "otel_metrics_custom_exp_histogram" clickhouse/invalid-endpoint: endpoint: 127.0.0.1:9000 From 56ae481ed739aee7c62311bf80587d1d9d05b374 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 2 Aug 2024 10:54:21 +0200 Subject: [PATCH 02/12] PR review Signed-off-by: odubajDT --- exporter/clickhouseexporter/README.md | 10 ++- exporter/clickhouseexporter/config.go | 26 ++------ exporter/clickhouseexporter/config_test.go | 66 +++++-------------- .../clickhouseexporter/exporter_metrics.go | 44 ++++++++----- .../exporter_metrics_test.go | 31 +++++---- exporter/clickhouseexporter/factory.go | 2 +- .../internal/metrics_model.go | 16 +++-- 7 files changed, 88 insertions(+), 107 deletions(-) diff --git a/exporter/clickhouseexporter/README.md b/exporter/clickhouseexporter/README.md index 9f3a0e72ee40..3f956b064623 100644 --- a/exporter/clickhouseexporter/README.md +++ b/exporter/clickhouseexporter/README.md @@ -365,8 +365,13 @@ exporters: create_schema: true logs_table_name: otel_logs traces_table_name: otel_traces - metrics_table_name: otel_metrics timeout: 5s + metrics_tables: + gauge: "otel_metrics_gauge" + sum: "otel_metrics_sum" + summary: "otel_metrics_summary" + histogram: "otel_metrics_histogram" + exponential_histogram: "otel_metrics_exp_histogram" retry_on_failure: enabled: true initial_interval: 5s @@ -384,6 +389,9 @@ service: exporters: [ clickhouse ] ``` +**Note:** +Parameter `metrics_table_name` will be deprecated and replaced by `metric_tables` which support setting the full table name (including suffix) for tables of each metric type. Using `metrics_table_name` and `metric_tables` together will result in ignoring the `metrics_table_name` parameter value. + ## Contributing Before contributing, review the contribution guidelines in [CONTRIBUTING.md](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md). diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 883f1ba75d1f..93c43b7c601e 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -53,10 +53,10 @@ type Config struct { // Async inserts may still be overridden server-side. AsyncInsert bool `mapstructure:"async_insert"` // MetricsTables defines the table names for metric types. - MetricsTables TableNames `mapstructure:"metrics_tables"` + MetricsTables MetricTableNames `mapstructure:"metrics_tables"` } -type TableNames struct { +type MetricTableNames struct { // Gauge is the table name for gauge metric type. default is `otel_metrics_gauge`. Gauge string `mapstructure:"gauge"` // Sum is the table name for sum metric type. default is `otel_metrics_sum`. @@ -99,11 +99,7 @@ func (cfg *Config) Validate() (err error) { err = errors.Join(err, e) } - if e := cfg.ValidateMetricTableNames(); err != nil { - err = errors.Join(err, e) - } else { - cfg.buildTableNames() - } + cfg.buildMetricTableNames() // Validate DSN with clickhouse driver. // Last chance to catch invalid config. @@ -180,20 +176,10 @@ func (cfg *Config) shouldCreateSchema() bool { return cfg.CreateSchema } -func (cfg *Config) ValidateMetricTableNames() error { - if len(cfg.MetricsTableName) == 0 { - return nil - } - if cfg.areTableNamesSet() { - return fmt.Errorf("Warning: 'metrics_table_name' is deprecated, use 'metrics_tables' instead") - } - return nil -} - -func (cfg *Config) buildTableNames() { +func (cfg *Config) buildMetricTableNames() { tableName := defaultMetricTableName - if len(cfg.MetricsTableName) != 0 && !cfg.areTableNamesSet() { + if len(cfg.MetricsTableName) != 0 && !cfg.areMetricTableNamesSet() { tableName = cfg.MetricsTableName } @@ -214,7 +200,7 @@ func (cfg *Config) buildTableNames() { } } -func (cfg *Config) areTableNamesSet() bool { +func (cfg *Config) areMetricTableNamesSet() bool { return len(cfg.MetricsTables.Gauge) != 0 || len(cfg.MetricsTables.Sum) != 0 || len(cfg.MetricsTables.Summary) != 0 || diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index 99d41dd0557f..d01cc255370f 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -67,7 +67,7 @@ func TestLoadConfig(t *testing.T) { RandomizationFactor: backoff.DefaultRandomizationFactor, Multiplier: backoff.DefaultMultiplier, }, - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "otel_metrics_custom_gauge", Sum: "otel_metrics_custom_sum", Summary: "otel_metrics_custom_summary", @@ -109,43 +109,7 @@ func withDefaultConfig(fns ...func(*Config)) *Config { return cfg } -func TestValidateMetricTableNames(t *testing.T) { - tests := []struct { - name string - cfg Config - wantErr error - }{ - { - name: "metric_table_name not set", - cfg: Config{}, - wantErr: nil, - }, - { - name: "metric_table_name set", - cfg: Config{ - MetricsTableName: "table", - }, - wantErr: nil, - }, - { - name: "metric_table_name set with metric_tables", - cfg: Config{ - MetricsTableName: "table", - MetricsTables: TableNames{ - Gauge: "gauge", - }, - }, - wantErr: fmt.Errorf("Warning: 'metrics_table_name' is deprecated, use 'metrics_tables' instead"), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.wantErr, tt.cfg.ValidateMetricTableNames()) - }) - } -} - -func TestBuildMetricTableNames(t *testing.T) { +func TestBuildMetricMetricTableNames(t *testing.T) { tests := []struct { name string cfg Config @@ -155,7 +119,7 @@ func TestBuildMetricTableNames(t *testing.T) { name: "nothing set", cfg: Config{}, want: Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "otel_metrics_gauge", Sum: "otel_metrics_sum", Summary: "otel_metrics_summary", @@ -171,7 +135,7 @@ func TestBuildMetricTableNames(t *testing.T) { }, want: Config{ MetricsTableName: "table_name", - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "table_name_gauge", Sum: "table_name_sum", Summary: "table_name_summary", @@ -183,7 +147,7 @@ func TestBuildMetricTableNames(t *testing.T) { { name: "only metric_tables set fully", cfg: Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "table_name_gauge", Sum: "table_name_sum", Summary: "table_name_summary", @@ -192,7 +156,7 @@ func TestBuildMetricTableNames(t *testing.T) { }, }, want: Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "table_name_gauge", Sum: "table_name_sum", Summary: "table_name_summary", @@ -204,14 +168,14 @@ func TestBuildMetricTableNames(t *testing.T) { { name: "only metric_tables set partially", cfg: Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Summary: "table_name_summary", Histogram: "table_name_histogram", ExponentialHistogram: "table_name_exp_histogram", }, }, want: Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "otel_metrics_gauge", Sum: "otel_metrics_sum", Summary: "table_name_summary", @@ -224,7 +188,7 @@ func TestBuildMetricTableNames(t *testing.T) { name: "only metric_tables set partially with metric_table_name", cfg: Config{ MetricsTableName: "custom_name", - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Summary: "table_name_summary", Histogram: "table_name_histogram", ExponentialHistogram: "table_name_exp_histogram", @@ -232,7 +196,7 @@ func TestBuildMetricTableNames(t *testing.T) { }, want: Config{ MetricsTableName: "custom_name", - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "otel_metrics_gauge", Sum: "otel_metrics_sum", Summary: "table_name_summary", @@ -244,22 +208,22 @@ func TestBuildMetricTableNames(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.cfg.buildTableNames() + tt.cfg.buildMetricTableNames() require.Equal(t, tt.want, tt.cfg) }) } } -func TestAreTableNamesSet(t *testing.T) { +func TestAreMetricTableNamesSet(t *testing.T) { cfg := Config{} - require.False(t, cfg.areTableNamesSet()) + require.False(t, cfg.areMetricTableNamesSet()) cfg = Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "gauge", }, } - require.True(t, cfg.areTableNamesSet()) + require.True(t, cfg.areMetricTableNamesSet()) } func TestConfig_buildDSN(t *testing.T) { diff --git a/exporter/clickhouseexporter/exporter_metrics.go b/exporter/clickhouseexporter/exporter_metrics.go index 7c924485354f..f1113601ef46 100644 --- a/exporter/clickhouseexporter/exporter_metrics.go +++ b/exporter/clickhouseexporter/exporter_metrics.go @@ -19,8 +19,9 @@ import ( type metricsExporter struct { client *sql.DB - logger *zap.Logger - cfg *Config + logger *zap.Logger + cfg *Config + tableNames internal.MetricTablesConfigMapper } func newMetricsExporter(logger *zap.Logger, cfg *Config) (*metricsExporter, error) { @@ -29,10 +30,13 @@ func newMetricsExporter(logger *zap.Logger, cfg *Config) (*metricsExporter, erro return nil, err } + tableNames := generateMetricTablesConfigMapper(cfg) + return &metricsExporter{ - client: client, - logger: logger, - cfg: cfg, + client: client, + logger: logger, + cfg: cfg, + tableNames: tableNames, }, nil } @@ -48,17 +52,26 @@ func (e *metricsExporter) start(ctx context.Context, _ component.Host) error { } ttlExpr := generateTTLExpr(e.cfg.TTL, "toDateTime(TimeUnix)") - tableNames := e.generateTableNames() - return internal.NewMetricsTable(ctx, tableNames, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client) + return internal.NewMetricsTable(ctx, e.tableNames, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client) } -func (e *metricsExporter) generateTableNames() internal.MetricTableNames { - return internal.MetricTableNames{ - pmetric.MetricTypeGauge: e.cfg.MetricsTables.Gauge, - pmetric.MetricTypeSum: e.cfg.MetricsTables.Sum, - pmetric.MetricTypeSummary: e.cfg.MetricsTables.Summary, - pmetric.MetricTypeHistogram: e.cfg.MetricsTables.Histogram, - pmetric.MetricTypeExponentialHistogram: e.cfg.MetricsTables.ExponentialHistogram, +func generateMetricTablesConfigMapper(cfg *Config) internal.MetricTablesConfigMapper { + return internal.MetricTablesConfigMapper{ + pmetric.MetricTypeGauge: { + Name: cfg.MetricsTables.Gauge, + }, + pmetric.MetricTypeSum: { + Name: cfg.MetricsTables.Sum, + }, + pmetric.MetricTypeSummary: { + Name: cfg.MetricsTables.Summary, + }, + pmetric.MetricTypeHistogram: { + Name: cfg.MetricsTables.Histogram, + }, + pmetric.MetricTypeExponentialHistogram: { + Name: cfg.MetricsTables.ExponentialHistogram, + }, } } @@ -71,8 +84,7 @@ func (e *metricsExporter) shutdown(_ context.Context) error { } func (e *metricsExporter) pushMetricsData(ctx context.Context, md pmetric.Metrics) error { - tableNames := e.generateTableNames() - metricsMap := internal.NewMetricsModel(tableNames) + metricsMap := internal.NewMetricsModel(e.tableNames) for i := 0; i < md.ResourceMetrics().Len(); i++ { metrics := md.ResourceMetrics().At(i) resAttr := attributesToMap(metrics.Resource().Attributes()) diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index 9c193fc2c789..7eed425dd534 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -35,9 +35,9 @@ func TestMetricsTableEngineConfig(t *testing.T) { }) } -func Test_generateMetricTableNames(t *testing.T) { +func Test_generateMetricMetricTableNames(t *testing.T) { cfg := Config{ - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: "otel_metrics_custom_gauge", Sum: "otel_metrics_custom_sum", Summary: "otel_metrics_custom_summary", @@ -45,17 +45,24 @@ func Test_generateMetricTableNames(t *testing.T) { ExponentialHistogram: "otel_metrics_custom_exp_histogram", }, } - exporter := metricsExporter{ - cfg: &cfg, - } - require.Equal(t, internal.MetricTableNames{ - pmetric.MetricTypeGauge: cfg.MetricsTables.Gauge, - pmetric.MetricTypeSum: cfg.MetricsTables.Sum, - pmetric.MetricTypeSummary: cfg.MetricsTables.Summary, - pmetric.MetricTypeHistogram: cfg.MetricsTables.Histogram, - pmetric.MetricTypeExponentialHistogram: cfg.MetricsTables.ExponentialHistogram, - }, exporter.generateTableNames()) + require.Equal(t, internal.MetricTablesConfigMapper{ + pmetric.MetricTypeGauge: { + Name: cfg.MetricsTables.Gauge, + }, + pmetric.MetricTypeSum: { + Name: cfg.MetricsTables.Sum, + }, + pmetric.MetricTypeSummary: { + Name: cfg.MetricsTables.Summary, + }, + pmetric.MetricTypeHistogram: { + Name: cfg.MetricsTables.Histogram, + }, + pmetric.MetricTypeExponentialHistogram: { + Name: cfg.MetricsTables.ExponentialHistogram, + }, + }, generateMetricTablesConfigMapper(&cfg)) } func TestExporter_pushMetricsData(t *testing.T) { diff --git a/exporter/clickhouseexporter/factory.go b/exporter/clickhouseexporter/factory.go index 0bb84633d747..61e7d21d9cb9 100644 --- a/exporter/clickhouseexporter/factory.go +++ b/exporter/clickhouseexporter/factory.go @@ -41,7 +41,7 @@ func createDefaultConfig() component.Config { TTL: 0, CreateSchema: true, AsyncInsert: true, - MetricsTables: TableNames{ + MetricsTables: MetricTableNames{ Gauge: defaultMetricTableName + defaultGaugeSuffix, Sum: defaultMetricTableName + defaultSumSuffix, Summary: defaultMetricTableName + defaultSummarySuffix, diff --git a/exporter/clickhouseexporter/internal/metrics_model.go b/exporter/clickhouseexporter/internal/metrics_model.go index a7664756c8a4..ab4ea4226c37 100644 --- a/exporter/clickhouseexporter/internal/metrics_model.go +++ b/exporter/clickhouseexporter/internal/metrics_model.go @@ -18,7 +18,7 @@ import ( "go.uber.org/zap" ) -var supportedMetricTypes = MetricTableNames{ +var supportedMetricTypes = map[pmetric.MetricType]string{ pmetric.MetricTypeGauge: createGaugeTableSQL, pmetric.MetricTypeSum: createSumTableSQL, pmetric.MetricTypeHistogram: createHistogramTableSQL, @@ -28,7 +28,11 @@ var supportedMetricTypes = MetricTableNames{ var logger *zap.Logger -type MetricTableNames map[pmetric.MetricType]string +type MetricTablesConfigMapper map[pmetric.MetricType]MetricTableConfig + +type MetricTableConfig struct { + Name string +} // MetricsModel is used to group metric data and insert into clickhouse // any type of metrics need implement it. @@ -53,9 +57,9 @@ func SetLogger(l *zap.Logger) { } // NewMetricsTable create metric tables with an expiry time to storage metric telemetry data -func NewMetricsTable(ctx context.Context, tableNames MetricTableNames, cluster, engine, ttlExpr string, db *sql.DB) error { - for key, table := range supportedMetricTypes { - query := fmt.Sprintf(table, tableNames[key], cluster, engine, ttlExpr) +func NewMetricsTable(ctx context.Context, tableNames MetricTablesConfigMapper, cluster, engine, ttlExpr string, db *sql.DB) error { + for key, query := range supportedMetricTypes { + query := fmt.Sprintf(query, tableNames[key].Name, cluster, engine, ttlExpr) if _, err := db.ExecContext(ctx, query); err != nil { return fmt.Errorf("exec create metrics table sql: %w", err) } @@ -64,7 +68,7 @@ func NewMetricsTable(ctx context.Context, tableNames MetricTableNames, cluster, } // NewMetricsModel create a model for contain different metric data -func NewMetricsModel(tableNames MetricTableNames) map[pmetric.MetricType]MetricsModel { +func NewMetricsModel(tableNames MetricTablesConfigMapper) map[pmetric.MetricType]MetricsModel { return map[pmetric.MetricType]MetricsModel{ pmetric.MetricTypeGauge: &gaugeMetrics{ insertSQL: fmt.Sprintf(insertGaugeTableSQL, tableNames[pmetric.MetricTypeGauge]), From fa16a086472274d69991a8ad81abf658653ede56 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 2 Aug 2024 11:04:14 +0200 Subject: [PATCH 03/12] fix lint Signed-off-by: odubajDT --- exporter/clickhouseexporter/internal/metrics_model.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/clickhouseexporter/internal/metrics_model.go b/exporter/clickhouseexporter/internal/metrics_model.go index ab4ea4226c37..70a8dae4b4a1 100644 --- a/exporter/clickhouseexporter/internal/metrics_model.go +++ b/exporter/clickhouseexporter/internal/metrics_model.go @@ -58,8 +58,8 @@ func SetLogger(l *zap.Logger) { // NewMetricsTable create metric tables with an expiry time to storage metric telemetry data func NewMetricsTable(ctx context.Context, tableNames MetricTablesConfigMapper, cluster, engine, ttlExpr string, db *sql.DB) error { - for key, query := range supportedMetricTypes { - query := fmt.Sprintf(query, tableNames[key].Name, cluster, engine, ttlExpr) + for key, queryTemplate := range supportedMetricTypes { + query := fmt.Sprintf(queryTemplate, tableNames[key].Name, cluster, engine, ttlExpr) if _, err := db.ExecContext(ctx, query); err != nil { return fmt.Errorf("exec create metrics table sql: %w", err) } From 8c365d39b2480aca9ad04a084facc0baf6f1f612 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 2 Aug 2024 11:23:12 +0200 Subject: [PATCH 04/12] standardize config with internal structs Signed-off-by: odubajDT --- exporter/clickhouseexporter/README.md | 15 ++-- exporter/clickhouseexporter/config.go | 41 ++++----- exporter/clickhouseexporter/config_test.go | 85 ++++++++++--------- .../clickhouseexporter/exporter_metrics.go | 20 ++--- .../exporter_metrics_test.go | 30 +++---- exporter/clickhouseexporter/factory.go | 11 +-- .../internal/metrics_model.go | 6 +- .../clickhouseexporter/testdata/config.yaml | 15 ++-- 8 files changed, 108 insertions(+), 115 deletions(-) diff --git a/exporter/clickhouseexporter/README.md b/exporter/clickhouseexporter/README.md index 3f956b064623..d1a5156a010b 100644 --- a/exporter/clickhouseexporter/README.md +++ b/exporter/clickhouseexporter/README.md @@ -367,11 +367,16 @@ exporters: traces_table_name: otel_traces timeout: 5s metrics_tables: - gauge: "otel_metrics_gauge" - sum: "otel_metrics_sum" - summary: "otel_metrics_summary" - histogram: "otel_metrics_histogram" - exponential_histogram: "otel_metrics_exp_histogram" + gauge: + name: "otel_metrics_gauge" + sum: + name: "otel_metrics_sum" + summary: + name: "otel_metrics_summary" + histogram: + name: "otel_metrics_histogram" + exponential_histogram: + name: "otel_metrics_exp_histogram" retry_on_failure: enabled: true initial_interval: 5s diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 93c43b7c601e..dc2703e432e7 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -11,6 +11,7 @@ import ( "time" "github.com/ClickHouse/clickhouse-go/v2" + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configretry" "go.opentelemetry.io/collector/exporter/exporterhelper" @@ -58,15 +59,15 @@ type Config struct { type MetricTableNames struct { // Gauge is the table name for gauge metric type. default is `otel_metrics_gauge`. - Gauge string `mapstructure:"gauge"` + Gauge internal.MetricTypeConfig `mapstructure:"gauge"` // Sum is the table name for sum metric type. default is `otel_metrics_sum`. - Sum string `mapstructure:"sum"` + Sum internal.MetricTypeConfig `mapstructure:"sum"` // Summary is the table name for summary metric type. default is `otel_metrics_summary`. - Summary string `mapstructure:"summary"` + Summary internal.MetricTypeConfig `mapstructure:"summary"` // Histogram is the table name for histogram metric type. default is `otel_metrics_histogram`. - Histogram string `mapstructure:"histogram"` + Histogram internal.MetricTypeConfig `mapstructure:"histogram"` // ExponentialHistogram is the table name for exponential histogram metric type. default is `otel_metrics_exponential_histogram`. - ExponentialHistogram string `mapstructure:"exponential_histogram"` + ExponentialHistogram internal.MetricTypeConfig `mapstructure:"exponential_histogram"` } // TableEngine defines the ENGINE string value when creating the table. @@ -183,29 +184,29 @@ func (cfg *Config) buildMetricTableNames() { tableName = cfg.MetricsTableName } - if len(cfg.MetricsTables.Gauge) == 0 { - cfg.MetricsTables.Gauge = tableName + defaultGaugeSuffix + if len(cfg.MetricsTables.Gauge.Name) == 0 { + cfg.MetricsTables.Gauge.Name = tableName + defaultGaugeSuffix } - if len(cfg.MetricsTables.Sum) == 0 { - cfg.MetricsTables.Sum = tableName + defaultSumSuffix + if len(cfg.MetricsTables.Sum.Name) == 0 { + cfg.MetricsTables.Sum.Name = tableName + defaultSumSuffix } - if len(cfg.MetricsTables.Summary) == 0 { - cfg.MetricsTables.Summary = tableName + defaultSummarySuffix + if len(cfg.MetricsTables.Summary.Name) == 0 { + cfg.MetricsTables.Summary.Name = tableName + defaultSummarySuffix } - if len(cfg.MetricsTables.Histogram) == 0 { - cfg.MetricsTables.Histogram = tableName + defaultHistogramSuffix + if len(cfg.MetricsTables.Histogram.Name) == 0 { + cfg.MetricsTables.Histogram.Name = tableName + defaultHistogramSuffix } - if len(cfg.MetricsTables.ExponentialHistogram) == 0 { - cfg.MetricsTables.ExponentialHistogram = tableName + defaultExpHistogramSuffix + if len(cfg.MetricsTables.ExponentialHistogram.Name) == 0 { + cfg.MetricsTables.ExponentialHistogram.Name = tableName + defaultExpHistogramSuffix } } func (cfg *Config) areMetricTableNamesSet() bool { - return len(cfg.MetricsTables.Gauge) != 0 || - len(cfg.MetricsTables.Sum) != 0 || - len(cfg.MetricsTables.Summary) != 0 || - len(cfg.MetricsTables.Histogram) != 0 || - len(cfg.MetricsTables.ExponentialHistogram) != 0 + return len(cfg.MetricsTables.Gauge.Name) != 0 || + len(cfg.MetricsTables.Sum.Name) != 0 || + len(cfg.MetricsTables.Summary.Name) != 0 || + len(cfg.MetricsTables.Histogram.Name) != 0 || + len(cfg.MetricsTables.ExponentialHistogram.Name) != 0 } // tableEngineString generates the ENGINE string. diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index d01cc255370f..d8db7cdbe4af 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -19,6 +19,7 @@ import ( "go.opentelemetry.io/collector/confmap/confmaptest" "go.opentelemetry.io/collector/exporter/exporterhelper" + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal/metadata" ) @@ -68,11 +69,11 @@ func TestLoadConfig(t *testing.T) { Multiplier: backoff.DefaultMultiplier, }, MetricsTables: MetricTableNames{ - Gauge: "otel_metrics_custom_gauge", - Sum: "otel_metrics_custom_sum", - Summary: "otel_metrics_custom_summary", - Histogram: "otel_metrics_custom_histogram", - ExponentialHistogram: "otel_metrics_custom_exp_histogram", + Gauge: internal.MetricTypeConfig{Name: "otel_metrics_custom_gauge"}, + Sum: internal.MetricTypeConfig{Name: "otel_metrics_custom_sum"}, + Summary: internal.MetricTypeConfig{Name: "otel_metrics_custom_summary"}, + Histogram: internal.MetricTypeConfig{Name: "otel_metrics_custom_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "otel_metrics_custom_exp_histogram"}, }, ConnectionParams: map[string]string{}, QueueSettings: exporterhelper.QueueConfig{ @@ -120,11 +121,11 @@ func TestBuildMetricMetricTableNames(t *testing.T) { cfg: Config{}, want: Config{ MetricsTables: MetricTableNames{ - Gauge: "otel_metrics_gauge", - Sum: "otel_metrics_sum", - Summary: "otel_metrics_summary", - Histogram: "otel_metrics_histogram", - ExponentialHistogram: "otel_metrics_exponential_histogram", + Gauge: internal.MetricTypeConfig{Name: "otel_metrics_gauge"}, + Sum: internal.MetricTypeConfig{Name: "otel_metrics_sum"}, + Summary: internal.MetricTypeConfig{Name: "otel_metrics_summary"}, + Histogram: internal.MetricTypeConfig{Name: "otel_metrics_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "otel_metrics_exponential_histogram"}, }, }, }, @@ -136,11 +137,11 @@ func TestBuildMetricMetricTableNames(t *testing.T) { want: Config{ MetricsTableName: "table_name", MetricsTables: MetricTableNames{ - Gauge: "table_name_gauge", - Sum: "table_name_sum", - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exponential_histogram", + Gauge: internal.MetricTypeConfig{Name: "table_name_gauge"}, + Sum: internal.MetricTypeConfig{Name: "table_name_sum"}, + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exponential_histogram"}, }, }, }, @@ -148,20 +149,20 @@ func TestBuildMetricMetricTableNames(t *testing.T) { name: "only metric_tables set fully", cfg: Config{ MetricsTables: MetricTableNames{ - Gauge: "table_name_gauge", - Sum: "table_name_sum", - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exponential_histogram", + Gauge: internal.MetricTypeConfig{Name: "table_name_gauge"}, + Sum: internal.MetricTypeConfig{Name: "table_name_sum"}, + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exponential_histogram"}, }, }, want: Config{ MetricsTables: MetricTableNames{ - Gauge: "table_name_gauge", - Sum: "table_name_sum", - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exponential_histogram", + Gauge: internal.MetricTypeConfig{Name: "table_name_gauge"}, + Sum: internal.MetricTypeConfig{Name: "table_name_sum"}, + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exponential_histogram"}, }, }, }, @@ -169,18 +170,18 @@ func TestBuildMetricMetricTableNames(t *testing.T) { name: "only metric_tables set partially", cfg: Config{ MetricsTables: MetricTableNames{ - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exp_histogram", + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exp_histogram"}, }, }, want: Config{ MetricsTables: MetricTableNames{ - Gauge: "otel_metrics_gauge", - Sum: "otel_metrics_sum", - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exp_histogram", + Gauge: internal.MetricTypeConfig{Name: "otel_metrics_gauge"}, + Sum: internal.MetricTypeConfig{Name: "otel_metrics_sum"}, + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exp_histogram"}, }, }, }, @@ -189,19 +190,19 @@ func TestBuildMetricMetricTableNames(t *testing.T) { cfg: Config{ MetricsTableName: "custom_name", MetricsTables: MetricTableNames{ - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exp_histogram", + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exp_histogram"}, }, }, want: Config{ MetricsTableName: "custom_name", MetricsTables: MetricTableNames{ - Gauge: "otel_metrics_gauge", - Sum: "otel_metrics_sum", - Summary: "table_name_summary", - Histogram: "table_name_histogram", - ExponentialHistogram: "table_name_exp_histogram", + Gauge: internal.MetricTypeConfig{Name: "otel_metrics_gauge"}, + Sum: internal.MetricTypeConfig{Name: "otel_metrics_sum"}, + Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, + Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exp_histogram"}, }, }, }, @@ -220,7 +221,7 @@ func TestAreMetricTableNamesSet(t *testing.T) { cfg = Config{ MetricsTables: MetricTableNames{ - Gauge: "gauge", + Gauge: internal.MetricTypeConfig{Name: "gauge"}, }, } require.True(t, cfg.areMetricTableNamesSet()) diff --git a/exporter/clickhouseexporter/exporter_metrics.go b/exporter/clickhouseexporter/exporter_metrics.go index f1113601ef46..5172a50c5368 100644 --- a/exporter/clickhouseexporter/exporter_metrics.go +++ b/exporter/clickhouseexporter/exporter_metrics.go @@ -57,21 +57,11 @@ func (e *metricsExporter) start(ctx context.Context, _ component.Host) error { func generateMetricTablesConfigMapper(cfg *Config) internal.MetricTablesConfigMapper { return internal.MetricTablesConfigMapper{ - pmetric.MetricTypeGauge: { - Name: cfg.MetricsTables.Gauge, - }, - pmetric.MetricTypeSum: { - Name: cfg.MetricsTables.Sum, - }, - pmetric.MetricTypeSummary: { - Name: cfg.MetricsTables.Summary, - }, - pmetric.MetricTypeHistogram: { - Name: cfg.MetricsTables.Histogram, - }, - pmetric.MetricTypeExponentialHistogram: { - Name: cfg.MetricsTables.ExponentialHistogram, - }, + pmetric.MetricTypeGauge: cfg.MetricsTables.Gauge, + pmetric.MetricTypeSum: cfg.MetricsTables.Sum, + pmetric.MetricTypeSummary: cfg.MetricsTables.Summary, + pmetric.MetricTypeHistogram: cfg.MetricsTables.Histogram, + pmetric.MetricTypeExponentialHistogram: cfg.MetricsTables.ExponentialHistogram, } } diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index 7eed425dd534..beb3defa76b7 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -38,30 +38,20 @@ func TestMetricsTableEngineConfig(t *testing.T) { func Test_generateMetricMetricTableNames(t *testing.T) { cfg := Config{ MetricsTables: MetricTableNames{ - Gauge: "otel_metrics_custom_gauge", - Sum: "otel_metrics_custom_sum", - Summary: "otel_metrics_custom_summary", - Histogram: "otel_metrics_custom_histogram", - ExponentialHistogram: "otel_metrics_custom_exp_histogram", + Gauge: internal.MetricTypeConfig{Name: "otel_metrics_custom_gauge"}, + Sum: internal.MetricTypeConfig{Name: "otel_metrics_custom_sum"}, + Summary: internal.MetricTypeConfig{Name: "otel_metrics_custom_summary"}, + Histogram: internal.MetricTypeConfig{Name: "otel_metrics_custom_histogram"}, + ExponentialHistogram: internal.MetricTypeConfig{Name: "otel_metrics_custom_exp_histogram"}, }, } require.Equal(t, internal.MetricTablesConfigMapper{ - pmetric.MetricTypeGauge: { - Name: cfg.MetricsTables.Gauge, - }, - pmetric.MetricTypeSum: { - Name: cfg.MetricsTables.Sum, - }, - pmetric.MetricTypeSummary: { - Name: cfg.MetricsTables.Summary, - }, - pmetric.MetricTypeHistogram: { - Name: cfg.MetricsTables.Histogram, - }, - pmetric.MetricTypeExponentialHistogram: { - Name: cfg.MetricsTables.ExponentialHistogram, - }, + pmetric.MetricTypeGauge: cfg.MetricsTables.Gauge, + pmetric.MetricTypeSum: cfg.MetricsTables.Sum, + pmetric.MetricTypeSummary: cfg.MetricsTables.Summary, + pmetric.MetricTypeHistogram: cfg.MetricsTables.Histogram, + pmetric.MetricTypeExponentialHistogram: cfg.MetricsTables.ExponentialHistogram, }, generateMetricTablesConfigMapper(&cfg)) } diff --git a/exporter/clickhouseexporter/factory.go b/exporter/clickhouseexporter/factory.go index 61e7d21d9cb9..b6670267740c 100644 --- a/exporter/clickhouseexporter/factory.go +++ b/exporter/clickhouseexporter/factory.go @@ -15,6 +15,7 @@ import ( "go.opentelemetry.io/collector/exporter" "go.opentelemetry.io/collector/exporter/exporterhelper" + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal/metadata" ) @@ -42,11 +43,11 @@ func createDefaultConfig() component.Config { CreateSchema: true, AsyncInsert: true, MetricsTables: MetricTableNames{ - Gauge: defaultMetricTableName + defaultGaugeSuffix, - Sum: defaultMetricTableName + defaultSumSuffix, - Summary: defaultMetricTableName + defaultSummarySuffix, - Histogram: defaultMetricTableName + defaultHistogramSuffix, - ExponentialHistogram: defaultMetricTableName + defaultExpHistogramSuffix, + Gauge: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultGaugeSuffix}, + Sum: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultSumSuffix}, + Summary: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultSummarySuffix}, + Histogram: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultHistogramSuffix}, + ExponentialHistogram: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultExpHistogramSuffix}, }, } } diff --git a/exporter/clickhouseexporter/internal/metrics_model.go b/exporter/clickhouseexporter/internal/metrics_model.go index 70a8dae4b4a1..f11f423e657e 100644 --- a/exporter/clickhouseexporter/internal/metrics_model.go +++ b/exporter/clickhouseexporter/internal/metrics_model.go @@ -28,10 +28,10 @@ var supportedMetricTypes = map[pmetric.MetricType]string{ var logger *zap.Logger -type MetricTablesConfigMapper map[pmetric.MetricType]MetricTableConfig +type MetricTablesConfigMapper map[pmetric.MetricType]MetricTypeConfig -type MetricTableConfig struct { - Name string +type MetricTypeConfig struct { + Name string `mapstructure:"name"` } // MetricsModel is used to group metric data and insert into clickhouse diff --git a/exporter/clickhouseexporter/testdata/config.yaml b/exporter/clickhouseexporter/testdata/config.yaml index 87c006359fe9..9eb443cde3da 100644 --- a/exporter/clickhouseexporter/testdata/config.yaml +++ b/exporter/clickhouseexporter/testdata/config.yaml @@ -18,11 +18,16 @@ clickhouse/full: queue_size: 100 storage: file_storage/clickhouse metrics_tables: - gauge: "otel_metrics_custom_gauge" - sum: "otel_metrics_custom_sum" - summary: "otel_metrics_custom_summary" - histogram: "otel_metrics_custom_histogram" - exponential_histogram: "otel_metrics_custom_exp_histogram" + gauge: + name: "otel_metrics_custom_gauge" + sum: + name: "otel_metrics_custom_sum" + summary: + name: "otel_metrics_custom_summary" + histogram: + name: "otel_metrics_custom_histogram" + exponential_histogram: + name: "otel_metrics_custom_exp_histogram" clickhouse/invalid-endpoint: endpoint: 127.0.0.1:9000 From 26de919ee16e74df82c3a6bb5e88c7540d210c70 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 2 Aug 2024 11:30:04 +0200 Subject: [PATCH 05/12] polish naming Signed-off-by: odubajDT --- exporter/clickhouseexporter/config.go | 4 ++-- exporter/clickhouseexporter/config_test.go | 20 +++++++++---------- .../clickhouseexporter/exporter_metrics.go | 20 +++++++++---------- .../exporter_metrics_test.go | 2 +- exporter/clickhouseexporter/factory.go | 2 +- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index dc2703e432e7..4ad470df195e 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -54,10 +54,10 @@ type Config struct { // Async inserts may still be overridden server-side. AsyncInsert bool `mapstructure:"async_insert"` // MetricsTables defines the table names for metric types. - MetricsTables MetricTableNames `mapstructure:"metrics_tables"` + MetricsTables MetricTablesConfig `mapstructure:"metrics_tables"` } -type MetricTableNames struct { +type MetricTablesConfig struct { // Gauge is the table name for gauge metric type. default is `otel_metrics_gauge`. Gauge internal.MetricTypeConfig `mapstructure:"gauge"` // Sum is the table name for sum metric type. default is `otel_metrics_sum`. diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index d8db7cdbe4af..2f541696fa65 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -68,7 +68,7 @@ func TestLoadConfig(t *testing.T) { RandomizationFactor: backoff.DefaultRandomizationFactor, Multiplier: backoff.DefaultMultiplier, }, - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "otel_metrics_custom_gauge"}, Sum: internal.MetricTypeConfig{Name: "otel_metrics_custom_sum"}, Summary: internal.MetricTypeConfig{Name: "otel_metrics_custom_summary"}, @@ -120,7 +120,7 @@ func TestBuildMetricMetricTableNames(t *testing.T) { name: "nothing set", cfg: Config{}, want: Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "otel_metrics_gauge"}, Sum: internal.MetricTypeConfig{Name: "otel_metrics_sum"}, Summary: internal.MetricTypeConfig{Name: "otel_metrics_summary"}, @@ -136,7 +136,7 @@ func TestBuildMetricMetricTableNames(t *testing.T) { }, want: Config{ MetricsTableName: "table_name", - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "table_name_gauge"}, Sum: internal.MetricTypeConfig{Name: "table_name_sum"}, Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, @@ -148,7 +148,7 @@ func TestBuildMetricMetricTableNames(t *testing.T) { { name: "only metric_tables set fully", cfg: Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "table_name_gauge"}, Sum: internal.MetricTypeConfig{Name: "table_name_sum"}, Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, @@ -157,7 +157,7 @@ func TestBuildMetricMetricTableNames(t *testing.T) { }, }, want: Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "table_name_gauge"}, Sum: internal.MetricTypeConfig{Name: "table_name_sum"}, Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, @@ -169,14 +169,14 @@ func TestBuildMetricMetricTableNames(t *testing.T) { { name: "only metric_tables set partially", cfg: Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exp_histogram"}, }, }, want: Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "otel_metrics_gauge"}, Sum: internal.MetricTypeConfig{Name: "otel_metrics_sum"}, Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, @@ -189,7 +189,7 @@ func TestBuildMetricMetricTableNames(t *testing.T) { name: "only metric_tables set partially with metric_table_name", cfg: Config{ MetricsTableName: "custom_name", - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, Histogram: internal.MetricTypeConfig{Name: "table_name_histogram"}, ExponentialHistogram: internal.MetricTypeConfig{Name: "table_name_exp_histogram"}, @@ -197,7 +197,7 @@ func TestBuildMetricMetricTableNames(t *testing.T) { }, want: Config{ MetricsTableName: "custom_name", - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "otel_metrics_gauge"}, Sum: internal.MetricTypeConfig{Name: "otel_metrics_sum"}, Summary: internal.MetricTypeConfig{Name: "table_name_summary"}, @@ -220,7 +220,7 @@ func TestAreMetricTableNamesSet(t *testing.T) { require.False(t, cfg.areMetricTableNamesSet()) cfg = Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "gauge"}, }, } diff --git a/exporter/clickhouseexporter/exporter_metrics.go b/exporter/clickhouseexporter/exporter_metrics.go index 5172a50c5368..66f67b5cc065 100644 --- a/exporter/clickhouseexporter/exporter_metrics.go +++ b/exporter/clickhouseexporter/exporter_metrics.go @@ -19,9 +19,9 @@ import ( type metricsExporter struct { client *sql.DB - logger *zap.Logger - cfg *Config - tableNames internal.MetricTablesConfigMapper + logger *zap.Logger + cfg *Config + tablesConfig internal.MetricTablesConfigMapper } func newMetricsExporter(logger *zap.Logger, cfg *Config) (*metricsExporter, error) { @@ -30,13 +30,13 @@ func newMetricsExporter(logger *zap.Logger, cfg *Config) (*metricsExporter, erro return nil, err } - tableNames := generateMetricTablesConfigMapper(cfg) + tablesConfig := generateMetricTablesConfigMapper(cfg) return &metricsExporter{ - client: client, - logger: logger, - cfg: cfg, - tableNames: tableNames, + client: client, + logger: logger, + cfg: cfg, + tablesConfig: tablesConfig, }, nil } @@ -52,7 +52,7 @@ func (e *metricsExporter) start(ctx context.Context, _ component.Host) error { } ttlExpr := generateTTLExpr(e.cfg.TTL, "toDateTime(TimeUnix)") - return internal.NewMetricsTable(ctx, e.tableNames, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client) + return internal.NewMetricsTable(ctx, e.tablesConfig, e.cfg.clusterString(), e.cfg.tableEngineString(), ttlExpr, e.client) } func generateMetricTablesConfigMapper(cfg *Config) internal.MetricTablesConfigMapper { @@ -74,7 +74,7 @@ func (e *metricsExporter) shutdown(_ context.Context) error { } func (e *metricsExporter) pushMetricsData(ctx context.Context, md pmetric.Metrics) error { - metricsMap := internal.NewMetricsModel(e.tableNames) + metricsMap := internal.NewMetricsModel(e.tablesConfig) for i := 0; i < md.ResourceMetrics().Len(); i++ { metrics := md.ResourceMetrics().At(i) resAttr := attributesToMap(metrics.Resource().Attributes()) diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index beb3defa76b7..2000303cab67 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -37,7 +37,7 @@ func TestMetricsTableEngineConfig(t *testing.T) { func Test_generateMetricMetricTableNames(t *testing.T) { cfg := Config{ - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: "otel_metrics_custom_gauge"}, Sum: internal.MetricTypeConfig{Name: "otel_metrics_custom_sum"}, Summary: internal.MetricTypeConfig{Name: "otel_metrics_custom_summary"}, diff --git a/exporter/clickhouseexporter/factory.go b/exporter/clickhouseexporter/factory.go index b6670267740c..6faf3e3acfdb 100644 --- a/exporter/clickhouseexporter/factory.go +++ b/exporter/clickhouseexporter/factory.go @@ -42,7 +42,7 @@ func createDefaultConfig() component.Config { TTL: 0, CreateSchema: true, AsyncInsert: true, - MetricsTables: MetricTableNames{ + MetricsTables: MetricTablesConfig{ Gauge: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultGaugeSuffix}, Sum: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultSumSuffix}, Summary: internal.MetricTypeConfig{Name: defaultMetricTableName + defaultSummarySuffix}, From 34c564cc21fd0e5729dcdea9b04f0769a21e5ea5 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 2 Aug 2024 11:34:07 +0200 Subject: [PATCH 06/12] polish naming Signed-off-by: odubajDT --- .../clickhouseexporter/internal/metrics_model.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/exporter/clickhouseexporter/internal/metrics_model.go b/exporter/clickhouseexporter/internal/metrics_model.go index f11f423e657e..92c51b550643 100644 --- a/exporter/clickhouseexporter/internal/metrics_model.go +++ b/exporter/clickhouseexporter/internal/metrics_model.go @@ -57,9 +57,9 @@ func SetLogger(l *zap.Logger) { } // NewMetricsTable create metric tables with an expiry time to storage metric telemetry data -func NewMetricsTable(ctx context.Context, tableNames MetricTablesConfigMapper, cluster, engine, ttlExpr string, db *sql.DB) error { +func NewMetricsTable(ctx context.Context, tablesConfig MetricTablesConfigMapper, cluster, engine, ttlExpr string, db *sql.DB) error { for key, queryTemplate := range supportedMetricTypes { - query := fmt.Sprintf(queryTemplate, tableNames[key].Name, cluster, engine, ttlExpr) + query := fmt.Sprintf(queryTemplate, tablesConfig[key].Name, cluster, engine, ttlExpr) if _, err := db.ExecContext(ctx, query); err != nil { return fmt.Errorf("exec create metrics table sql: %w", err) } @@ -68,22 +68,22 @@ func NewMetricsTable(ctx context.Context, tableNames MetricTablesConfigMapper, c } // NewMetricsModel create a model for contain different metric data -func NewMetricsModel(tableNames MetricTablesConfigMapper) map[pmetric.MetricType]MetricsModel { +func NewMetricsModel(tablesConfig MetricTablesConfigMapper) map[pmetric.MetricType]MetricsModel { return map[pmetric.MetricType]MetricsModel{ pmetric.MetricTypeGauge: &gaugeMetrics{ - insertSQL: fmt.Sprintf(insertGaugeTableSQL, tableNames[pmetric.MetricTypeGauge]), + insertSQL: fmt.Sprintf(insertGaugeTableSQL, tablesConfig[pmetric.MetricTypeGauge].Name), }, pmetric.MetricTypeSum: &sumMetrics{ - insertSQL: fmt.Sprintf(insertSumTableSQL, tableNames[pmetric.MetricTypeSum]), + insertSQL: fmt.Sprintf(insertSumTableSQL, tablesConfig[pmetric.MetricTypeSum].Name), }, pmetric.MetricTypeHistogram: &histogramMetrics{ - insertSQL: fmt.Sprintf(insertHistogramTableSQL, tableNames[pmetric.MetricTypeHistogram]), + insertSQL: fmt.Sprintf(insertHistogramTableSQL, tablesConfig[pmetric.MetricTypeHistogram].Name), }, pmetric.MetricTypeExponentialHistogram: &expHistogramMetrics{ - insertSQL: fmt.Sprintf(insertExpHistogramTableSQL, tableNames[pmetric.MetricTypeExponentialHistogram]), + insertSQL: fmt.Sprintf(insertExpHistogramTableSQL, tablesConfig[pmetric.MetricTypeExponentialHistogram].Name), }, pmetric.MetricTypeSummary: &summaryMetrics{ - insertSQL: fmt.Sprintf(insertSummaryTableSQL, tableNames[pmetric.MetricTypeSummary]), + insertSQL: fmt.Sprintf(insertSummaryTableSQL, tablesConfig[pmetric.MetricTypeSummary].Name), }, } } From 9ea7c8be22717123cab2ab0e8034d071d5979e09 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 2 Aug 2024 11:39:39 +0200 Subject: [PATCH 07/12] lint Signed-off-by: odubajDT --- exporter/clickhouseexporter/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 4ad470df195e..348162765f96 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -11,10 +11,11 @@ import ( "time" "github.com/ClickHouse/clickhouse-go/v2" - "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configretry" "go.opentelemetry.io/collector/exporter/exporterhelper" + + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/clickhouseexporter/internal" ) // Config defines configuration for Elastic exporter. From e46ae10186cb7845932a58290f2205a780683e2e Mon Sep 17 00:00:00 2001 From: odubajDT Date: Mon, 5 Aug 2024 07:28:34 +0200 Subject: [PATCH 08/12] adapt readme after PR review Signed-off-by: odubajDT --- exporter/clickhouseexporter/README.md | 35 ++++++++++++++++----------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/exporter/clickhouseexporter/README.md b/exporter/clickhouseexporter/README.md index d1a5156a010b..a618262a6c4c 100644 --- a/exporter/clickhouseexporter/README.md +++ b/exporter/clickhouseexporter/README.md @@ -294,7 +294,17 @@ ClickHouse tables: - `logs_table_name` (default = otel_logs): The table name for logs. - `traces_table_name` (default = otel_traces): The table name for traces. -- `metrics_table_name` (default = otel_metrics): The table name for metrics. +- `metrics_tables` + - `gauge` + - `name` (default = "otel_metrics_gauge") + - `sum` + - `name` (default = "otel_metrics_sum") + - `summary` + - `name` (default = "otel_metrics_summary") + - `histogram` + - `name` (default = "otel_metrics_histogram") + - `exponential_histogram` + - `name` (default = "otel_metrics_exp_histogram") Cluster definition: @@ -367,16 +377,16 @@ exporters: traces_table_name: otel_traces timeout: 5s metrics_tables: - gauge: - name: "otel_metrics_gauge" - sum: - name: "otel_metrics_sum" - summary: - name: "otel_metrics_summary" - histogram: - name: "otel_metrics_histogram" - exponential_histogram: - name: "otel_metrics_exp_histogram" + gauge: + name: "otel_metrics_gauge" + sum: + name: "otel_metrics_sum" + summary: + name: "otel_metrics_summary" + histogram: + name: "otel_metrics_histogram" + exponential_histogram: + name: "otel_metrics_exp_histogram" retry_on_failure: enabled: true initial_interval: 5s @@ -394,9 +404,6 @@ service: exporters: [ clickhouse ] ``` -**Note:** -Parameter `metrics_table_name` will be deprecated and replaced by `metric_tables` which support setting the full table name (including suffix) for tables of each metric type. Using `metrics_table_name` and `metric_tables` together will result in ignoring the `metrics_table_name` parameter value. - ## Contributing Before contributing, review the contribution guidelines in [CONTRIBUTING.md](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md). From 7365b637c09902fceddea0bc01e54187a7dd4aef Mon Sep 17 00:00:00 2001 From: odubajDT Date: Mon, 16 Sep 2024 08:20:24 +0200 Subject: [PATCH 09/12] fix after rebase Signed-off-by: odubajDT --- exporter/clickhouseexporter/config_test.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index 2f541696fa65..b4b3ff73c633 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -48,15 +48,14 @@ func TestLoadConfig(t *testing.T) { { id: component.NewIDWithName(metadata.Type, "full"), expected: &Config{ - Endpoint: defaultEndpoint, - Database: "otel", - Username: "foo", - Password: "bar", - TTL: 72 * time.Hour, - LogsTableName: "otel_logs", - TracesTableName: "otel_traces", - MetricsTableName: "otel_metrics", - CreateSchema: true, + Endpoint: defaultEndpoint, + Database: "otel", + Username: "foo", + Password: "bar", + TTL: 72 * time.Hour, + LogsTableName: "otel_logs", + TracesTableName: "otel_traces", + CreateSchema: true, TimeoutSettings: exporterhelper.TimeoutConfig{ Timeout: 5 * time.Second, }, From 659825e5b8f202406e36272b4261b3ab6212f6a8 Mon Sep 17 00:00:00 2001 From: odubajDT <93584209+odubajDT@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:23:01 +0200 Subject: [PATCH 10/12] Update .chloggen/metrics-tables-clickhouse.yaml --- .chloggen/metrics-tables-clickhouse.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/metrics-tables-clickhouse.yaml b/.chloggen/metrics-tables-clickhouse.yaml index 19d55a8cef7b..d34648dd3a46 100644 --- a/.chloggen/metrics-tables-clickhouse.yaml +++ b/.chloggen/metrics-tables-clickhouse.yaml @@ -15,7 +15,7 @@ issues: [34225] # (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: +subtext: `metrics_table_name` of the clickhouseexported config is therefore deprecated and newly introduced parameter `metrics_tables` should be used instead. # 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. From 3d741771b60e29bcd9f0b5d8a9e9f89c155396ad Mon Sep 17 00:00:00 2001 From: odubajDT <93584209+odubajDT@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:23:36 +0200 Subject: [PATCH 11/12] Update .chloggen/metrics-tables-clickhouse.yaml --- .chloggen/metrics-tables-clickhouse.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/metrics-tables-clickhouse.yaml b/.chloggen/metrics-tables-clickhouse.yaml index d34648dd3a46..cd454447f0d2 100644 --- a/.chloggen/metrics-tables-clickhouse.yaml +++ b/.chloggen/metrics-tables-clickhouse.yaml @@ -15,7 +15,7 @@ issues: [34225] # (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: `metrics_table_name` of the clickhouseexported config is therefore deprecated and newly introduced parameter `metrics_tables` should be used instead. +subtext: `metrics_table_name` of the clickhouse exporter config is deprecated and newly introduced parameter `metrics_tables` should be used instead. # 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. From d47be57a11a06413d8daa62034841fb882285b11 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Thu, 19 Sep 2024 13:56:54 +0200 Subject: [PATCH 12/12] fix chlog Signed-off-by: odubajDT --- .chloggen/metrics-tables-clickhouse.yaml | 3 ++- exporter/clickhouseexporter/config.go | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.chloggen/metrics-tables-clickhouse.yaml b/.chloggen/metrics-tables-clickhouse.yaml index cd454447f0d2..fb63b0eeb94f 100644 --- a/.chloggen/metrics-tables-clickhouse.yaml +++ b/.chloggen/metrics-tables-clickhouse.yaml @@ -15,7 +15,8 @@ issues: [34225] # (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: `metrics_table_name` of the clickhouse exporter config is deprecated and newly introduced parameter `metrics_tables` should be used instead. +subtext: | + 'metrics_table_name' of the clickhouse exporter config is deprecated and newly introduced parameter 'metrics_tables' should be used instead. # 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. diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 348162765f96..f21d96a77830 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -39,6 +39,10 @@ type Config struct { // TracesTableName is the table name for traces. default is `otel_traces`. TracesTableName string `mapstructure:"traces_table_name"` // MetricsTableName is the table name for metrics. default is `otel_metrics`. + // + // Deprecated: MetricsTableName exists for historical compatibility + // and should not be used. To set the metrics tables name, + // use the MetricsTables parameter instead. MetricsTableName string `mapstructure:"metrics_table_name"` // TTL is The data time-to-live example 30m, 48h. 0 means no ttl. TTL time.Duration `mapstructure:"ttl"`