Remainder of the OTel Exporter changes

[JoshDreamland]

Contains all remaining OTel Exporter changes, including GUC plumbing (OTelExport branch) and tests.

[Copilot]

Pull request overview

Adds OpenTelemetry (OTLP gRPC) export support alongside the existing ClickHouse exporter, including new GUCs, build plumbing, Docker-based collector setup, and TAP integration tests.

Changes:

• Add OTel exporter implementation and runtime selection via pg_stat_ch.use_otel
• Add OTel collector Docker/Compose config + new TAP tests for export and reconnect behavior
• Refactor ClickHouse exporter into its own compilation unit; update build system and docs

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
third_party/opentelemetry-cpp	Adds OTel C++ SDK as a submodule dependency.
test/regression/expected/guc.out	Updates regression expectations for new OTel/hostname GUCs.
t/psch.pm	Adds TAP helper functions for OTel collector + Prometheus assertions.
t/024_otel_export.pl	New TAP test for basic OTel export, labels, and stats invariants.
t/025_otel_reconnect.pl	New TAP test for collector restart/reconnect behavior.
src/export/stats_exporter.cc	Selects exporter at init time and factors ClickHouse exporter out.
src/export/otel_exporter.h	Declares the OTel exporter factory.
src/export/otel_exporter.cc	Implements OTel logs+metrics export via OTLP gRPC.
src/export/clickhouse_exporter.h	Declares the ClickHouse exporter factory.
src/export/clickhouse_exporter.cc	Moves ClickHouse exporter implementation out of stats_exporter.cc.
src/config/guc.h	Adds new GUC declarations for OTel.
src/config/guc.cc	Adds new GUC definitions and registration (use_otel, endpoint, hostname).
scripts/run-tests.sh	Adds otel test target and PERL5LIB bootstrapping.
mise.toml	Adds mise tasks to run OTel tests and manage collector container.
include/config/guc.h	Exposes new OTel GUCs to external includes.
docs/testing.md	Updates ClickHouse test docs; references Docker/TAP setup doc.
docs/docker-setup.md	Adds Docker + TAP-enabled Postgres setup documentation.
docker/otel/collector-config.yaml	Adds collector config for OTLP receiver + Prometheus exporter.
docker/docker-compose.otel.yml	Adds compose file for local OTel collector used by tests.
cmake/Findabsl.cmake	Adds CMake shim for find_package(absl) with vendored abseil.
README.md	Updates testing docs pointers and Docker prerequisites wording.
CMakeLists.txt	Builds OTel SDK first (grpc/absl), then clickhouse-cpp; links OTel libs.
.gitmodules	Registers OTel C++ SDK submodule.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

LogNegativeValue is called here but there’s no visible declaration/definition in this translation unit (and the diff shows a new LogNegativeValue in stats_exporter.cc without a header declaration). This will fail to compile unless it’s declared via an included header. Consider moving LogNegativeValue into otel_exporter.cc (e.g., in the anonymous namespace) or adding a proper declaration in a shared header and ensuring it links from exactly one TU.

[Copilot]

This uses system with a single string, which invokes a shell and interpolates $compose_file. If PROJECT_DIR (or the path) contains spaces or shell metacharacters, this can break or be exploited. Prefer the multi-arg system('docker','compose','-f',$compose_file,'up','-d') form (and similarly for down -v).

[Copilot]

The Docker availability check relies on !system(...) == 0, which is correct but hard to read due to operator precedence. Please rewrite as system(...) != 0 (or capture the exit code explicitly) to avoid confusion and make intent clear. Same pattern appears in t/025_otel_reconnect.pl.

Suggested change

	if (!system("docker ps >/dev/null 2>&1") == 0) {
	if (system("docker ps >/dev/null 2>&1") != 0) {

[Copilot]

For string-like attributes, OpenTelemetry C++ commonly stores attribute values as non-owning string views. NoStringViews(std::string_view) returns a temporary std::string, which may be converted/stored as an attribute string view and then dangle after the call. Consider storing a stable std::string member (similar to CounterColumn::stash_val) and passing that stable storage to SetAttribute, or use an owning attribute type (if available in your OTel SDK version).

Suggested change

	string NoStringViews(std::string_view x) { return string{x}; }
	private:
	OTelExporter* exp;
	std::string name;
	const std::string& NoStringViews(std::string_view x) {
	stash_val.assign(x);
	return stash_val;
	}
	private:
	OTelExporter* exp;
	std::string name;
	std::string stash_val;

[Copilot]

On initialization failure, the exception is swallowed and the function returns false with no server-visible error. This makes diagnosing endpoint/config issues difficult. Please emit an elog(WARNING, ...) (or your project’s logging helper) with e.what() and consider recording an export failure so send_failures/stats reflect the error.

[Copilot]

The docs list clickhouse but not the newly added otel test type (added in scripts/run-tests.sh and mise.toml). Please add an otel row describing prerequisites (Docker + TAP-enabled PG) and how to run it.

[Copilot]

This doc now effectively covers Docker + TAP-enabled PostgreSQL setup that also applies to OTel integration tests. Consider updating the title (and/or introductory paragraph) to reflect that it’s a shared prerequisite doc for both ClickHouse and OTel tests, so readers don’t assume it’s ClickHouse-only.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This Docker availability check relies on !system(...) == 0, which is easy to misread and depends on operator precedence. Using system(...) != 0 (or reusing psch_otelcol_available() for a single skip path) would be clearer and less error-prone.

Suggested change

	if (!system("docker ps >/dev/null 2>&1") == 0) {
	if (system("docker ps >/dev/null 2>&1") != 0) {

[Copilot]

The comment says instrument names like duration_us become pg_stat_ch_duration_us_count, but the tests/assertions in this PR query metrics like pg_stat_ch_duration_us_unit_*. Please align this comment with the actual exported Prometheus metric names so future test/debug work isn’t misled.

Suggested change

	# Prefix all metric names with "pg_stat_ch_" so that instrument names
	# like "duration_us" become "pg_stat_ch_duration_us_count" etc.
	# Prefix all metric names with "pg_stat_ch_"; instrument names like
	# "duration_us" are exported as "pg_stat_ch_duration_us_unit_*" etc.

[Copilot]

Same as above: system("docker compose -f $compose_file down -v") uses the shell and can mis-handle paths with spaces/metacharacters. Use the multi-arg system form so the compose file path is passed safely as a single argument.

[Copilot]

The Prometheus text exposition format can emit numbers in scientific notation (e.g., 1e+06) as well as special values (NaN, +Inf). The current regex only matches plain decimals, so this helper can silently return 0/undercount if the formatter uses exponent notation. Consider parsing with a float regex that supports exponents (or using a numeric parser) so the helper is robust.

Suggested change

	if ($line =~ /^\Q${metric_base}\E_count(?:\{[^}]*\})?\s+(\d+(?:\.\d+)?)/) {
	$total += $1;
	if ($line =~ /^\Q${metric_base}\E_count(?:\{[^}]*\})?\s+([+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?|NaN|[+-]?Inf)/) {
	my $value = $1;
	next if $value eq 'NaN' || $value eq '+Inf' || $value eq '-Inf';
	$total += $value;

[Copilot]

The regex used to parse Prometheus metric values only matches simple decimals, but Prometheus exposition may use scientific notation. If _sum is formatted as 1.23e+04, this will be missed and the test may fail/flap. Consider updating the value parsing to accept exponent notation (and optionally NaN/+Inf).

Suggested change

	if ($line =~ /^pg_stat_ch_duration_us_unit_sum(?:\{[^}]*\})?\s+(\d+(?:\.\d+)?)/) {
	$sum += $1;
	if ($line =~ /^pg_stat_ch_duration_us_unit_sum(?:\{[^}]*\})?\s+([+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?|NaN|[+-]Inf)/) {
	my $value = $1;
	next if $value eq 'NaN' || $value eq '+Inf' || $value eq '-Inf';
	$sum += $value;