Skip to content

[CODE HEALTH] Enable clang-tidy modernize-deprecated-headers and fix violations - #4349

Merged
marcalff merged 6 commits into
open-telemetry:mainfrom
om7057:chore/modernize-deprecated-headers
Aug 4, 2026
Merged

[CODE HEALTH] Enable clang-tidy modernize-deprecated-headers and fix violations#4349
marcalff merged 6 commits into
open-telemetry:mainfrom
om7057:chore/modernize-deprecated-headers

Conversation

@om7057

@om7057 om7057 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #4348

Changes

Enables the modernize-deprecated-headers clang-tidy check, which was disabled in .clang-tidy, and replaces deprecated C headers with their C++ equivalents (stdint.h -> cstdint, stddef.h -> cstddef, stdlib.h -> cstdlib, string.h -> cstring, stdio.h -> cstdio, ctype.h -> cctype, limits.h -> climits, assert.h -> cassert) across api, sdk, exporters, ext, examples, opentracing-shim, resource_detectors, test_common, and functional.

As noted in the issue, IWYU was still recommending the deprecated headers, so .iwyu.imp is updated first to map struct tm/struct FILE usages to <ctime>/<cstdio> instead of <time.h>/<stdio.h>.

Two now-duplicate includes that this change surfaced were also removed, in api/test/core/timestamp_test.cc and exporters/elasticsearch/src/es_log_recordable.cc.

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

Replace deprecated C headers (stdint.h, stddef.h, stdlib.h, string.h,
stdio.h, ctype.h, limits.h, assert.h) with their C++ equivalents
across api, sdk, exporters, ext, examples, opentracing-shim,
resource_detectors, test_common, and functional tests.

Also fix the IWYU mapping file, which was recommending the deprecated
<time.h>/<stdio.h> headers instead of <ctime>/<cstdio>.

Fixes open-telemetry#4348
@om7057
om7057 requested a review from a team as a code owner August 3, 2026 18:34
om7057 added 2 commits August 4, 2026 00:17
The previous commit converted deprecated C headers to their C++
equivalents but didn't re-sort the resulting #include blocks,
which clang-format's CI check flagged.
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.86%. Comparing base (89729e1) to head (bafac0f).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4349   +/-   ##
=======================================
  Coverage   80.86%   80.86%           
=======================================
  Files         450      450           
  Lines       19216    19216           
=======================================
  Hits        15537    15537           
  Misses       3679     3679           
Files with missing lines Coverage Δ
...ude/opentelemetry/common/key_value_iterable_view.h 100.00% <ø> (ø)
api/include/opentelemetry/common/string_util.h 87.50% <ø> (ø)
...pi/include/opentelemetry/context/runtime_context.h 92.14% <ø> (ø)
api/include/opentelemetry/logs/logger.h 81.12% <ø> (ø)
api/include/opentelemetry/trace/noop.h 100.00% <ø> (ø)
api/include/opentelemetry/trace/span_context.h 100.00% <ø> (ø)
api/include/opentelemetry/trace/trace_state.h 94.12% <ø> (ø)
...emetry/exporters/elasticsearch/es_log_recordable.h 0.00% <ø> (ø)
exporters/elasticsearch/src/es_log_recordable.cc 65.39% <ø> (ø)
...telemetry/exporters/memory/in_memory_metric_data.h 100.00% <ø> (ø)
... and 51 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The initial pass only matched unindented #include directives and
missed headers included inside #if/#else branches (e.g. "#  include
<assert.h>"), which is why clang-tidy's CI check still flagged 9
extra modernize-deprecated-headers warnings. Also drops two includes
that became duplicates once the guarded branch matched an
already-unconditional include of the same C++ header.

@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 cleanup! Please see feedback below.

Comment thread api/include/opentelemetry/nostd/internal/absl/base/config.h
Comment thread exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h Outdated
Comment thread exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h Outdated
Comment thread sdk/test/metrics/meter_provider_set_test.cc
api/include/opentelemetry/nostd/internal/absl and
exporters/etw/include/opentelemetry/exporters/etw/TraceLoggingDynamic.h
are vendored third-party sources excluded from clang-tidy, per review
feedback from dbarker.
@om7057

om7057 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@dbarker Thanks for the review!

Please go through it once, and suggest if any feedback.

@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 changes. Approved pending clean CI checks

@dbarker dbarker added the pr:please-review This PR is ready for review label Aug 4, 2026

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

LGTM

@marcalff
marcalff merged commit 41cf715 into open-telemetry:main Aug 4, 2026
72 checks passed
om7057 added a commit to om7057/opentelemetry-cpp that referenced this pull request Aug 4, 2026
Follow-up to open-telemetry#4349: swapping deprecated C headers for their C++
equivalents left the underlying calls unqualified (memcpy, strcmp,
isspace, strtod, snprintf, strftime, etc.), relying on most
implementations also exposing these names in the global namespace,
which isn't guaranteed by the standard. Adds std:: to call sites for
the functions covered by <cstring>, <cctype>, <cstdlib>, <cstdio>,
and <ctime>, plus the FILE and tm types, across api, sdk, exporters,
ext, examples, and functional. Leaves already ::-qualified call
sites, vendored code (absl, TraceLoggingDynamic.h), and
platform-specific functions (gmtime_r/gmtime_s, strtok_s/strtok_r)
untouched.

Fixes open-telemetry#4354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:please-review This PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CODE HEALTH] Enable clang-tidy modernize-deprecated-headers and fixes

3 participants