Skip to content

Commit 95baed7

Browse files
owentowent
authored
[SDK] Fix lifetime of GlobalLogHandler (open-telemetry#3221)
1 parent 62b7197 commit 95baed7

File tree

5 files changed

+211
-35
lines changed

5 files changed

+211
-35
lines changed

‎sdk/include/opentelemetry/sdk/common/global_log_handler.h‎

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -99,37 +99,28 @@ class GlobalLogHandler
9999
*
100100
* By default, a default LogHandler is returned.
101101
*/
102-
static inline const nostd::shared_ptr<LogHandler> &GetLogHandler() noexcept
103-
{
104-
return GetHandlerAndLevel().first;
105-
}
102+
static nostd::shared_ptr<LogHandler> GetLogHandler() noexcept;
106103

107104
/**
108105
* Changes the singleton LogHandler.
109106
* This should be called once at the start of application before creating any Provider
110107
* instance.
111108
*/
112-
static inline void SetLogHandler(const nostd::shared_ptr<LogHandler> &eh) noexcept
113-
{
114-
GetHandlerAndLevel().first = eh;
115-
}
109+
static void SetLogHandler(const nostd::shared_ptr<LogHandler> &eh) noexcept;
116110

117111
/**
118112
* Returns the singleton log level.
119113
*
120114
* By default, a default log level is returned.
121115
*/
122-
static inline LogLevel GetLogLevel() noexcept { return GetHandlerAndLevel().second; }
116+
static LogLevel GetLogLevel() noexcept;
123117

124118
/**
125119
* Changes the singleton Log level.
126120
* This should be called once at the start of application before creating any Provider
127121
* instance.
128122
*/
129-
static inline void SetLogLevel(LogLevel level) noexcept { GetHandlerAndLevel().second = level; }
130-
131-
private:
132-
static std::pair<nostd::shared_ptr<LogHandler>, LogLevel> &GetHandlerAndLevel() noexcept;
123+
static void SetLogLevel(LogLevel level) noexcept;
133124
};
134125

135126
} // namespace internal_log
@@ -142,24 +133,23 @@ OPENTELEMETRY_END_NAMESPACE
142133
* To ensure that GlobalLogHandler is the first one to be initialized (and so last to be
143134
* destroyed), it is first used inside the constructors of TraceProvider, MeterProvider
144135
* and LoggerProvider for debug logging. */
145-
#define OTEL_INTERNAL_LOG_DISPATCH(level, message, attributes) \
146-
do \
147-
{ \
148-
using opentelemetry::sdk::common::internal_log::GlobalLogHandler; \
149-
using opentelemetry::sdk::common::internal_log::LogHandler; \
150-
if (level > GlobalLogHandler::GetLogLevel()) \
151-
{ \
152-
break; \
153-
} \
154-
const opentelemetry::nostd::shared_ptr<LogHandler> &log_handler = \
155-
GlobalLogHandler::GetLogHandler(); \
156-
if (!log_handler) \
157-
{ \
158-
break; \
159-
} \
160-
std::stringstream tmp_stream; \
161-
tmp_stream << message; \
162-
log_handler->Handle(level, __FILE__, __LINE__, tmp_stream.str().c_str(), attributes); \
136+
#define OTEL_INTERNAL_LOG_DISPATCH(level, message, attributes) \
137+
do \
138+
{ \
139+
using opentelemetry::sdk::common::internal_log::GlobalLogHandler; \
140+
using opentelemetry::sdk::common::internal_log::LogHandler; \
141+
if (level > GlobalLogHandler::GetLogLevel()) \
142+
{ \
143+
break; \
144+
} \
145+
opentelemetry::nostd::shared_ptr<LogHandler> log_handler = GlobalLogHandler::GetLogHandler(); \
146+
if (!log_handler) \
147+
{ \
148+
break; \
149+
} \
150+
std::stringstream tmp_stream; \
151+
tmp_stream << message; \
152+
log_handler->Handle(level, __FILE__, __LINE__, tmp_stream.str().c_str(), attributes); \
163153
} while (false);
164154

165155
#define OTEL_INTERNAL_LOG_GET_3RD_ARG(arg1, arg2, arg3, ...) arg3

‎sdk/src/common/global_log_handler.cc‎

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,38 @@ namespace common
1313
namespace internal_log
1414
{
1515

16+
namespace
17+
{
18+
struct GlobalLogHandlerData
19+
{
20+
nostd::shared_ptr<LogHandler> handler;
21+
LogLevel log_level;
22+
23+
GlobalLogHandlerData()
24+
: handler(nostd::shared_ptr<LogHandler>(new DefaultLogHandler)), log_level(LogLevel::Warning)
25+
{}
26+
~GlobalLogHandlerData() { is_singleton_destroyed = true; }
27+
28+
GlobalLogHandlerData(const GlobalLogHandlerData &) = delete;
29+
GlobalLogHandlerData(GlobalLogHandlerData &&) = delete;
30+
31+
GlobalLogHandlerData &operator=(const GlobalLogHandlerData &) = delete;
32+
GlobalLogHandlerData &operator=(GlobalLogHandlerData &&) = delete;
33+
34+
static GlobalLogHandlerData &Instance() noexcept;
35+
static bool is_singleton_destroyed;
36+
};
37+
38+
bool GlobalLogHandlerData::is_singleton_destroyed = false;
39+
40+
GlobalLogHandlerData &GlobalLogHandlerData::Instance() noexcept
41+
{
42+
static GlobalLogHandlerData instance;
43+
return instance;
44+
}
45+
46+
} // namespace
47+
1648
LogHandler::~LogHandler() {}
1749

1850
void DefaultLogHandler::Handle(LogLevel level,
@@ -57,11 +89,43 @@ void NoopLogHandler::Handle(LogLevel,
5789
const sdk::common::AttributeMap &) noexcept
5890
{}
5991

60-
std::pair<nostd::shared_ptr<LogHandler>, LogLevel> &GlobalLogHandler::GetHandlerAndLevel() noexcept
92+
nostd::shared_ptr<LogHandler> GlobalLogHandler::GetLogHandler() noexcept
93+
{
94+
if OPENTELEMETRY_UNLIKELY_CONDITION (GlobalLogHandlerData::is_singleton_destroyed)
95+
{
96+
return nostd::shared_ptr<LogHandler>();
97+
}
98+
99+
return GlobalLogHandlerData::Instance().handler;
100+
}
101+
102+
void GlobalLogHandler::SetLogHandler(const nostd::shared_ptr<LogHandler> &eh) noexcept
103+
{
104+
if OPENTELEMETRY_UNLIKELY_CONDITION (GlobalLogHandlerData::is_singleton_destroyed)
105+
{
106+
return;
107+
}
108+
109+
GlobalLogHandlerData::Instance().handler = eh;
110+
}
111+
112+
LogLevel GlobalLogHandler::GetLogLevel() noexcept
61113
{
62-
static std::pair<nostd::shared_ptr<LogHandler>, LogLevel> handler_and_level{
63-
nostd::shared_ptr<LogHandler>(new DefaultLogHandler), LogLevel::Warning};
64-
return handler_and_level;
114+
if OPENTELEMETRY_UNLIKELY_CONDITION (GlobalLogHandlerData::is_singleton_destroyed)
115+
{
116+
return LogLevel::None;
117+
}
118+
119+
return GlobalLogHandlerData::Instance().log_level;
120+
}
121+
122+
void GlobalLogHandler::SetLogLevel(LogLevel level) noexcept
123+
{
124+
if OPENTELEMETRY_UNLIKELY_CONDITION (GlobalLogHandlerData::is_singleton_destroyed)
125+
{
126+
return;
127+
}
128+
GlobalLogHandlerData::Instance().log_level = level;
65129
}
66130

67131
} // namespace internal_log

‎sdk/test/common/BUILD‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,20 @@ cc_test(
160160
],
161161
)
162162

163+
cc_test(
164+
name = "global_log_handle_singleton_lifetime_test",
165+
srcs = [
166+
"global_log_handle_singleton_lifetime_test.cc",
167+
],
168+
tags = ["test"],
169+
deps = [
170+
"//api",
171+
"//sdk:headers",
172+
"//sdk/src/common:global_log_handler",
173+
"@com_google_googletest//:gtest_main",
174+
],
175+
)
176+
163177
cc_test(
164178
name = "attributemap_hash_test",
165179
srcs = [

‎sdk/test/common/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ foreach(
1111
attribute_utils_test
1212
attributemap_hash_test
1313
global_log_handle_test
14+
global_log_handle_singleton_lifetime_test
1415
env_var_test)
1516

1617
add_executable(${testname} "${testname}.cc")

‎sdk/test/common/global_log_handle_singleton_lifetime_test.cc‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
#include <gtest/gtest.h>
5+
#include <cstdlib>
6+
#include <cstring>
7+
#include <iostream>
8+
9+
#include "opentelemetry/nostd/shared_ptr.h"
10+
#include "opentelemetry/sdk/common/attribute_utils.h"
11+
#include "opentelemetry/sdk/common/global_log_handler.h"
12+
13+
class GlobalLogHandlerChecker
14+
{
15+
public:
16+
GlobalLogHandlerChecker() {}
17+
~GlobalLogHandlerChecker()
18+
{
19+
using opentelemetry::sdk::common::internal_log::GlobalLogHandler;
20+
auto handle = GlobalLogHandler::GetLogHandler();
21+
if (handle && custom_handler_destroyed)
22+
{
23+
OTEL_INTERNAL_LOG_ERROR("GlobalLogHandler should be destroyed here");
24+
abort();
25+
}
26+
std::cout << "GlobalLogHandlerChecker destroyed and check pass.\n";
27+
}
28+
29+
void Print() { std::cout << "GlobalLogHandlerChecker constructed\n"; }
30+
31+
GlobalLogHandlerChecker(const GlobalLogHandlerChecker &) = delete;
32+
GlobalLogHandlerChecker(GlobalLogHandlerChecker &&) = delete;
33+
GlobalLogHandlerChecker &operator=(const GlobalLogHandlerChecker &) = delete;
34+
GlobalLogHandlerChecker &operator=(GlobalLogHandlerChecker &&) = delete;
35+
36+
static bool custom_handler_destroyed;
37+
};
38+
bool GlobalLogHandlerChecker::custom_handler_destroyed = false;
39+
40+
namespace
41+
{
42+
static GlobalLogHandlerChecker &ConstructChecker()
43+
{
44+
static GlobalLogHandlerChecker checker;
45+
return checker;
46+
}
47+
} // namespace
48+
49+
class CustomLogHandler : public opentelemetry::sdk::common::internal_log::LogHandler
50+
{
51+
public:
52+
~CustomLogHandler() override
53+
{
54+
GlobalLogHandlerChecker::custom_handler_destroyed = true;
55+
std::cout << "Custom Gobal Log Handle destroyed\n";
56+
}
57+
58+
void Handle(opentelemetry::sdk::common::internal_log::LogLevel level,
59+
const char *,
60+
int,
61+
const char *msg,
62+
const opentelemetry::sdk::common::AttributeMap &) noexcept override
63+
{
64+
if (level == opentelemetry::sdk::common::internal_log::LogLevel::Debug)
65+
{
66+
std::cout << "Custom Gobal Log Handle[Debug]: " << msg << "\n";
67+
}
68+
else if (level == opentelemetry::sdk::common::internal_log::LogLevel::Error)
69+
{
70+
std::cout << "Custom Gobal Log Handle[Error]: " << msg << "\n";
71+
}
72+
else if (level == opentelemetry::sdk::common::internal_log::LogLevel::Info)
73+
{
74+
std::cout << "Custom Gobal Log Handle[Info]: " << msg << "\n";
75+
}
76+
else if (level == opentelemetry::sdk::common::internal_log::LogLevel::Warning)
77+
{
78+
std::cout << "Custom Gobal Log Handle[Warning]: " << msg << "\n";
79+
}
80+
++count;
81+
}
82+
83+
size_t count = 0;
84+
};
85+
86+
TEST(GlobalLogHandleSingletonTest, Lifetime)
87+
{
88+
// Setup a new static variable which will be destroyed after the global handle
89+
ConstructChecker().Print();
90+
91+
using opentelemetry::sdk::common::internal_log::GlobalLogHandler;
92+
using opentelemetry::sdk::common::internal_log::LogHandler;
93+
94+
auto custom_log_handler = opentelemetry::nostd::shared_ptr<LogHandler>(new CustomLogHandler{});
95+
opentelemetry::sdk::common::internal_log::GlobalLogHandler::SetLogHandler(custom_log_handler);
96+
auto before_count = static_cast<CustomLogHandler *>(custom_log_handler.get())->count;
97+
opentelemetry::sdk::common::AttributeMap attributes = {
98+
{"url", "https://opentelemetry.io/"}, {"content-length", 0}, {"content-type", "text/html"}};
99+
OTEL_INTERNAL_LOG_ERROR("Error message");
100+
OTEL_INTERNAL_LOG_DEBUG("Debug message. Headers:", attributes);
101+
EXPECT_EQ(before_count + 1, static_cast<CustomLogHandler *>(custom_log_handler.get())->count);
102+
103+
{
104+
auto handle = GlobalLogHandler::GetLogHandler();
105+
EXPECT_TRUE(!!handle);
106+
}
107+
}

0 commit comments

Comments
 (0)