Skip to content

[CODE HEALTH] Move registry.cc propagator builders into anonymous namespace#4121

Merged
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:codehealth/registry-internal-linkage
Jun 4, 2026
Merged

[CODE HEALTH] Move registry.cc propagator builders into anonymous namespace#4121
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:codehealth/registry-internal-linkage

Conversation

@thc1006
Copy link
Copy Markdown
Contributor

@thc1006 thc1006 commented Jun 4, 2026

Summary

Wraps the 5 file-local TextMapPropagatorBuilder subclasses in
sdk/src/configuration/registry.cc within an anonymous namespace,
clearing the 5 misc-use-internal-linkage warnings clang-tidy reports
on this file. First production-code PR of the
misc-use-internal-linkage campaign opened by #4115 / #4116.

Changes

  • sdk/src/configuration/registry.cc: insert namespace { after the
    namespace configuration { opening and } // namespace before
    Registry::Registry(). Wrap covers TraceContextBuilder,
    BaggageBuilder, B3Builder, B3MultiBuilder, and JaegerBuilder.
    The 5 classes are only instantiated in Registry::Registry() via
    std::make_unique and are not referenced from any other translation
    unit (verified by repository-wide grep).
  • .github/workflows/clang-tidy.yaml: abiv1 371 -> 366 (-5),
    abiv2 377 -> 372 (-5).
  • CHANGELOG.md: one bullet under [Unreleased].

Linkage rationale

Registry::Registry() and the other Registry:: member function
definitions remain at namespace scope outside the anonymous namespace,
because as members of class Registry (declared in registry.h) 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.

Test plan

  • Local build of opentelemetry_configuration,
    yaml_propagator_test, and yaml_test succeeded (109/109 ninja
    steps with -DWITH_CONFIGURATION=ON)
  • yaml_propagator_test passes 9/9 (directly exercises the 5
    builders via YAML propagator configuration)
  • yaml_test passes 50/50
  • 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 03:43
…espace

Wraps the 5 file-local TextMapPropagatorBuilder subclasses in
sdk/src/configuration/registry.cc within an anonymous namespace inside
the existing opentelemetry::sdk::configuration namespace. This gives
them internal linkage and clears the 5 misc-use-internal-linkage
warnings clang-tidy reports on this file.

The wrap covers TraceContextBuilder, BaggageBuilder, B3Builder,
B3MultiBuilder, and JaegerBuilder. The 5 classes are only instantiated
inside Registry::Registry() via std::make_unique. None of them are
declared in registry.h or referenced from any other translation unit.

Registry 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
still reachable from Registry::Registry() via the implicit
using-directive that hoists anonymous namespace symbols into the
enclosing configuration namespace.

Ratchet:
* abiv1-preview warning_limit lowered from 371 to 366
* abiv2-preview warning_limit lowered from 377 to 372

Part of open-telemetry#2053

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006 thc1006 force-pushed the codehealth/registry-internal-linkage branch from 64fc56f to b5ac2f3 Compare June 4, 2026 03:43
@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 (d8cfe2f) to head (d6e7146).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4121   +/-   ##
=======================================
  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.

Copy link
Copy Markdown
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix.

@marcalff marcalff merged commit 5533a30 into open-telemetry:main Jun 4, 2026
71 checks passed
@thc1006 thc1006 deleted the codehealth/registry-internal-linkage branch June 4, 2026 15:47
thc1006 added a commit to thc1006/opentelemetry-cpp that referenced this pull request Jun 4, 2026
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>
marcalff pushed a commit that referenced this pull request Jun 5, 2026
…4122)

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 #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 #2053

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
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