Skip to content

[CODE HEALTH] Move SDK trace and metrics test helpers into anonymous namespaces#4303

Merged
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:codehealth/sdk-trace-metrics-internal-linkage-4196
Jul 25, 2026
Merged

[CODE HEALTH] Move SDK trace and metrics test helpers into anonymous namespaces#4303
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:codehealth/sdk-trace-metrics-internal-linkage-4196

Conversation

@thc1006

@thc1006 thc1006 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Part of #4196

This is the last of three batches after #4301 and #4302, and clears the SDK test sites in #4196.

file classes
sdk/test/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir_test.cc AlignedHistogramBucketExemplarReservoirTestPeer
sdk/test/metrics/measurements_benchmark.cc MockMetricExporter
sdk/test/trace/batch_span_processor_test.cc MockSpanExporter, BatchSpanProcessorTestPeer, MockLogHandler
sdk/test/trace/tracer_provider_set_test.cc TestProvider
sdk/test/trace/tracer_test.cc MockSampler, MockDecisionSampler, MockIdGenerator

Shape of the change

tracer_test.cc and measurements_benchmark.cc already had an anonymous namespace that stopped just short of the flagged classes, so those are extended rather than joined by a second one.

batch_span_processor_test.cc is the one worth a second look. It already sits inside a named namespace opened by OPENTELEMETRY_BEGIN_NAMESPACE, so the anonymous namespace nests inside that and closes just before OPENTELEMETRY_END_NAMESPACE. It covers every class and every TEST in the file, including the BatchSpanProcessorOptionsEnvTest group at the end.

tracer_provider_set_test.cc closes its namespace after the trailing #endif of the NO_GETENV block, so the placement holds whether or not that block is compiled.

Warning limit

151 to 142 and 161 to 152, nine on each preset since none are ABI gated.

Cut from main rather than stacked on #4301 and #4302. If any of the three merge before this, the limit needs recomputing down by the merged batch's count. Happy to rebase whenever; they were kept independent so they can be reviewed in any order.

Verification

  • All nine classes end up inside an anonymous namespace, checked by finding the nearest namespace open and close around each rather than by eye.
  • Every file compiles under -DOPENTELEMETRY_ABI_VERSION_NO=1 and =2.
  • BatchSpanProcessorTestPeer and AlignedHistogramBucketExemplarReservoirTestPeer are the only TEST_F suite names in the batch, and each appears in exactly one file, so no all_tests redefinition. This was the specific risk from [CODE HEALTH] Move api/test and sdk/test classes into anonymous namespace #4217 and the main thing I wanted to rule out.
  • measurements_benchmark.cc is a benchmark target, so it needs to link rather than only compile; the change is a namespace wrap and does not touch linkage of any symbol the benchmark harness needs.
  • clang-format 18 clean.

Same honest gap as the earlier two batches: misc-use-internal-linkage only reports classes from llvm-22 onward, so a local clang-tidy run returns zero regardless and proves nothing. The site list is from the CI log artifact for main, and CI is the authority on the count.

thc1006 added a commit to thc1006/opentelemetry-cpp that referenced this pull request Jul 24, 2026
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.24%. Comparing base (cf62199) to head (4a57b6e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4303      +/-   ##
==========================================
- Coverage   81.25%   81.24%   -0.01%     
==========================================
  Files         446      446              
  Lines       18872    18872              
==========================================
- Hits        15332    15330       -2     
- Misses       3540     3542       +2     

see 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.

thc1006 added 2 commits July 25, 2026 14:34
…namespaces

Nine misc-use-internal-linkage sites, the last of three batches after
open-telemetry#4301 and open-telemetry#4302.

tracer_test.cc had an anonymous namespace holding only initTracer(),
with the three mock classes above it, so that namespace is extended over
them rather than joined by a second one. measurements_benchmark.cc is
the same shape, with MockMetricExporter pulled into the existing
namespace. The other three get a namespace around the test body.

batch_span_processor_test.cc already sits inside a named namespace via
OPENTELEMETRY_BEGIN_NAMESPACE; the anonymous namespace nests inside it
and covers every class and TEST in the file. Its
BatchSpanProcessorTestPeer and the metrics file's
AlignedHistogramBucketExemplarReservoirTestPeer are the only TEST_F
suite names here, and each is unique to its file, so this does not
repeat the all_tests redefinition from open-telemetry#4217.

The warning limit drops by nine on both presets, 151 to 142 and 161 to
152. None of these are ABI gated.

This branch is cut from main. If open-telemetry#4301 or open-telemetry#4302 merge first, the limit
needs recomputing down by their counts as well.

Part of open-telemetry#4196

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006
thc1006 force-pushed the codehealth/sdk-trace-metrics-internal-linkage-4196 branch from 673ee39 to 4a57b6e Compare July 25, 2026 06:34
@marcalff
marcalff marked this pull request as ready for review July 25, 2026 07:22
@marcalff
marcalff requested a review from a team as a code owner July 25, 2026 07:22

@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

@marcalff
marcalff merged commit 991ffad into open-telemetry:main Jul 25, 2026
64 of 72 checks passed
@thc1006
thc1006 deleted the codehealth/sdk-trace-metrics-internal-linkage-4196 branch July 25, 2026 07:25
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.

2 participants