From 969eeacde6b5a1c1d72f3a7287c28fb5f00796ae Mon Sep 17 00:00:00 2001 From: "minquan.chen" Date: Tue, 26 Dec 2023 15:41:08 +0800 Subject: [PATCH 1/5] fix clickhouse exporter insert metrics data bug --- .../fix_ck_exporter_insert_metrics_bug.yaml | 27 +++++++++++++++++++ .../internal/exponential_histogram_metrics.go | 2 +- .../internal/gauge_metrics.go | 2 +- .../internal/histogram_metrics.go | 2 +- .../internal/sum_metrics.go | 2 +- 5 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 .chloggen/fix_ck_exporter_insert_metrics_bug.yaml diff --git a/.chloggen/fix_ck_exporter_insert_metrics_bug.yaml b/.chloggen/fix_ck_exporter_insert_metrics_bug.yaml new file mode 100644 index 000000000000..c95930b16d20 --- /dev/null +++ b/.chloggen/fix_ck_exporter_insert_metrics_bug.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: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: clickhouseexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix clickhouse exporter insert metrics data bug + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [] + +# (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: [user] diff --git a/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go b/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go index 421d967cd61c..7c97731bbb78 100644 --- a/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go +++ b/exporter/clickhouseexporter/internal/exponential_histogram_metrics.go @@ -154,8 +154,8 @@ func (e *expHistogramMetrics) insert(ctx context.Context, db *sql.DB) error { attrs, times, values, - traceIDs, spanIDs, + traceIDs, uint32(dp.Flags()), dp.Min(), dp.Max(), diff --git a/exporter/clickhouseexporter/internal/gauge_metrics.go b/exporter/clickhouseexporter/internal/gauge_metrics.go index d16caa78f56a..1a8a1a9e1978 100644 --- a/exporter/clickhouseexporter/internal/gauge_metrics.go +++ b/exporter/clickhouseexporter/internal/gauge_metrics.go @@ -128,8 +128,8 @@ func (g *gaugeMetrics) insert(ctx context.Context, db *sql.DB) error { attrs, times, values, - traceIDs, spanIDs, + traceIDs, ) if err != nil { return fmt.Errorf("ExecContext:%w", err) diff --git a/exporter/clickhouseexporter/internal/histogram_metrics.go b/exporter/clickhouseexporter/internal/histogram_metrics.go index 0ce19fe4d08b..6dbb8ee73c12 100644 --- a/exporter/clickhouseexporter/internal/histogram_metrics.go +++ b/exporter/clickhouseexporter/internal/histogram_metrics.go @@ -140,8 +140,8 @@ func (h *histogramMetrics) insert(ctx context.Context, db *sql.DB) error { attrs, times, values, - traceIDs, spanIDs, + traceIDs, uint32(dp.Flags()), dp.Min(), dp.Max(), diff --git a/exporter/clickhouseexporter/internal/sum_metrics.go b/exporter/clickhouseexporter/internal/sum_metrics.go index 69f9a9d1d5d5..b933e052bfb6 100644 --- a/exporter/clickhouseexporter/internal/sum_metrics.go +++ b/exporter/clickhouseexporter/internal/sum_metrics.go @@ -132,8 +132,8 @@ func (s *sumMetrics) insert(ctx context.Context, db *sql.DB) error { attrs, times, values, - traceIDs, spanIDs, + traceIDs, int32(model.sum.AggregationTemporality()), model.sum.IsMonotonic(), ) From bd6d1ffe06c2ab19933ec9737e0d600fc08402a7 Mon Sep 17 00:00:00 2001 From: "minquan.chen" Date: Tue, 26 Dec 2023 15:49:01 +0800 Subject: [PATCH 2/5] patch changelog --- .chloggen/fix_ck_exporter_insert_metrics_bug.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/fix_ck_exporter_insert_metrics_bug.yaml b/.chloggen/fix_ck_exporter_insert_metrics_bug.yaml index c95930b16d20..b57103ae6a9c 100644 --- a/.chloggen/fix_ck_exporter_insert_metrics_bug.yaml +++ b/.chloggen/fix_ck_exporter_insert_metrics_bug.yaml @@ -10,7 +10,7 @@ component: clickhouseexporter note: Fix clickhouse exporter insert metrics data bug # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. -issues: [] +issues: [30210] # (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. From f529e37011c50de3066a8be31a75fb03e805cd97 Mon Sep 17 00:00:00 2001 From: "minquan.chen" Date: Tue, 26 Dec 2023 16:40:35 +0800 Subject: [PATCH 3/5] add ut --- .../exporter_metrics_test.go | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index 4058a80bc8ac..e38547110ea8 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -5,13 +5,15 @@ package clickhouseexporter import ( "context" - "database/sql/driver" "fmt" + "regexp" "strings" "sync/atomic" "testing" "time" + "database/sql/driver" + "github.com/ClickHouse/clickhouse-go/v2" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" @@ -97,6 +99,30 @@ func TestExporter_pushMetricsData(t *testing.T) { require.Equal(t, int32(15), items.Load()) }) + t.Run("check traceID and spanID", func(t *testing.T) { + initClickhouseTestServer(t, func(query string, values []driver.Value) error { + var sumMetrics = regexp.MustCompile(`^INSERT INTO otel_metrics_sum `) + if strings.HasPrefix(query, "INSERT INTO otel_metrics_gauge") { + require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[18]) + require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[19]) + } + if strings.HasPrefix(query, "INSERT INTO otel_metrics_histogram") { + require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[20]) + require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[21]) + } + if sumMetrics.MatchString(query) { + require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[18]) + require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[19]) + } + if strings.HasPrefix(query, "INSERT INTO otel_metrics_exponential_histogram") { + require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[24]) + require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[25]) + } + return nil + }) + exporter := newTestMetricsExporter(t) + mustPushMetricsData(t, exporter, simpleMetrics(1)) + }) } func Benchmark_pushMetricsData(b *testing.B) { From 5ffc54ea2da974c6e6e4ea54a68f94351a871ede Mon Sep 17 00:00:00 2001 From: "minquan.chen" Date: Tue, 26 Dec 2023 18:16:03 +0800 Subject: [PATCH 4/5] fix lint --- exporter/clickhouseexporter/exporter_metrics_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index e38547110ea8..2834b9e552a4 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -5,6 +5,7 @@ package clickhouseexporter import ( "context" + "database/sql/driver" "fmt" "regexp" "strings" @@ -12,7 +13,6 @@ import ( "testing" "time" - "database/sql/driver" "github.com/ClickHouse/clickhouse-go/v2" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" From b8291470a66877ed36076a80a5199c09b19c8753 Mon Sep 17 00:00:00 2001 From: "minquan.chen" Date: Mon, 8 Jan 2024 17:18:48 +0800 Subject: [PATCH 5/5] apply reviewer's suggestion --- exporter/clickhouseexporter/exporter_metrics_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/exporter/clickhouseexporter/exporter_metrics_test.go b/exporter/clickhouseexporter/exporter_metrics_test.go index 2834b9e552a4..4d407131e257 100644 --- a/exporter/clickhouseexporter/exporter_metrics_test.go +++ b/exporter/clickhouseexporter/exporter_metrics_test.go @@ -7,7 +7,6 @@ import ( "context" "database/sql/driver" "fmt" - "regexp" "strings" "sync/atomic" "testing" @@ -101,7 +100,6 @@ func TestExporter_pushMetricsData(t *testing.T) { }) t.Run("check traceID and spanID", func(t *testing.T) { initClickhouseTestServer(t, func(query string, values []driver.Value) error { - var sumMetrics = regexp.MustCompile(`^INSERT INTO otel_metrics_sum `) if strings.HasPrefix(query, "INSERT INTO otel_metrics_gauge") { require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[18]) require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[19]) @@ -110,7 +108,7 @@ func TestExporter_pushMetricsData(t *testing.T) { require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[20]) require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[21]) } - if sumMetrics.MatchString(query) { + if strings.HasPrefix(query, "INSERT INTO otel_metrics_sum ") { require.Equal(t, clickhouse.ArraySet{"0102030000000000"}, values[18]) require.Equal(t, clickhouse.ArraySet{"01020300000000000000000000000000"}, values[19]) }