[owent]

Fixes #3588

Changes

Please provide a brief description of the changes here.

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

[netlify]

✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name	Link
🔨 Latest commit	4123b34
🔍 Latest deploy log	https://app.netlify.com/projects/opentelemetry-cpp-api-docs/deploys/68a37223638a2500089df1ab

[Copilot]

Pull Request Overview

This PR removes duplicated deprecated warnings in the logs headers by adding pragma directives to suppress deprecation warnings around code that uses deprecated APIs. The changes ensure that deprecation warnings are properly silenced during compilation across different compiler platforms (MSVC, GCC, and Clang).

• Adds compiler-specific pragma directives to suppress deprecation warnings
• Wraps usage of deprecated APIs with warning suppression blocks
• Ensures consistent warning suppression across MSVC, GCC, and Clang compilers

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
api/include/opentelemetry/logs/provider.h	Adds pragma directives around NoopEventLoggerProvider instantiation to suppress deprecation warnings
api/include/opentelemetry/logs/noop.h	Adds pragma directives around NoopEventLoggerProvider constructor to suppress deprecation warnings

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The pragma directive blocks are duplicated across multiple files. Consider creating a macro or header file to define OPENTELEMETRY_SUPPRESS_DEPRECATED_START and OPENTELEMETRY_SUPPRESS_DEPRECATED_END to reduce code duplication and improve maintainability.

Suggested change

	# endif
	OPENTELEMETRY_SUPPRESS_DEPRECATED_START
	static nostd::shared_ptr<EventLoggerProvider> provider(new NoopEventLoggerProvider);
	return provider;
	OPENTELEMETRY_SUPPRESS_DEPRECATED_END

[Copilot]

The pragma directive blocks are duplicated across multiple files. Consider creating a macro or header file to define OPENTELEMETRY_SUPPRESS_DEPRECATED_START and OPENTELEMETRY_SUPPRESS_DEPRECATED_END to reduce code duplication and improve maintainability.

Suggested change

	OPENTELEMETRY_SUPPRESS_DEPRECATED_START
	NoopEventLoggerProvider() : event_logger_{nostd::shared_ptr<EventLogger>(new NoopEventLogger())}
	{}
	OPENTELEMETRY_SUPPRESS_DEPRECATED_END

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.03%. Comparing base (a18feb9) to head (4123b34).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3592      +/-   ##
==========================================
+ Coverage   90.02%   90.03%   +0.02%     
==========================================
  Files         220      220              
  Lines        7069     7069              
==========================================
+ Hits         6363     6364       +1     
+ Misses        706      705       -1     

Files with missing lines	Coverage Δ
api/include/opentelemetry/logs/noop.h	77.78% <ø> (ø)
api/include/opentelemetry/logs/provider.h	100.00% <ø> (ø)

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

[nikhilbhatia08]

@owent The same test of CI / Bazel on MacOS fails on my pull request too with the same log, sorry but I'm not able to understand why it fails, can you please tell the fix for it. Thanks !

[lalitb]

@owent The same test of CI / Bazel on MacOS fails on my pull request too with the same log, sorry but I'm not able to understand why it fails, can you please tell the fix for it. Thanks !

Not for sure, but the bazel cache could be corrupted here. If so, a nit PR deleting the cache should fix it. I believe the PR needn't be merged, just run on the main build to clean the cache. Or else, provide a custom CI action (through workflow_dispatch) to let maintainers delete the cache manually on such errors.

[lalitb]

Thanks. LGTM.

[marcalff]

LGTM, thanks for the fix.

[lalitb]

The only limitation with this change is that anyone using EventLogger won’t see a deprecation warning, so they may never realize it’s deprecated. That said, I think the current PR is still the best path forward. Splitting EventLogger into its own header could address this, but that would be an API-breaking change.

[marcalff]

The only limitation with this change is that anyone using EventLogger won’t see a deprecation warning, so they may never realize it’s deprecated. That said, I think the current PR is still the best path forward. Splitting EventLogger into its own header could address this, but that would be an API-breaking change.

In provider.h, warnings are silenced only in the implementation of GetEventProvider(), because it uses deprecated code, which was the reported issue.

Not tested, but I think anyone calling GetEventProvider() will still see a deprecation warning, because warnings are raised when using a deprecated api, not just when declaring an api deprecated..

[marcalff]

Seen in CI:

/home/runner/work/opentelemetry-cpp/opentelemetry-cpp/api/test/logs/provider_test.cc:90:23: warning: 'GetEventLoggerProvider' is deprecated [-Wdeprecated-declarations]
   90 |   auto tf = Provider::GetEventLoggerProvider();
      |                       ^
/home/runner/work/opentelemetry-cpp/opentelemetry-cpp/api/include/opentelemetry/logs/provider.h:57:3: note: 'GetEventLoggerProvider' has been explicitly marked deprecated here
   57 |   OPENTELEMETRY_DEPRECATED static nostd::shared_ptr<EventLoggerProvider>

So, calling deprecated api Provider::GetEventLoggerProvider() still raises warnings, as expected.