Skip to content

[CODE HEALTH] Fix clang-tidy bugprone-unused-local-non-trivial-variable warnings#4202

Merged
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:feat/bugprone-unused-local-4198
Jul 3, 2026
Merged

[CODE HEALTH] Fix clang-tidy bugprone-unused-local-non-trivial-variable warnings#4202
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:feat/bugprone-unused-local-4198

Conversation

@thc1006

@thc1006 thc1006 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Resolves the bugprone-unused-local-non-trivial-variable warnings tracked in #4198 (part of #2053). 12 warnings across 6 files.

  • exporters/prometheus/src/exporter_utils.cc: removed two dead local copies (origin_name, unit); the loop already uses instrument_descriptor.name_ / .unit_ directly.
  • api/test/baggage/baggage_benchmark.cc: the two ToHeader() results are the benchmarked work but were unused; wrapped them in benchmark::DoNotOptimize(...) so the calls are not optimized away and the locals are no longer flagged.
  • examples/otlp/{file_metric,http_instrumented,http_metric}_main.cc: removed unused version / schema locals.
  • exporters/otlp/test/otlp_file_client_test.cc: removed two unused output_stream locals in the file-rotation tests (the ostream test keeps its own).

Ratchet lowered 328 -> 316 (abiv1-preview) and 338 -> 326 (abiv2-preview).

Verified locally: IWYU (all-options-abiv1-preview) clean, clang-format fixed point; the 12 warnings are the complete set for this check per the main clang-tidy CI report.

Fixes #4198

Copilot AI review requested due to automatic review settings July 1, 2026 02:40
@thc1006 thc1006 requested a review from a team as a code owner July 1, 2026 02:40
@thc1006 thc1006 force-pushed the feat/bugprone-unused-local-4198 branch from 7f1d70d to c15e858 Compare July 1, 2026 02:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses clang-tidy bugprone-unused-local-non-trivial-variable warnings across the SDK/tests/examples, reducing the clang-tidy warning ratchet thresholds accordingly as part of ongoing clang-tidy cleanup.

Changes:

  • Remove dead/unused local variables in Prometheus exporter utils, OTLP examples, and OTLP file client tests.
  • Ensure Baggage::ToHeader() benchmark work isn’t optimized away by wrapping the call in benchmark::DoNotOptimize(...).
  • Update the clang-tidy GitHub Actions warning limits (ratchet) and add a changelog entry.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
exporters/prometheus/src/exporter_utils.cc Removes unused local copies of instrument name/unit.
api/test/baggage/baggage_benchmark.cc Prevents benchmarked ToHeader() work from being optimized away.
examples/otlp/file_metric_main.cc Removes unused version/schema locals.
examples/otlp/http_metric_main.cc Removes unused version/schema locals.
examples/otlp/http_instrumented_main.cc Removes unused version/schema locals.
exporters/otlp/test/otlp_file_client_test.cc Removes unused stringstream locals in rotation tests.
CHANGELOG.md Adds an entry for this code health change (currently contains a placeholder link).
.github/workflows/clang-tidy.yaml Lowers warning limits to reflect the removed warnings.

Comment thread CHANGELOG.md Outdated
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.88%. Comparing base (7ffb5ec) to head (c9bec9d).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4202      +/-   ##
==========================================
- Coverage   82.90%   82.88%   -0.01%     
==========================================
  Files         405      405              
  Lines       17289    17287       -2     
==========================================
- Hits        14331    14327       -4     
- Misses       2958     2960       +2     
Files with missing lines Coverage Δ
exporters/prometheus/src/exporter_utils.cc 89.11% <ø> (-0.06%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…le warnings

Resolve the 12 bugprone-unused-local-non-trivial-variable warnings from

- prometheus exporter_utils.cc: drop two dead local copies (origin_name,
  unit); the descriptor fields are already used directly below.
- baggage_benchmark.cc: wrap the two measured ToHeader() results in
  benchmark::DoNotOptimize so the calls are not optimized away and the
  locals are no longer flagged as unused.
- otlp examples (file_metric / http_instrumented / http_metric main): drop
  unused version / schema locals.
- otlp_file_client_test.cc: drop two unused output_stream locals in the
  file-rotation tests.

Lower the clang-tidy warning limit 328 -> 316 (abiv1-preview) and
338 -> 326 (abiv2-preview).

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006 thc1006 force-pushed the feat/bugprone-unused-local-4198 branch from c15e858 to 6fe1cf7 Compare July 2, 2026 07:33
@thc1006 thc1006 requested a review from marcalff July 2, 2026 07:47
@thc1006

thc1006 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Dear reviewer
I have fix the Merge conflict.
now is clean to merge

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the cleanup.

@marcalff marcalff merged commit 8bc3640 into open-telemetry:main Jul 3, 2026
72 checks passed
@thc1006 thc1006 deleted the feat/bugprone-unused-local-4198 branch July 3, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CODE HEALTH] clang-tidy bugprone-unused-local-non-trivial-variable warnings

4 participants