[clickhouse] Add linter for ClickHouse by mahadzaryab1 · Pull Request #8641 · jaegertracing/jaeger

mahadzaryab1 · 2026-05-26T02:16:36Z

Description of the changes

Adds the clickhouselint linter to catch any mistakes in our ClickHouse API usage.

How was this change tested?

CI

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully: make lint test

AI Usage in this PR (choose one)

See AI Usage Policy.

None: No AI tools were used in creating this PR
Light: AI provided minor assistance (formatting, simple suggestions)
Moderate: AI helped with code generation or debugging specific parts
Heavy: AI generated most or all of the code changes

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

codecov · 2026-05-26T02:25:29Z

Codecov Report

❌ Patch coverage is 88.40580% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.54%. Comparing base (10f8198) to head (f1d06d3).

Files with missing lines	Patch %	Lines
...nternal/storage/v2/clickhouse/tracestore/reader.go	88.05%	4 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8641      +/-   ##
==========================================
- Coverage   96.58%   96.54%   -0.04%     
==========================================
  Files         334      334              
  Lines       17789    17825      +36     
==========================================
+ Hits        17182    17210      +28     
- Misses        456      460       +4     
- Partials      151      155       +4

Flag	Coverage Δ
badger_direct	`8.83% <0.00%> (-0.04%)`	⬇️
badger_e2e	`1.03% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-direct-manual	`14.36% <0.00%> (-0.06%)`	⬇️
cassandra-4.x-e2e-auto	`1.02% <0.00%> (-0.01%)`	⬇️
cassandra-4.x-e2e-manual	`1.02% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-direct-manual	`14.36% <0.00%> (-0.06%)`	⬇️
cassandra-5.x-e2e-auto	`1.02% <0.00%> (-0.01%)`	⬇️
cassandra-5.x-e2e-manual	`1.02% <0.00%> (-0.01%)`	⬇️
clickhouse-direct	`8.90% <20.28%> (+<0.01%)`	⬆️
clickhouse-e2e	`1.17% <0.00%> (-0.01%)`	⬇️
elasticsearch-6.x-direct	`16.66% <0.00%> (-0.07%)`	⬇️
elasticsearch-7.x-direct	`16.69% <0.00%> (-0.07%)`	⬇️
elasticsearch-8.x-direct	`16.84% <0.00%> (-0.07%)`	⬇️
elasticsearch-8.x-e2e	`1.03% <0.00%> (-0.01%)`	⬇️
elasticsearch-9.x-e2e	`1.03% <0.00%> (-0.01%)`	⬇️
grpc_direct	`7.80% <0.00%> (-0.03%)`	⬇️
grpc_e2e	`1.05% <0.00%> (-0.01%)`	⬇️
kafka-3.x-v2	`1.03% <0.00%> (-0.01%)`	⬇️
memory_v2	`1.05% <0.00%> (-0.01%)`	⬇️
opensearch-1.x-direct	`16.73% <0.00%> (-0.07%)`	⬇️
opensearch-2.x-direct	`16.73% <0.00%> (-0.07%)`	⬇️
opensearch-2.x-e2e	`1.03% <0.00%> (-0.01%)`	⬇️
opensearch-3.x-e2e	`1.03% <0.00%> (-0.01%)`	⬇️
query	`1.05% <0.00%> (-0.01%)`	⬇️
tailsampling-processor	`0.54% <0.00%> (-0.01%)`	⬇️
unittests	`94.87% <88.40%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

github-actions · 2026-05-26T03:00:37Z

CI Summary Report

Metrics Comparison

✅ No significant metric changes

Code Coverage

✅ Coverage 97.3% (baseline 97.3%)

➡️ View CI run | View publish logs
2026-05-31 04:06:23 UTC

Copilot

Pull request overview

Adds the clickhouselint golangci-lint linter and updates ClickHouse readers/tests to satisfy the stricter resource/rows handling expectations when iterating query results.

Changes:

Enable clickhouselint in .golangci.yml.
Add rows.Err() checks after row iteration in ClickHouse trace/dependency readers.
Add/adjust unit tests to cover rows.Err() behavior and linter-driven cleanup in the ClickHouse test driver.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/storage/v2/clickhouse/tracestore/reader.go	Adds `rows.Err()` checks in multiple reader methods after iterating rows.
internal/storage/v2/clickhouse/tracestore/reader_test.go	Adds new test cases for `rows.Err()` propagation in trace/service/operation queries.
internal/storage/v2/clickhouse/depstore/reader.go	Adds `rows.Err()` check after iterating dependency snapshot rows.
internal/storage/v2/clickhouse/depstore/reader_test.go	Adds test coverage for dependencies `rows.Err()` propagation.
internal/storage/v2/clickhouse/clickhousetest/driver_test.go	Ensures prepared batches are closed; adds targeted `//nolint:clickhouselint` where results are nil.
.golangci.yml	Enables the `clickhouselint` linter.

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

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Move the per-iteration logic of GetTraces into a getTrace helper method so defer rows.Close() works in its natural scope, replacing the inline IIFE with named-return error joining. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Inline the per-traceID body and use an []error slice with errors.Join to collect scan, rows.Err, and close errors uniformly. Drops the helper method, the named return, and the conditional join introduced earlier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Apply the same []error + errors.Join pattern as GetTraces so close, scan, and rows.Err errors all reach the consumer instead of being dropped by defer rows.Close(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Apply the same []error + errors.Join pattern so scan, rows.Err, and close errors are all reported instead of dropped by defer rows.Close(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Yuri Shkuro <github@ysh.us>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

+	got, err := d.PrepareBatch(context.Background(), "INSERT INTO spans VALUES (?)") //nolint:clickhouselint // got is nil on error
 	require.ErrorIs(t, err, wantErr)
 	assert.Nil(t, got)


+	got, err := d.PrepareBatch(context.Background(), "INSERT INTO other VALUES (?)") //nolint:clickhouselint // got is nil when no match
 	require.NoError(t, err)
 	assert.Nil(t, got)
 }


## Description of the changes - Adds the [clickhouselint](https://golangci-lint.run/docs/linters/configuration/#clickhouselint) linter to catch any mistakes in our ClickHouse API usage. ## How was this change tested? - CI ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully: `make lint test` ## AI Usage in this PR (choose one) See [AI Usage Policy](https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md#ai-usage-policy). - [ ] **None**: No AI tools were used in creating this PR - [x] **Light**: AI provided minor assistance (formatting, simple suggestions) - [ ] **Moderate**: AI helped with code generation or debugging specific parts - [ ] **Heavy**: AI generated most or all of the code changes --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Victor Chidera Obiezue <obiezuechidera@gmail.com>

mahadzaryab1 added 2 commits May 25, 2026 18:59

Enable ClickHouse Linter

0294b78

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Fix Lint Violations

47362c4

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Fix

a372333

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

mahadzaryab1 added the changelog:bugfix-or-minor-feature label May 29, 2026

mahadzaryab1 marked this pull request as ready for review May 30, 2026 15:42

mahadzaryab1 requested a review from a team as a code owner May 30, 2026 15:42

Copilot AI review requested due to automatic review settings May 30, 2026 15:42

dosubot Bot added area/storage go Pull requests that update go code labels May 30, 2026

Copilot started reviewing on behalf of mahadzaryab1 May 30, 2026 15:42 View session

Copilot AI reviewed May 30, 2026

View reviewed changes

Comment thread internal/storage/v2/clickhouse/tracestore/reader.go

Comment thread internal/storage/v2/clickhouse/tracestore/reader.go Outdated

yurishkuro reviewed May 30, 2026

View reviewed changes

Comment thread internal/storage/v2/clickhouse/tracestore/reader.go Outdated

Comment thread internal/storage/v2/clickhouse/tracestore/reader.go Outdated

mahadzaryab1 added 2 commits May 30, 2026 11:36

Address CoPilot Feedback

4624e7b

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Address Feedback

35f3c68

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Copilot AI review requested due to automatic review settings May 30, 2026 18:46

Copilot started reviewing on behalf of mahadzaryab1 May 30, 2026 18:46 View session

Copilot AI reviewed May 30, 2026

View reviewed changes

Comment thread internal/storage/v2/clickhouse/tracestore/reader.go Outdated

Comment thread internal/storage/v2/clickhouse/tracestore/reader.go

mahadzaryab1 and others added 2 commits May 30, 2026 11:57

Address Feedback

31b8480

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>

Merge branch 'main' into add-ch-lint

50c2822

Copilot AI review requested due to automatic review settings May 31, 2026 03:24

Copilot started reviewing on behalf of yurishkuro May 31, 2026 03:24 View session

Copilot AI reviewed May 31, 2026

View reviewed changes

yurishkuro and others added 2 commits May 30, 2026 23:30

Copilot AI review requested due to automatic review settings May 31, 2026 03:48

Copilot started reviewing on behalf of yurishkuro May 31, 2026 03:48 View session

Copilot AI reviewed May 31, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings May 31, 2026 03:54

Copilot started reviewing on behalf of yurishkuro May 31, 2026 03:54 View session

yurishkuro approved these changes May 31, 2026

View reviewed changes

Copilot AI reviewed May 31, 2026

View reviewed changes

yurishkuro merged commit 3f8a5de into jaegertracing:main May 31, 2026
71 of 72 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clickhouse] Add linter for ClickHouse #8641

[clickhouse] Add linter for ClickHouse #8641
yurishkuro merged 11 commits into
jaegertracing:mainfrom
mahadzaryab1:add-ch-lint

mahadzaryab1 commented May 26, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mahadzaryab1 commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the changes

How was this change tested?

Checklist

AI Usage in this PR (choose one)

Uh oh!

codecov Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Summary Report

Metrics Comparison

Code Coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mahadzaryab1 commented May 26, 2026 •

edited

Loading

codecov Bot commented May 26, 2026 •

edited

Loading

github-actions Bot commented May 26, 2026 •

edited

Loading