Skip to content

[CODE HEALTH] Fix clang-tidy bugprone-exception-escape in sum_aggregation.cc#4109

Merged
marcalff merged 4 commits into
open-telemetry:mainfrom
jensecrest:clangtidy-exceptionesc-sumagg
Jun 5, 2026
Merged

[CODE HEALTH] Fix clang-tidy bugprone-exception-escape in sum_aggregation.cc#4109
marcalff merged 4 commits into
open-telemetry:mainfrom
jensecrest:clangtidy-exceptionesc-sumagg

Conversation

@jensecrest
Copy link
Copy Markdown
Contributor

@jensecrest jensecrest commented May 28, 2026

Fixes #2053

Changes

Replace throwing nostd::get() calls with non-throwing nostd::get_if() in 4 noexcept functions. On type mismatch (which should never happen in practice), log an error and return a zero-valued aggregation instead of invoking std::terminate().

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

AI

Assisted-by: This contribution was prepared with the assistance of an AI development tool.

@jensecrest jensecrest requested a review from a team as a code owner May 28, 2026 20:03
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 28, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: jensecrest / name: Jen (c57175b)
  • ✅ login: jensecrest / name: Jennifer Secrest (671202e)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 31, 2026

Codecov Report

❌ Patch coverage is 69.23077% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.95%. Comparing base (b706b56) to head (0d388cb).

Files with missing lines Patch % Lines
sdk/src/metrics/aggregation/sum_aggregation.cc 69.24% 16 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4109      +/-   ##
==========================================
- Coverage   82.01%   81.95%   -0.05%     
==========================================
  Files         385      385              
  Lines       16031    16067      +36     
==========================================
+ Hits        13146    13166      +20     
- Misses       2885     2901      +16     
Files with missing lines Coverage Δ
sdk/src/metrics/aggregation/sum_aggregation.cc 80.00% <69.24%> (-13.75%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

return;
}
const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
point_data_.value_ = nostd::get<int64_t>(point_data_.value_) + value;
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.

should this also be fixed?

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.

Probably.

This should still raise a warning, so it will be picked up by subsequent cleanups.

@lalitb
Copy link
Copy Markdown
Member

lalitb commented Jun 1, 2026

LGTM once the - #4109 (comment) - is resolved. Separately, same issue exists for other aggregations too, probably good to have a followup ?

@ThomsonTan
Copy link
Copy Markdown
Contributor

Add an entry to the changelog?

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.

LGTM

return;
}
const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
point_data_.value_ = nostd::get<int64_t>(point_data_.value_) + value;
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.

Probably.

This should still raise a warning, so it will be picked up by subsequent cleanups.

@marcalff marcalff changed the title Fix clang-tidy bugprone-exception-escape in sum_aggregation.cc [CODE HEALTH] Fix clang-tidy bugprone-exception-escape in sum_aggregation.cc Jun 5, 2026
@marcalff marcalff merged commit d2f09f1 into open-telemetry:main Jun 5, 2026
120 of 121 checks passed
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 cleanup

4 participants