Skip to content

[CODE HEALTH] Move sdk_builder.cc builders into anonymous namespace#4122

Open
thc1006 wants to merge 1 commit into
open-telemetry:mainfrom
thc1006:codehealth/sdk-builder-internal-linkage
Open

[CODE HEALTH] Move sdk_builder.cc builders into anonymous namespace#4122
thc1006 wants to merge 1 commit into
open-telemetry:mainfrom
thc1006:codehealth/sdk-builder-internal-linkage

Conversation

@thc1006
Copy link
Copy Markdown
Contributor

@thc1006 thc1006 commented Jun 4, 2026

Summary

Wraps the 10 file-local builder classes in
sdk/src/configuration/sdk_builder.cc within an anonymous namespace,
clearing the 10 misc-use-internal-linkage warnings clang-tidy reports
on this file. Second production-code PR of the misc-use-internal-linkage
campaign, after #4121 (registry.cc).

Changes

  • sdk/src/configuration/sdk_builder.cc: insert namespace { after the
    using common::WildcardMatch; declaration at line 189 and
    } // namespace before the first SdkBuilder:: member function
    definition at line 750. Wrap covers ResourceAttributeValueSetter,
    SamplerBuilder, SpanProcessorBuilder, SpanExporterBuilder,
    MetricReaderBuilder, PushMetricExporterBuilder,
    PullMetricExporterBuilder, AggregationConfigBuilder,
    LogRecordProcessorBuilder, and LogRecordExporterBuilder. The 10
    classes 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 scope
outside the anonymous namespace, because as members of class SdkBuilder
they 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 configuration namespace. This is the same structure used in
#4121.

Test plan

  • Local build of opentelemetry_configuration and 7 yaml tests
    (-DWITH_CONFIGURATION=ON, 119/119 ninja steps)
  • yaml_propagator_test passes 9/9
  • yaml_test passes 50/50
  • yaml_trace_test passes 38/38 (exercises Sampler / SpanProcessor /
    SpanExporter builders)
  • yaml_metrics_test passes 32/32 (exercises MetricReader /
    PushMetricExporter / PullMetricExporter / AggregationConfig builders)
  • yaml_logs_test passes 20/20 (exercises LogRecordProcessor /
    LogRecordExporter builders)
  • yaml_resource_test passes 13/13 (exercises ResourceAttributeValueSetter)
  • yaml_distribution_test passes 3/3
  • clang-format --dry-run --Werror clean on the modified file
  • DCO signed off

Part of #2053

@thc1006 thc1006 requested a review from a team as a code owner June 4, 2026 17:53
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>
@thc1006 thc1006 force-pushed the codehealth/sdk-builder-internal-linkage branch from 2c3f1d6 to 19c7879 Compare June 4, 2026 17:53
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.00%. Comparing base (5533a30) to head (19c7879).

Additional details and impacted files

Impacted file tree graph

@@           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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants