[exporter/clickhouseexporter] Clickhouse replication

[alex1704]

Description:
Allow replication for fault tolerance when create databases and tables.

Testing:
I have added docker compose file in example/replication_setup which will setup 2 clickhouse servers and on clickhouse-keeper. Tests for data replication were added to:

• exporter_logs_test.go (TestLogsTableCreationOnCluster)
• exporter_metrics_test.go (TestMetricsTablesCreationOnCluster)
• exporter_traces_test.go (TestTracesTablesCreationOnCluster)

Testable yml config was added to file testdata/config-table-engine.yml

Additionally test was added for checking how TableEngine is parsed in config_test.go (TestTableEngineConfigParsing)

Documentation:
Two new fields were added to Config:

TableEngine TableEngine `mapstructure:"table_engine"`
ClusterName string `mapstructure:"cluster_name"`

TableEngine definition

type TableEngine struct {
  Name string `mapstructure:"name"`
  Params string `mapstructure:"params"`
}

When ClusterName is present then when database and table are created "ON CLUSTER ClusterName" is present in 'CREATE definition'. TableEngine is used to change table 'ENGINE' field in 'CREATE definition'. In order replication to work TableEngine.Name need to be set to ReplicatedMergeTree, TableEngine.Params can be omitted.
Here is an example of how clickhouse exporter config look like in order to support fault tolerance replication:

exporters:
  clickhouse:
    endpoint: tcp://127.0.0.1:9000
    cluster_name: cluster_1
    table_engine:
      name: ReplicatedMergeTree

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: alex1704 / name: Oleksandr Kostenko (92c57b8, 66b34ee, 6eb0726, a53a09c, eec11be, f3f6aef, d0d2cf0, cd5d854, 2a64806, a04fbbb, c91eb40, 5198f1d, d08f668, 6d0960e, 7d2ff65, 530edf6, acf8f33, 7b27807)
• ✅ login: Frapschen / name: Murphy Chen (7b27807)
• ✅ login: atoulme / name: Antoine Toulme (4a5c6f5)

[Frapschen]

Will it depend on a local clickhouse server? Using a mock server like other UT tests.
The real replicated table count is guaranteed by clickhouse server, We just verify if the query statement is correct.

[alex1704]

Will it depend on a local clickhouse server? - yes, it need to be start by running docker-compose.yml in example/replication_setup.

[Frapschen]

I don't think this will pass in the current UT test pipeline, but this docker-compose.yml is very helpful for local dev.

[alex1704]

What would you propose to best handle tests? I just can't other way how to test it without creating clickhouse, zookeeper instances.

[Frapschen]

As I said above, you can verify the query statement whether is your expectation when ClusterName or TableEngine is configured. you can follow this:

opentelemetry-collector-contrib/exporter/clickhouseexporter/exporter_logs_test.go

Lines 80 to 93 in 403626b

	initClickhouseTestServer(t, func(query string, values []driver.Value) error {
	t.Logf("%d, values:%+v", items, values)
	if strings.HasPrefix(query, "INSERT") {
	items++
	}
	return nil
	})
	exporter := newTestLogsExporter(t, defaultEndpoint)
	mustPushLogsData(t, exporter, simpleLogs(1))
	mustPushLogsData(t, exporter, simpleLogs(2))
	require.Equal(t, 3, items)
	})

[hanjm]

The local test with real clickhouse server is useful but i do not known how to use it in the ci pipeline, may be we can add a build tag for only test in local? https://mickey.dev/posts/go-build-tags-testing/

hi @dmitryax, could you have some ideal about this?

[Frapschen]

Suggested change

	Params string `mapstructure:"params"`
	Zoo_path_prefix`mapstructure:"zoo_path_prefix"`
	Replica_name: `mapstructure:"replica_name"`
	Other_parameters: `mapstructure:"other_parameters"`

zoo_path_prefix is the prefix of /clickhouse/tables/{shard}/table_name, for trace table, it will be /clickhouse/tables/{shard}/otel_traces.

[alex1704]

The idea behind TableEngine field is to be able to construct any engine with variable list of params like - AnyEngine(p1, p2, p3, p4, p5), it is not necessary have to be related to replication. Am I missing something?

[Frapschen]

AnyEngine(p1, p2, p3, p4, p5) is a general idea, but we need carefully handle the zoo_path param in Replicated*MergeTree, we expect the table creation query string like:

For traces:

      CREATE TABLE IF NOT EXISTS otel.otel_traces on cluster 'otel' (
      ...
      ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{layer}-{shard}/otel_traces', '{replica}');

For logs:

      CREATE TABLE IF NOT EXISTS otel.otel_logs on cluster 'otel' (
      ...
      ) ENGINE ReplicatedMergeTree('/clickhouse/tables/{layer}-{shard}/otel_logs', '{replica}');

instead of every ReplicatedMergeTree uses the same zoo_path.

[alex1704]

If we set Config.TableEngine to TableEngine{Name: ReplicatedMergeTree} then generated query will have cluase ENGINE = ReplicatedMergeTree(). Clickhouse transforms it to ENGINE = ReplicatedMergeTree('/clickhouse/tables/{uuid}/{shard}', '{replica}'). We can check it with show create table otel.otel_logs. Next we can check that placehoders actually generate unique zookeper_path for each table by executing:

SELECT
    fqdn(),replica_name,
    zookeeper_path,
    replica_path
FROM clusterAllReplicas('my_cluster', system.replicas)
WHERE (table = 'otel_logs') AND (database = 'otel')

Here is output example for otel_traces and otel_logs respectively:

┌─FQDN()────┬─replica_name─┬─zookeeper_path─────────────────────────────────────────────┬─replica_path───────────────────────────────────────────────────────────┐
│ localhost │ 02           │ /clickhouse/tables/052912e1-9697-4d2f-8241-4927e15062d0/01 │ /clickhouse/tables/052912e1-9697-4d2f-8241-4927e15062d0/01/replicas/02 │
└───────────┴──────────────┴────────────────────────────────────────────────────────────┴────────────────────────────────────────────────────────────────────────┘

┌─FQDN()────┬─replica_name─┬─zookeeper_path─────────────────────────────────────────────┬─replica_path───────────────────────────────────────────────────────────┐
│ localhost │ 02           │ /clickhouse/tables/8e490fbe-9857-4d25-90af-c2fffbed22b8/01 │ /clickhouse/tables/8e490fbe-9857-4d25-90af-c2fffbed22b8/01/replicas/02 │
└───────────┴──────────────┴────────────────────────────────────────────────────────────┴────────────────────────────────────────────────────────────────────────┘

As we can see different path is generated.
On the other hand, if user wish to provide ReplicatedMergeTree params then it is user's responsibility to do it correctly.

[Frapschen]

thank you for your clarify

[Frapschen]

@alex1704 some comments need resolve.

[Frapschen]

relate issue: #24649

[hanjm]

Thanks, general LGTM. just the comment format.

[hanjm]

Comments should begin with the name of the thing being described and end in a period. https://github.com/golang/go/wiki/CodeReviewComments#comment-sentences

[Frapschen]

@alex1704 Please add a changelog and give some explanation on your change in README.md.

[alex1704]

@Frapschen done.

[Frapschen]

please use t.Run function

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[hanjm]

still valid. @dmitryax could you help to approve the ci workflow, thank you.

[zhangjiabin1010]

Wait for this function. @dmitryax could you help to approve the ci workflow, thank you.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[ondrej-smola]

@Frapschen , @dmitryax is there anything that can be (or can do :)) done to get this merged - really need it for HA setups.

[Frapschen]

@Frapschen , @dmitryax is there anything that can be (or can do :)) done to get this merged - really need it for HA setups.

@alex1704 please fix lint https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/6606464073/job/17942708476?pr=26599#step:8:224

[alex1704]

@Frapschen I currently have a lot on my plate, I welcome anybody willing to fix linter issue, otherwise ticket will be staled for long time.

[Frapschen]

@Frapschen I currently have a lot on my plate, I welcome anybody willing to fix linter issue, otherwise ticket will be staled for long time.

I have resolved the linter issues and conflicts in my code. However, I am unable to merge my local changes into your branch. Please let me know if there is anything else I can do to help.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.