Skip to content

[CONFIGURATION] Internal logging cleanup - #4479

Open
dbarker wants to merge 8 commits into
open-telemetry:mainfrom
dbarker:config_internal_logging_cleanup
Open

[CONFIGURATION] Internal logging cleanup#4479
dbarker wants to merge 8 commits into
open-telemetry:mainfrom
dbarker:config_internal_logging_cleanup

Conversation

@dbarker

@dbarker dbarker commented Aug 25, 2026

Copy link
Copy Markdown
Member

Fixes # (issue)

This PR cleans up internal logging from declarative configuration.

The following use cases are addressed:

  • The internal logger log level is configurable in yaml. The SDK builder should set that log level independent of the providers being installed to the global singletons.
  • If yaml options are configured that the SDK does not yet support the user should see a warning indicating the option is ignored.
  • If the user configures TLS but the SDK (or endpoint) do not support TLS then a warning must be logged.
  • When configuring debug logs the user should see a helpful stream of log lines that indicate important top level configuration options (SDK enabled or disabled) and the builders invoked.
  • As developers we should be able to create an SDK configuration from the kitchen-sink.yaml and see warnings for all the components that still need configuration support.

Changes

  • set the model configured log_level as soon as the SDK is successfully created
  • move ryml document development debug logging to a compile time option
  • add warning logs to the exporters when unsupported options are configured and when TLS configuration is not applied as expected.
  • clean up internal logging and testing of the example_yaml project
  • enable testing of the yaml example with install tests
  • add missing yaml elements to the kitchen-sink.yaml

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 Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.09836% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.10%. Comparing base (cd252b4) to head (426a787).

Files with missing lines Patch % Lines
sdk/src/configuration/sdk_builder.cc 52.73% 26 Missing ⚠️
sdk/src/configuration/configuration_parser.cc 66.67% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4479      +/-   ##
==========================================
+ Coverage   83.09%   83.10%   +0.02%     
==========================================
  Files         519      519              
  Lines       20253    20234      -19     
==========================================
- Hits        16827    16814      -13     
+ Misses       3426     3420       -6     
Files with missing lines Coverage Δ
sdk/src/configuration/configured_sdk.cc 85.42% <ø> (-0.29%) ⬇️
sdk/src/configuration/ryml_document_node.cc 83.55% <ø> (-0.04%) ⬇️
sdk/src/configuration/configuration_parser.cc 84.83% <66.67%> (-0.03%) ⬇️
sdk/src/configuration/sdk_builder.cc 61.26% <52.73%> (+0.05%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dbarker
dbarker marked this pull request as ready for review August 25, 2026 20:38
@dbarker
dbarker requested a review from a team as a code owner August 25, 2026 20:38
options.ssl_client_key_path = tls->key_file;
options.ssl_client_cert_path = tls->cert_file;
#else
if (!tls->key_file.empty() || !tls->cert_file.empty())

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The alternative reasonable option is to throw an exception when these tls options are configured but not supported.

else
{
std::fprintf(stdout, "FAILED TO CREATE SDK\n");
std::exit(2);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The return codes are now propagated to main by the InitOtel and CleanupOtel and the functional tests see the program exit with the same return value as before for failure to parse the yaml and failure to create the sdk.

}

// Set the log level if the SDK has been created successfully.
opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(sdk->log_level);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Before the log_level was only set if the user calls ConfiguredSdk::Install which seems unexpected as the Configured providers may be used with or without the Install step.

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.

1 participant