[CODE HEALTH] Remove unused alias declarations#4091
Conversation
64d7a89 to
9bea645
Compare
There was a problem hiding this comment.
Pull request overview
This PR is a code-health cleanup to reduce clang-tidy misc-unused-alias-decls warnings by removing unused file-scope namespace alias declarations across examples, exporters, SDK source, and tests, and by ratcheting the clang-tidy warning limits accordingly.
Changes:
- Remove unused namespace alias declarations (e.g.,
namespace nostd = opentelemetry::nostd;) across multiple components. - Update the clang-tidy GitHub Actions workflow warning limits to reflect the reduced warning count.
- Add an Unreleased changelog entry for this code-health change.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/test/metrics/sync_metric_storage_up_down_counter_test.cc | Removes unused namespace alias; formatting cleanup in local aliases. |
| sdk/test/metrics/sync_metric_storage_histogram_test.cc | Removes unused namespace alias; formatting cleanup in local aliases. |
| sdk/test/metrics/sync_metric_storage_gauge_test.cc | Removes unused namespace alias. |
| sdk/test/metrics/sync_metric_storage_counter_test.cc | Removes unused namespace alias; formatting cleanup in local aliases. |
| sdk/test/metrics/async_metric_storage_test.cc | Removes unused namespace alias. |
| sdk/src/logs/logger.cc | Removes unused namespace alias inside sdk::logs. |
| exporters/zipkin/src/zipkin_exporter_factory.cc | Removes unused HTTP client namespace alias. |
| exporters/prometheus/test/collector_test.cc | Removes unused metric-related namespace aliases in tests. |
| exporters/prometheus/src/collector.cc | Removes unused metrics SDK namespace alias. |
| exporters/otlp/test/otlp_recordable_test.cc | Removes redundant unused trace namespace alias. |
| exporters/otlp/src/otlp_recordable.cc | Removes unused nostd namespace alias. |
| exporters/ostream/test/ostream_metric_test.cc | Removes unused nostd namespace alias. |
| exporters/elasticsearch/src/es_log_record_exporter.cc | Removes unused nostd namespace alias. |
| examples/zipkin/main.cc | Removes unused trace namespace alias. |
| examples/simple/main.cc | Removes unused trace API namespace alias. |
| examples/prometheus/main.cc | Removes unused common namespace alias. |
| examples/otlp/http_metric_main.cc | Removes unused common namespace alias. |
| examples/otlp/http_main.cc | Removes unused trace namespace alias. |
| examples/otlp/http_log_main.cc | Removes unused trace namespace alias. |
| examples/otlp/grpc_metric_main.cc | Removes unused common namespace alias. |
| examples/otlp/grpc_main.cc | Removes unused trace namespace alias. |
| examples/otlp/grpc_log_main.cc | Removes unused trace namespace alias. |
| examples/otlp/file_metric_main.cc | Removes unused common namespace alias. |
| examples/otlp/file_main.cc | Removes unused trace namespace alias. |
| examples/otlp/file_log_main.cc | Removes unused trace namespace alias. |
| examples/metrics_simple/metrics_ostream.cc | Removes unused common namespace alias. |
| CHANGELOG.md | Adds an Unreleased changelog entry for the cleanup. |
| api/test/trace/tracer_test.cc | Removes unused context namespace alias. |
| .github/workflows/clang-tidy.yaml | Lowers clang-tidy warning limits to match the reduced warnings. |
| using namespace opentelemetry::common; | ||
| using M = std::map<std::string, std::string>; | ||
| namespace nostd = opentelemetry::nostd; | ||
| using M = std::map<std::string, std::string>; |
There was a problem hiding this comment.
Good catch. The 4 using M = std::map<std::string, std::string>; lines in the storage tests (counter, histogram, up_down_counter, async) were indeed unused. clang-tidy did not flag them because misc-unused-alias-decls targets namespace aliases only, not type aliases — but the same dead-code spirit applies and the files were already in this PR.
Removed all 4 in the next force-push. Adjusted the PR title and CHANGELOG entry from "namespace alias declarations" to the broader "alias declarations" to cover both kinds.
(For symmetry: sync_metric_storage_gauge_test.cc does not declare M at all, so nothing to remove there.)
| using namespace opentelemetry::common; | ||
| using M = std::map<std::string, std::string>; | ||
| namespace nostd = opentelemetry::nostd; | ||
| using M = std::map<std::string, std::string>; |
| using namespace opentelemetry::common; | ||
| using M = std::map<std::string, std::string>; | ||
| namespace nostd = opentelemetry::nostd; | ||
| using M = std::map<std::string, std::string>; |
| using M = std::map<std::string, std::string>; | ||
|
|
| * [CODE HEALTH] Remove unused namespace alias declarations | ||
| [#4091](https://github.com/open-telemetry/opentelemetry-cpp/pull/4091) | ||
|
|
There was a problem hiding this comment.
The CHANGELOG entry already uses the actual PR number #4091 (visible in the diff_hunk this review comment quotes). There is no #XXXX placeholder in the current file; the body of this review comment appears to misread its own quoted diff. Same false-positive pattern reported on PR #3989, #3997, #4013, and #4090.
9bea645 to
50167f0
Compare
50167f0 to
35d96e3
Compare
marcalff
left a comment
There was a problem hiding this comment.
LGTM, thanks for the cleanup.
|
Now that some aliases are removed, extra includes can be removed as well, see IWYU failures: |
Eliminate 29 misc-unused-alias-decls warnings flagged by clang-tidy v22 across 27 files in examples, exporters, sdk, and tests. The dominant pattern was a file-scope `namespace nostd = opentelemetry::nostd;` declaration never referenced via the file-scope alias path (uses of `nostd::` resolved through OPENTELEMETRY_BEGIN_NAMESPACE namespace nesting instead). Also remove 4 unused type-alias `using M = std::map<std::string, std::string>;` declarations in storage tests, observed by Copilot review. clang-tidy does not flag these (misc-unused-alias-decls targets only namespace aliases, not type aliases), but they are genuinely dead code in the same spirit and the same files this PR already touches. Also remove 9 now-unused #include directives that IWYU flagged as a consequence of the alias removals: when an alias like `namespace http_client = opentelemetry::ext::http::client;` is removed, the include that brought the namespace into scope is no longer needed. Same logic for several `opentelemetry/common/attribute_value.h` includes that supported `namespace common = opentelemetry::common;` aliases, and `<map>` after removing the unused `using M = ...` from async_metric_storage_test.cc. No behavior change. Removed declarations were genuinely unused. clang-format auto-collapsed adjacent orphan alignment whitespace where removing a line left aligned siblings. Ratchet: * abiv1-preview warning_limit lowered from 418 to 389 * abiv2-preview warning_limit lowered from 424 to 395 A 30th misc-unused-alias-decls site at exporters/otlp/src/otlp_populate_attribute_utils.cc:30 is intentionally not touched in this PR to avoid conflicting with the in-flight open-telemetry#4090. It can be cleaned up in a trivial follow-up after open-telemetry#4090 lands. Part of open-telemetry#2053 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
35d96e3 to
2a00ad2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4091 +/- ##
=======================================
Coverage 82.01% 82.01%
=======================================
Files 385 385
Lines 16023 16023
=======================================
Hits 13139 13139
Misses 2884 2884
🚀 New features to boost your workflow:
|
Summary
Eliminates 29
misc-unused-alias-declswarnings flagged by clang-tidy v22, spread across 27 files in examples, exporters, sdk source, and tests. The dominant pattern was a file-scope declaration like:that was never referenced via the file-scope alias path. Uses of
nostd::inside the surroundingOPENTELEMETRY_BEGIN_NAMESPACEblock resolved through namespace nesting (findingopentelemetry::nostdvia parent-namespace lookup) before ever reaching the file-scope alias. Other variants flagged:common,context,metric_sdk,metric_api,metric_exporter,http_client,trace,trace_api,trace_sdk_2.No behavior change. Every removed declaration was genuinely unreferenced per clang-tidy. Verified against three representative samples (production code with namespace nesting, test code with
using namespace, examples with zero references).Where removing a declaration left adjacent aligned lines (e.g.
using M = std::map<...>;next to a removednamespace nostd = ...;), clang-format auto-collapsed the now-unnecessary alignment whitespace.Excluded from this PR
The 30th
misc-unused-alias-declssite atexporters/otlp/src/otlp_populate_attribute_utils.cc:30is intentionally not touched, to avoid a file-level conflict with the in-flight #4090. It can be cleaned up in a trivial follow-up after #4090 lands.Ratchet
all-options-abiv1-previewwarning_limitlowered from 418 to 389all-options-abiv2-previewwarning_limitlowered from 424 to 395(If #4090 lands first, this PR will need a trivial ratchet rebase from 414→385 and 420→391 — the relative drop of 29 is unchanged.)
Verification
Counts measured against artifacts of the most recent successful CI run on
main(workflowclang-tidy.yaml, run26018587378):Sample verification of the "unused" claim was performed on three representative files (
exporters/elasticsearch/src/es_log_record_exporter.cc,sdk/test/metrics/sync_metric_storage_counter_test.cc,examples/simple/main.cc) — all three confirmed the alias is genuinely dead.Local format verification: ran
clang-format --dry-run --Werrorvia the project's clang-format-compatible toolchain against all 27 modified source files; clean.Test plan
clang-tidyjob passes both abiv1-preview and abiv2-preview at the new limitsFormatjob passesPart of #2053