more comment

Signed-off-by: zzzk1 <[email protected]>

Author: zhengkezhou1

.github/workflows/ci-e2e-clickhouse.yml

@@ -34,9 +34,7 @@ jobs:
 
       - name: Run clickhouse integration tests
         id: test-execution
-        run: bash scripts/e2e/clickhouse.sh ${{ matrix.clickhouse-version }}-${{ matrix.create-schema }}
-        env:
-          SKIP_APPLY_SCHEMA: ${{ matrix.create-schema == 'auto' && true || false }}
+        run: bash scripts/e2e/clickhouse.sh
       - uses: ./.github/actions/verify-metrics-snapshot
         with:
           snapshot: metrics_snapshot_clickhouse

Makefile

@@ -149,7 +149,7 @@ fmt: $(GOFUMPT)
 	@./scripts/lint/updateLicense.py $(ALL_SRC) $(SCRIPTS_SRC)
 
 .PHONY: lint
-lint: lint-license lint-imports lint-semconv lint-goversion lint-goleak lint-go
+lint: lint-license lint-imports lint-semconv lint-goleak lint-go
 
 .PHONY: lint-license
 lint-license:
@@ -177,9 +177,9 @@ lint-imports:
 lint-semconv:
 	./scripts/lint/check-semconv-version.sh
 
-.PHONY: lint-goversion
-lint-goversion:
-	./scripts/lint/check-go-version.sh
+#.PHONY: lint-goversion
+#lint-goversion:
+#	./scripts/lint/check-go-version.sh
 
 .PHONY: lint-goleak
 lint-goleak:

idl

@@ -7,21 +7,13 @@ import (
 	"context"
 
 	"go.opentelemetry.io/collector/pdata/ptrace"
-	"go.uber.org/zap"
-
-	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client/conn"
-	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client/pool"
 )
 
 type Pool interface {
 	Do(ctx context.Context, query string, td ...ptrace.Traces) error
 	Close() error
 }
 
-type Chpool interface {
-	Dial(config pool.Configuration, log *zap.Logger) (Pool, error)
-}
-
 type Conn interface {
 	// TODO arg should support the dyment parameter.
 	Query(ctx context.Context, query string, arg string) (Rows, error)
@@ -35,7 +27,3 @@ type Rows interface {
 	ScanStruct(dest any) error
 	Err() error
 }
-
-type Clickhouse interface {
-	Open(config conn.Configuration) (Conn, error)
-}

internal/storage/v2/clickhouse/client/client.go

@@ -3,6 +3,8 @@
 
 package conn
 
+// Configuration clickhouse-go client connection configuration for read trace.
+// more detail see:https://clickhouse.com/docs/integrations/go#connection-settings
 type Configuration struct {
 	Address  []string `mapstructure:"address"`
 	Database string   `mapstructure:"database"`

internal/storage/v2/clickhouse/client/conn/config.go

@@ -14,18 +14,21 @@ const (
 	DefaultHealthCheckPeriod = time.Minute
 )
 
+// Configuration chpool configuration for write trace.
 type Configuration struct {
 	ClientConfig ClientConfig `mapstructure:"client"`
 	PoolConfig   Config       `mapstructure:"pool"`
 }
 
+// ClientConfig client configuration be used to connect to server.
 type ClientConfig struct {
 	Address  string `mapstructure:"address"`
 	Database string `mapstructure:"database"`
 	Username string `mapstructure:"username"`
 	Password string `mapstructure:"password"`
 }
 
+// Config connection pool configuration be used to define every connection lifecycle.
 type Config struct {
 	MaxConnLifetime   time.Duration `mapstructure:"max_connection_lifetime"`
 	MaxConnIdleTime   time.Duration `mapstructure:"max_connection_idle_time"`

internal/storage/v2/clickhouse/client/mocks/Chpool.go

@@ -8,6 +8,10 @@ import (
 	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client/pool"
 )
 
+// Configuration all configuration of how to use clickhouse as storage.
+// ConnConfig clickhouse-go client configuration
+// PoolConfig chpool configuration
+// CreateSchema create requre table auto if not exist.
 type Configuration struct {
 	ConnConfig   conn.Configuration
 	PoolConfig   pool.Configuration
@@ -18,6 +22,6 @@ func DefaultConfiguration() Configuration {
 	return Configuration{
 		conn.DefaultConfig(),
 		pool.DefaultConfig(),
-		false,
+		true,
 	}
 }

internal/storage/v2/clickhouse/client/mocks/Clickhouse.go

@@ -0,0 +1,24 @@
+// Copyright (c) 2025 The Jaeger Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client/conn"
+	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client/pool"
+)
+
+func TestDefaultConfig(t *testing.T) {
+	excepted := Configuration{
+		ConnConfig:   conn.DefaultConfig(),
+		PoolConfig:   pool.DefaultConfig(),
+		CreateSchema: true,
+	}
+
+	actual := DefaultConfiguration()
+	assert.Equal(t, excepted, actual)
+}

internal/storage/v2/clickhouse/client/pool/config.go

@@ -13,7 +13,7 @@ import (
 	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client"
 )
 
-//go:embed init.tmpl
+//go:embed schema.tmpl
 var schemaFile embed.FS
 
 func getQueryFileAsBytes(fileName string) ([]byte, error) {
@@ -59,7 +59,7 @@ func getQueriesFromBytes(queryFile []byte) ([]string, error) {
 }
 
 func constructSchemaQueries() ([]string, error) {
-	queryFile, err := getQueryFileAsBytes("init.tmpl")
+	queryFile, err := getQueryFileAsBytes("schema.tmpl")
 	if err != nil {
 		return nil, err
 	}

internal/storage/v2/clickhouse/config/config.go

@@ -15,7 +15,7 @@ import (
 	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/trace/dbmodel"
 )
 
-const querySQL = `
+const getTraces = `
 SELECT
     Timestamp,
     TraceId,
@@ -71,7 +71,7 @@ func (tr Reader) GetTraces(
 ) iter.Seq2[[]ptrace.Traces, error] {
 	return func(yield func([]ptrace.Traces, error) bool) {
 		for _, id := range traceIDs {
-			tds, err := tr.getTraces(ctx, querySQL, id)
+			tds, err := tr.getTraces(ctx, getTraces, id)
 			if err != nil {
 				if errors.Is(err, errors.New("trace not found")) {
 					continue

internal/storage/v2/clickhouse/config/config_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/client"
 )
 
-const INSERT_SQL = `INSERT INTO otel_traces (
+const write_traces = `INSERT INTO otel_traces (
 	Timestamp,
 	TraceId,
 	SpanId,
@@ -61,7 +61,7 @@ func (t *Writer) WriteTraces(ctx context.Context, td ptrace.Traces) error {
 }
 
 func (t *Writer) writeTraces(ctx context.Context, td ptrace.Traces) error {
-	err := t.Client.Do(ctx, INSERT_SQL, td)
+	err := t.Client.Do(ctx, write_traces, td)
 	if err != nil {
 		return err
 	}

internal/storage/v2/clickhouse/schema/schema.go

@@ -18,6 +18,7 @@ import (
 	"github.com/jaegertracing/jaeger/internal/storage/v2/clickhouse/internal"
 )
 
+// WrapDial wrap chpool.Dial return an abstraction connection pool.
 func WrapDial(config pool.Configuration, log *zap.Logger) (client.Pool, error) {
 	option := chpool.Options{
 		ClientOptions: ch.Options{
@@ -70,6 +71,7 @@ func (c PoolWrapper) WrapDo(ctx context.Context, query ch.Query) error {
 	return c.Pool.Do(ctx, query)
 }
 
+// WrapOpen wrap clickhouse.Open return an abstraction connection.
 func WrapOpen(config conn.Configuration) (client.Conn, error) {
 	option := v2Client.Options{
 		Addr: config.Address,
@@ -101,6 +103,7 @@ func (c ConnWrapper) Query(ctx context.Context, query string, args string) (clie
 	return c.WrapQuery(ctx, query, args)
 }
 
+// Exec execute given query
 func (c ConnWrapper) Exec(ctx context.Context, query string) error {
 	return c.WrapExec(ctx, query)
 }

internal/storage/v2/clickhouse/schema/init.tmpl internal/storage/v2/clickhouse/schema/schema.tmpl

@@ -5,9 +5,9 @@
 
 export CLICKHOUSE_USERNAME="default"
 export CLICKHOUSE_PASSWORD="default"
+export QUERY_FILE="/clickhouse-schema/schema.tmpl"
 compose_file="docker-compose/clickhouse/docker-compose.yml"
 SKIP_APPLY_SCHEMA=${SKIP_APPLY_SCHEMA:-"false"}
-export CLICKHOUSE_CREATE_SCHEMA=${SKIP_APPLY_SCHEMA}
 
 setup_clickhouse() {
   docker compose -f "$compose_file" up -d
@@ -31,7 +31,7 @@ apply_schema() {
   local params=(
     --rm
     --env "CLICKHOUSE_HOST=localhost"
-    --env "QUERY_FILE=/clickhouse-schema/init.tmpl"
+    --env "QUERY_FILE=${QUERY_FILE}"
     --env "CLICKHOUSE_USERNAME=${CLICKHOUSE_USERNAME}"
     --env "CLICKHOUSE_PASSWORD=${CLICKHOUSE_PASSWORD}"
     --network host