[CODE HEALTH] Move SDK trace and metrics test helpers into anonymous namespaces#4303
Merged
marcalff merged 2 commits intoJul 25, 2026
Conversation
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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 🚀 New features to boost your workflow:
|
…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
force-pushed
the
codehealth/sdk-trace-metrics-internal-linkage-4196
branch
from
July 25, 2026 06:34
673ee39 to
4a57b6e
Compare
marcalff
marked this pull request as ready for review
July 25, 2026 07:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #4196
This is the last of three batches after #4301 and #4302, and clears the SDK test sites in #4196.
sdk/test/metrics/exemplar/aligned_histogram_bucket_exemplar_reservoir_test.ccAlignedHistogramBucketExemplarReservoirTestPeersdk/test/metrics/measurements_benchmark.ccMockMetricExportersdk/test/trace/batch_span_processor_test.ccMockSpanExporter,BatchSpanProcessorTestPeer,MockLogHandlersdk/test/trace/tracer_provider_set_test.ccTestProvidersdk/test/trace/tracer_test.ccMockSampler,MockDecisionSampler,MockIdGeneratorShape of the change
tracer_test.ccandmeasurements_benchmark.ccalready 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.ccis the one worth a second look. It already sits inside a named namespace opened byOPENTELEMETRY_BEGIN_NAMESPACE, so the anonymous namespace nests inside that and closes just beforeOPENTELEMETRY_END_NAMESPACE. It covers every class and everyTESTin the file, including theBatchSpanProcessorOptionsEnvTestgroup at the end.tracer_provider_set_test.cccloses its namespace after the trailing#endifof theNO_GETENVblock, 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
-DOPENTELEMETRY_ABI_VERSION_NO=1and=2.BatchSpanProcessorTestPeerandAlignedHistogramBucketExemplarReservoirTestPeerare the onlyTEST_Fsuite names in the batch, and each appears in exactly one file, so noall_testsredefinition. 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.ccis 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.Same honest gap as the earlier two batches:
misc-use-internal-linkageonly 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.