[CODE HEALTH] Move sdk_builder.cc builders into anonymous namespace#4122
Open
thc1006 wants to merge 1 commit into
Open
[CODE HEALTH] Move sdk_builder.cc builders into anonymous namespace#4122thc1006 wants to merge 1 commit into
thc1006 wants to merge 1 commit into
Conversation
Wraps the 10 file-local builder classes in sdk/src/configuration/sdk_builder.cc within an anonymous namespace inside the existing opentelemetry::sdk::configuration namespace. This gives them internal linkage and clears the 10 misc-use-internal-linkage warnings clang-tidy reports on this file. The wrap covers ResourceAttributeValueSetter, SamplerBuilder, SpanProcessorBuilder, SpanExporterBuilder, MetricReaderBuilder, PushMetricExporterBuilder, PullMetricExporterBuilder, AggregationConfigBuilder, LogRecordProcessorBuilder, and LogRecordExporterBuilder. All 10 use inline methods and are only instantiated from SdkBuilder member functions. None are declared in any header or referenced from any other translation unit. SdkBuilder member function definitions remain outside the anonymous namespace since they must keep external linkage as members of a class declared in the public header. The anonymous namespace contents are reachable from SdkBuilder methods via the implicit using-directive that hoists anonymous namespace symbols into the enclosing configuration namespace. This mirrors the structure established by open-telemetry#4121 (registry.cc propagator builders). Ratchet: * abiv1-preview warning_limit lowered from 366 to 356 * abiv2-preview warning_limit lowered from 372 to 362 Part of open-telemetry#2053 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
2c3f1d6 to
19c7879
Compare
marcalff
approved these changes
Jun 4, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4122 +/- ##
=======================================
Coverage 82.00% 82.00%
=======================================
Files 385 385
Lines 16031 16031
=======================================
Hits 13144 13144
Misses 2887 2887 🚀 New features to boost your workflow:
|
ThomsonTan
approved these changes
Jun 4, 2026
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.
Summary
Wraps the 10 file-local builder classes in
sdk/src/configuration/sdk_builder.ccwithin an anonymous namespace,clearing the 10
misc-use-internal-linkagewarnings clang-tidy reportson this file. Second production-code PR of the misc-use-internal-linkage
campaign, after #4121 (registry.cc).
Changes
sdk/src/configuration/sdk_builder.cc: insertnamespace {after theusing common::WildcardMatch;declaration at line 189 and} // namespacebefore the firstSdkBuilder::member functiondefinition at line 750. Wrap covers
ResourceAttributeValueSetter,SamplerBuilder,SpanProcessorBuilder,SpanExporterBuilder,MetricReaderBuilder,PushMetricExporterBuilder,PullMetricExporterBuilder,AggregationConfigBuilder,LogRecordProcessorBuilder, andLogRecordExporterBuilder. The 10classes use inline methods only and are not referenced from any other
translation unit (verified by repository-wide grep).
.github/workflows/clang-tidy.yaml: abiv1 366 -> 356 (-10),abiv2 372 -> 362 (-10).
CHANGELOG.md: one bullet under [Unreleased].Linkage rationale
SdkBuilder::member function definitions remain at namespace scopeoutside the anonymous namespace, because as members of
class SdkBuilderthey must keep external linkage to satisfy ODR. The anonymous namespace
contents are reachable from the member function bodies via the implicit
using-directive that hoists anonymous namespace symbols into the
enclosing
configurationnamespace. This is the same structure used in#4121.
Test plan
opentelemetry_configurationand 7 yaml tests(
-DWITH_CONFIGURATION=ON, 119/119 ninja steps)yaml_propagator_testpasses 9/9yaml_testpasses 50/50yaml_trace_testpasses 38/38 (exercises Sampler / SpanProcessor /SpanExporter builders)
yaml_metrics_testpasses 32/32 (exercises MetricReader /PushMetricExporter / PullMetricExporter / AggregationConfig builders)
yaml_logs_testpasses 20/20 (exercises LogRecordProcessor /LogRecordExporter builders)
yaml_resource_testpasses 13/13 (exercises ResourceAttributeValueSetter)yaml_distribution_testpasses 3/3clang-format --dry-run --Werrorclean on the modified filePart of #2053