Skip to content

[CODE HEALTH] Fix clang-tidy bugprone-throwing-static-initialization warnings#4206

Open
RaviTriv wants to merge 16 commits into
open-telemetry:mainfrom
RaviTriv:ravtrive/clang-tidy-throwing-static-init
Open

[CODE HEALTH] Fix clang-tidy bugprone-throwing-static-initialization warnings#4206
RaviTriv wants to merge 16 commits into
open-telemetry:mainfrom
RaviTriv:ravtrive/clang-tidy-throwing-static-init

Conversation

@RaviTriv

@RaviTriv RaviTriv commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #4197

Changes

Addressing clang-tidy bugprone-throwing-static-initialization warnings

Fixes:

  • Replaced const with constexpr for compile-time constants
  • Replaced const std::string with constexpr const char *
  • Used nostd::string_view for a non-owning view
  • Converted shared objects to static local variables
  • Moved option objects out of namespace scope into main() and passed by reference
  • Moved class static member into function scope at use

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.27027% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.80%. Comparing base (4b58597) to head (64ac1a1).

Files with missing lines Patch % Lines
exporters/zipkin/src/recordable.cc 40.00% 9 Missing ⚠️
...ntelemetry/sdk/metrics/state/sync_metric_storage.h 66.67% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4206      +/-   ##
==========================================
- Coverage   82.82%   82.80%   -0.01%     
==========================================
  Files         415      415              
  Lines       17431    17452      +21     
==========================================
+ Hits        14435    14449      +14     
- Misses       2996     3003       +7     
Files with missing lines Coverage Δ
...pi/include/opentelemetry/baggage/baggage_context.h 100.00% <ø> (ø)
...ude/opentelemetry/common/key_value_iterable_view.h 100.00% <ø> (ø)
...pentelemetry/trace/span_context_kv_iterable_view.h 84.22% <ø> (ø)
exporters/prometheus/src/exporter_utils.cc 89.28% <100.00%> (+0.18%) ⬆️
...lemetry/ext/http/client/curl/http_operation_curl.h 90.91% <ø> (ø)
...entelemetry/sdk/metrics/state/attributes_hashmap.h 97.15% <100.00%> (+0.13%) ⬆️
sdk/include/opentelemetry/sdk/trace/tracer.h 100.00% <ø> (ø)
sdk/src/metrics/instrument_metadata_validator.cc 100.00% <ø> (ø)
sdk/src/metrics/state/sync_metric_storage.cc 94.74% <100.00%> (ø)
sdk/src/trace/tracer.cc 87.50% <100.00%> (+0.16%) ⬆️
... and 2 more

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

@RaviTriv RaviTriv force-pushed the ravtrive/clang-tidy-throwing-static-init branch from 60e25bf to 3436db9 Compare July 3, 2026 01:25
@RaviTriv RaviTriv marked this pull request as ready for review July 3, 2026 01:25
@RaviTriv RaviTriv requested a review from a team as a code owner July 3, 2026 01:25
@RaviTriv RaviTriv force-pushed the ravtrive/clang-tidy-throwing-static-init branch from 3436db9 to d873fbf Compare July 3, 2026 10:55

@dbarker dbarker 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.

Thanks for the PR! Please see some initial feedback below and take a pass at cleanup to use constexpr char [] where possible for literals and using the project's naming convention for new methods when needed.

Comment thread api/include/opentelemetry/baggage/baggage_context.h Outdated
Comment thread exporters/zipkin/src/recordable.cc Outdated
Comment thread sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h Outdated
Comment thread sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h Outdated
Comment thread sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h Outdated
Comment thread sdk/test/logs/logger_sdk_test.cc Outdated
Comment thread sdk/test/logs/logger_sdk_test.cc Outdated

@dbarker dbarker 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.

Thanks! Looks good with a few minor optional changes. Please resolve the conflicts. This PR needs a second review/approval of the API file changes.

Comment thread functional/otlp/func_grpc_main.cc Outdated
remaining_argc--;
remaining_argv++;
opt_endpoint = *remaining_argv;
GetOptEndpoint() = *remaining_argv;

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.

Assigning a value to the return value of the get accessor is a bit unexpected. Consider keeping the endpoint string as static global, creating a constexpr literal for the default value, and initialize the string in the main function.

Comment thread functional/otlp/func_grpc_main.cc Outdated
struct test_case
{
std::string m_name;
const char *m_name;

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.

can this be a string_view?

@RaviTriv RaviTriv force-pushed the ravtrive/clang-tidy-throwing-static-init branch from 35a92d6 to 64ac1a1 Compare July 8, 2026 00:07
@RaviTriv

RaviTriv commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! Looks good with a few minor optional changes. Please resolve the conflicts. This PR needs a second review/approval of the API file changes.

Thank you!

Rebased and pushed a commit for the suggested changes.

@marcalff would you mind giving the second review when you have a chance :)

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.

[CODE HEALTH] clang-tidy bugprone-throwing-static-initialization warnings

2 participants