Skip to content

Commit

Permalink
[SDK] DefaultLogHandler to print errors to std::cerr, add LogLevel::N…
Browse files Browse the repository at this point in the history
…one (#2622)
  • Loading branch information
marcalff authored Apr 13, 2024
1 parent cd22f0f commit 78947b2
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 16 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ Increment the:
[#2627](https://github.com/open-telemetry/opentelemetry-cpp/pull/2627)
* [PROTO] Upgrade to opentelemetry-proto v1.2.0
[#2631](https://github.com/open-telemetry/opentelemetry-cpp/pull/2631)
* [SDK] DefaultLogHandler to print errors to std::cerr, add LogLevel::None
[#2622](https://github.com/open-telemetry/opentelemetry-cpp/pull/2622)

Important changes:

Expand All @@ -45,6 +47,23 @@ Important changes:
* As part of this change, the script `ci/setup_cmake.sh` was renamed
to `ci/setup_googletest.sh`, for clarity, now that this script
only installs googletest.
* [SDK] DefaultLogHandler to print to std::cerr, add LogLevel::None
[#2622](https://github.com/open-telemetry/opentelemetry-cpp/pull/2622)
* Change DefaultLogHandler output
* Before, the default internal logger, DefaultLogHandler,
used to print to std::cout.
* Now, DefaultLogHandler prints errors and warnings to std::cerr,
as expected, while printing info and debug messages to std::cout.
* Applications that expected to find the opentelemetry-cpp internal
error log in std::cout may need adjustments, either by looking
at std::cerr instead, or by using a custom log handler.
* Additional LogLevel::None
* LogLevel::None is a new supported log level, which does not print
any message.
* Custom log handlers may need to implement a new case, to avoid
compiler warnings.
* Numbering of log levels like OTEL_INTERNAL_LOG_LEVEL_ERROR
has changed, which requires to rebuild, as the SDK ABI differs.

## [1.14.2] 2024-02-27

Expand Down
10 changes: 5 additions & 5 deletions exporters/ostream/test/ostream_log_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ TEST(OStreamLogRecordExporter, Shutdown)
auto exporter =
std::unique_ptr<sdklogs::LogRecordExporter>(new exporterlogs::OStreamLogRecordExporter);

// Save cout's original buffer here
std::streambuf *original = std::cout.rdbuf();
// Save cerr original buffer here
std::streambuf *original = std::cerr.rdbuf();

// Redirect cout to our stringstream buffer
// Redirect cerr to our stringstream buffer
std::stringstream output;
std::cout.rdbuf(output.rdbuf());
std::cerr.rdbuf(output.rdbuf());

EXPECT_TRUE(exporter->Shutdown());

Expand All @@ -64,7 +64,7 @@ TEST(OStreamLogRecordExporter, Shutdown)
exporter->Export(nostd::span<std::unique_ptr<sdklogs::Recordable>>(&record, 1));

// Restore original stringstream buffer
std::cout.rdbuf(original);
std::cerr.rdbuf(original);
std::string err_message =
"[Ostream Log Exporter] Exporting 1 log(s) failed, exporter is shutdown";
EXPECT_TRUE(output.str().find(err_message) != std::string::npos);
Expand Down
4 changes: 2 additions & 2 deletions exporters/ostream/test/ostream_span_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ TEST(OStreamSpanExporter, Shutdown)
auto recordable = processor->MakeRecordable();
recordable->SetName("Test Span");

// Capture the output of cout
const auto captured = WithOStreamCapture(std::cout, [&]() {
// Capture the output of cerr
const auto captured = WithOStreamCapture(std::cerr, [&]() {
EXPECT_TRUE(processor->Shutdown());
processor->OnEnd(std::move(recordable));
});
Expand Down
2 changes: 2 additions & 0 deletions functional/otlp/func_http_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ class TestLogHandler : public opentelemetry::sdk::common::internal_log::LogHandl

switch (level)
{
case opentelemetry::sdk::common::internal_log::LogLevel::None:
break;
case opentelemetry::sdk::common::internal_log::LogLevel::Error:
std::cout << " - [E] " << msg << std::endl;
parse_error_msg(&g_test_result, msg);
Expand Down
20 changes: 12 additions & 8 deletions sdk/include/opentelemetry/sdk/common/global_log_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/version.h"

#define OTEL_INTERNAL_LOG_LEVEL_ERROR 0
#define OTEL_INTERNAL_LOG_LEVEL_WARN 1
#define OTEL_INTERNAL_LOG_LEVEL_INFO 2
#define OTEL_INTERNAL_LOG_LEVEL_DEBUG 3
#define OTEL_INTERNAL_LOG_LEVEL_NONE 0
#define OTEL_INTERNAL_LOG_LEVEL_ERROR 1
#define OTEL_INTERNAL_LOG_LEVEL_WARN 2
#define OTEL_INTERNAL_LOG_LEVEL_INFO 3
#define OTEL_INTERNAL_LOG_LEVEL_DEBUG 4
#ifndef OTEL_INTERNAL_LOG_LEVEL
// DEBUG by default, we can change log level on runtime
# define OTEL_INTERNAL_LOG_LEVEL OTEL_INTERNAL_LOG_LEVEL_DEBUG
Expand All @@ -29,16 +30,19 @@ namespace internal_log

enum class LogLevel
{
Error = 0,
Warning,
Info,
Debug
None = OTEL_INTERNAL_LOG_LEVEL_NONE,
Error = OTEL_INTERNAL_LOG_LEVEL_ERROR,
Warning = OTEL_INTERNAL_LOG_LEVEL_WARN,
Info = OTEL_INTERNAL_LOG_LEVEL_INFO,
Debug = OTEL_INTERNAL_LOG_LEVEL_DEBUG
};

inline std::string LevelToString(LogLevel level)
{
switch (level)
{
case LogLevel::None:
return "None";
case LogLevel::Error:
return "Error";
case LogLevel::Warning:
Expand Down
16 changes: 15 additions & 1 deletion sdk/src/common/global_log_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,21 @@ void DefaultLogHandler::Handle(LogLevel level,
}
output_s << std::endl;
// TBD - print attributes
std::cout << output_s.str(); // thread safe.

switch (level)
{
case LogLevel::Error:
case LogLevel::Warning:
std::cerr << output_s.str(); // thread safe.
break;
case LogLevel::Info:
case LogLevel::Debug:
std::cout << output_s.str(); // thread safe.
break;
case LogLevel::None:
default:
break;
}
}

void NoopLogHandler::Handle(LogLevel,
Expand Down
8 changes: 8 additions & 0 deletions sdk/test/common/global_log_handle_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ TEST(GlobalLogHandleTest, CustomLogHandler)
OTEL_INTERNAL_LOG_WARN("Warning message");
EXPECT_EQ(before_count + 5, static_cast<CustomLogHandler *>(custom_log_handler.get())->count);

opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(
opentelemetry::sdk::common::internal_log::LogLevel::None);
OTEL_INTERNAL_LOG_ERROR("Error message");
OTEL_INTERNAL_LOG_DEBUG("Debug message. Headers:", attributes);
OTEL_INTERNAL_LOG_INFO("Info message");
OTEL_INTERNAL_LOG_WARN("Warning message");
EXPECT_EQ(before_count + 5, static_cast<CustomLogHandler *>(custom_log_handler.get())->count);

opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(backup_log_handle);
opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogLevel(backup_log_level);
}

2 comments on commit 78947b2

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 78947b2 Previous: cd22f0f Ratio
BM_AlwaysOffSamplerConstruction 1.60506945275492 ns/iter 0.706799776535672 ns/iter 2.27
BM_TraceIdRatioBasedSamplerShouldSample 14.62456511227923 ns/iter 7.126587070047527 ns/iter 2.05
BM_SpanCreation 1082.5645439567602 ns/iter 517.3550318112876 ns/iter 2.09
BM_NoopSpanCreation 291.3721170602957 ns/iter 130.783327008994 ns/iter 2.23

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 78947b2 Previous: cd22f0f Ratio
BM_ProcYieldSpinLockThrashing/2/process_time/real_time 7.602524757385254 ms/iter 0.1800376081397266 ms/iter 42.23
BM_ProcYieldSpinLockThrashing/4/process_time/real_time 12.021411548961293 ms/iter 0.6652311845259233 ms/iter 18.07
BM_NaiveSpinLockThrashing/1/process_time/real_time 6.719659356509938 ms/iter 0.11212611198425293 ms/iter 59.93

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.