Skip to content

[WIP] Modern lifecycle + Ownership #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/api/LogManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ namespace MAT_NS_BEGIN
return m_logConfiguration;
}

ILogger* LogManagerImpl::GetLogger(const std::string& tenantToken, const std::string& source, const std::string& scope)
std::shared_ptr<ILogger> LogManagerImpl::GetLogger(const std::string& tenantToken, const std::string& source, const std::string& scope)
{
{
LOCKGUARD(m_lock);
Expand All @@ -653,7 +653,7 @@ namespace MAT_NS_BEGIN
auto it = m_loggers.find(hash);
if (it == std::end(m_loggers))
{
m_loggers[hash] = std::make_unique<Logger>(
m_loggers[hash] = std::make_shared<Logger>(
normalizedTenantToken, normalizedSource, scope,
*this, m_context, *m_config);
}
Expand All @@ -662,7 +662,7 @@ namespace MAT_NS_BEGIN
{
m_loggers[hash]->SetLevel(level);
}
return m_loggers[hash].get();
return std::static_pointer_cast<ILogger>(m_loggers[hash]);
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions lib/api/LogManagerImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,15 @@ namespace MAT_NS_BEGIN

class Logger;

using LoggerMap = std::map<std::string, std::unique_ptr<Logger>>;
using LoggerMap = std::map<std::string, std::shared_ptr<Logger>>;

class DeadLoggers
{
public:
void AddMap(LoggerMap&& source);
size_t GetDeadLoggerCount() const noexcept;

std::vector<std::unique_ptr<Logger>> m_deadLoggers;
std::vector<std::shared_ptr<Logger>> m_deadLoggers;
mutable std::mutex m_deadLoggersMutex;
};

Expand Down Expand Up @@ -227,7 +227,7 @@ namespace MAT_NS_BEGIN

virtual ILogConfiguration& GetLogConfiguration() override;

virtual ILogger* GetLogger(std::string const& tenantToken, std::string const& source = std::string(), std::string const& scopeId = std::string()) override;
virtual std::shared_ptr<ILogger> GetLogger(std::string const& tenantToken, std::string const& source = std::string(), std::string const& scopeId = std::string()) override;

LogSessionData* GetLogSessionData() override;
void ResetLogSessionData() override;
Expand Down
10 changes: 5 additions & 5 deletions lib/api/LogManagerProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@

namespace MAT_NS_BEGIN {

ILogManager * LogManagerProvider::Get(
std::unique_ptr<ILogManager> LogManagerProvider::Get(
ILogConfiguration & config,
status_t &status
status_t &/*status*/
)
{
return LogManagerFactory::Get(config, status);
return std::unique_ptr<ILogManager>(LogManagerFactory::Create(config));
}

// TODO: consider utilizing a default reference
ILogManager* LogManagerProvider::Get(
std::unique_ptr<ILogManager> LogManagerProvider::Get(
const char * moduleName,
status_t& status
)
{
return LogManagerFactory::Get(moduleName, status);
return std::unique_ptr<ILogManager>(LogManagerFactory::Get(moduleName, status));
}

/// <summary>
Expand Down
5 changes: 3 additions & 2 deletions lib/api/capi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#if !defined (ANDROID) || defined(ENABLE_CAPI_HTTP_CLIENT)
#include "http/HttpClient_CAPI.hpp"
#endif
#include "LogManagerFactory.hpp"
#include "LogManagerProvider.hpp"
#include "mat.h"
#include "pal/TaskDispatcher_CAPI.hpp"
Expand Down Expand Up @@ -163,7 +164,7 @@ evt_status_t mat_open_core(
}

status_t status = static_cast<status_t>(EFAULT);
clients[code].logmanager = LogManagerProvider::CreateLogManager(clients[code].config, status);
clients[code].logmanager = LogManagerFactory::Get(clients[code].config, status);

// Verify that the instance pointer is valid
if (clients[code].logmanager == nullptr)
Expand Down Expand Up @@ -271,7 +272,7 @@ evt_status_t mat_log(evt_context_t *ctx)
const auto & it = m.find(COMMONFIELDS_EVENT_SOURCE);
std::string source = ((it != m.cend()) && (it->second.type == EventProperty::TYPE_STRING)) ? it->second.as_string : "";

ILogger *logger = client->logmanager->GetLogger(token, source, scope);
auto logger = client->logmanager->GetLogger(token, source, scope);
if (logger == nullptr)
{
ctx->result = EFAULT; /* invalid address */
Expand Down
2 changes: 1 addition & 1 deletion lib/include/public/ILogManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ namespace MAT_NS_BEGIN
/// <param name="scope">A string that contains the logger scope/project set (reserved for future use).</param>
///
/// <returns>A pointer to the ILogger instance.</returns>
virtual ILogger* GetLogger(std::string const& tenantToken, std::string const& source = std::string(), std::string const& scope = std::string()) = 0;
virtual std::shared_ptr<ILogger> GetLogger(std::string const& tenantToken, std::string const& source = std::string(), std::string const& scope = std::string()) = 0;

/// <summary>Retrieves the current LogManager instance configuration</summary>
virtual ILogConfiguration& GetLogConfiguration() = 0;
Expand Down
3 changes: 0 additions & 3 deletions lib/include/public/LogManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,4 @@ namespace MAT_NS_BEGIN {
class LogManager : public LogManagerBase<ModuleLogConfiguration> {};
} MAT_NS_END

#define LOGMANAGER_INSTANCE namespace MAT_NS_BEGIN { DEFINE_LOGMANAGER(LogManager, ModuleLogConfiguration); } MAT_NS_END

#endif

18 changes: 9 additions & 9 deletions lib/include/public/LogManagerBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ namespace MAT_NS_BEGIN
/// <summary>
/// Concrete instance for servicing all singleton calls
/// </summary>
static ILogManager* instance;
static std::unique_ptr<ILogManager> instance;

/// <summary>
/// Debug event source associated with this singleton
Expand Down Expand Up @@ -201,7 +201,7 @@ namespace MAT_NS_BEGIN
/// Initializes the telemetry logging system with default configuration and HTTPClient.
/// </summary>
/// <returns>A logger instance instantiated with the default tenantToken.</returns>
static ILogger* Initialize()
static std::shared_ptr<ILogger> Initialize()
{
return Initialize(std::string{});
}
Expand All @@ -211,7 +211,7 @@ namespace MAT_NS_BEGIN
/// </summary>
/// <param name="tenantToken">Token of the tenant with which the application is associated for collecting telemetry</param>
/// <returns>A logger instance instantiated with the tenantToken.</returns>
inline static ILogger* Initialize(const std::string& tenantToken)
inline static std::shared_ptr<ILogger> Initialize(const std::string& tenantToken)
{
return Initialize(tenantToken, GetLogConfiguration());
}
Expand All @@ -222,7 +222,7 @@ namespace MAT_NS_BEGIN
/// <param name="tenantToken">Token of the tenant with which the application is associated for collecting telemetry</param>
/// <param name="configuration">ILogConfiguration to be used.</param>
/// <returns>A logger instance instantiated with the tenantToken.</returns>
static ILogger* Initialize(
static std::shared_ptr<ILogger> Initialize(
const std::string& tenantToken,
ILogConfiguration& configuration)
{
Expand Down Expand Up @@ -559,15 +559,15 @@ namespace MAT_NS_BEGIN
/// Retrieves the ILogger interface of a Logger instance through which to log telemetry event.
/// </summary>
/// <returns>Pointer to the Ilogger interface of an logger instance</returns>
static ILogger* GetLogger()
static std::shared_ptr<ILogger> GetLogger()
LM_SAFE_CALL_PTR(GetLogger, GetPrimaryToken());

/// <summary>
/// Retrieves the ILogger interface of a Logger instance through which to log telemetry event.
/// </summary>
/// <param name="source">Source name of events sent by this logger instance</param>
/// <returns>Pointer to the Ilogger interface of the logger instance</returns>
static ILogger* GetLogger(const std::string& source)
static std::shared_ptr<ILogger> GetLogger(const std::string& source)
LM_SAFE_CALL_PTR(GetLogger, GetPrimaryToken(), source);

/// <summary>
Expand All @@ -576,7 +576,7 @@ namespace MAT_NS_BEGIN
/// <param name="tenantToken">Token of the tenant with which the application is associated for collecting telemetry</param>
/// <param name="source">Source name of events sent by this logger instance</param>
/// <returns>Pointer to the Ilogger interface of the logger instance</returns>
static ILogger* GetLogger(const std::string& tenantToken, const std::string& source)
static std::shared_ptr<ILogger> GetLogger(const std::string& tenantToken, const std::string& source)
LM_SAFE_CALL_PTR(GetLogger, tenantToken, source);

/// <summary>
Expand Down Expand Up @@ -731,7 +731,7 @@ namespace MAT_NS_BEGIN
static ILogManager* GetInstance() noexcept
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably needs to go as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Really all the static methods

Copy link
Contributor Author

@lalitb lalitb Mar 15, 2023

Choose a reason for hiding this comment

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

Yes, I am trying to see if we can make LogManager as instance of LogManagerBase instead of global static, and then make all these methods as non-static. Though LogConfiguration is still a concern here, as it is difficult to create deep copy of it (as it can contain void * as the config value).

{
LM_LOCKGUARD(stateLock());
return instance;
return instance.get();
}
};

Expand All @@ -743,7 +743,7 @@ namespace MAT_NS_BEGIN
// https://developercommunity.visualstudio.com/content/problem/134886/initialization-of-template-static-variable-wrong.html
//
#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \
ILogManager* LogManagerClass::instance = nullptr;
std::unique_ptr<ILogManager> LogManagerClass::instance = nullptr;
#elif defined(__APPLE__) && defined(MAT_USE_WEAK_LOGMANAGER)
#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \
template <> \
Expand Down
16 changes: 11 additions & 5 deletions lib/include/public/LogManagerProvider.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace MAT_NS_BEGIN
/// <param name="status">Status.</param>
/// <param name="wantController">WantController.</param>
/// </summary>
static ILogManager* MATSDK_SPEC CreateLogManager(
static std::unique_ptr<ILogManager> MATSDK_SPEC CreateLogManager(
char const* id,
bool wantController,
ILogConfiguration& cfg,
Expand Down Expand Up @@ -93,7 +93,7 @@ namespace MAT_NS_BEGIN
/// <param name="id">Instance Id.</param>
/// <param name="status">Status.</param>
/// </summary>
static ILogManager* MATSDK_SPEC CreateLogManager(char const* id,
static std::unique_ptr<ILogManager> MATSDK_SPEC CreateLogManager(char const* id,
status_t& status,
uint64_t targetVersion = MAT::Version)
{
Expand All @@ -105,13 +105,19 @@ namespace MAT_NS_BEGIN
);
}

static ILogManager* MATSDK_SPEC CreateLogManager(
static std::unique_ptr<ILogManager> MATSDK_SPEC CreateLogManager(
ILogConfiguration& cfg,
status_t& status)
{
return Get(cfg, status);
}

static std::unique_ptr<ILogManager> MATSDK_SPEC CreateLogManager(ILogConfiguration& cfg)
{
status_t status;
return Get(cfg, status);
}

/// <summary>
/// Releases a guest or host LogManager by its instance id.
/// <param name="id">Instance Id.</param>
Expand Down Expand Up @@ -139,12 +145,12 @@ namespace MAT_NS_BEGIN
// methods deprecated.
//

static ILogManager * MATSDK_SPEC Get(
static std::unique_ptr<ILogManager> MATSDK_SPEC Get(
ILogConfiguration & cfg,
status_t &status
);

static ILogManager* MATSDK_SPEC Get(
static std::unique_ptr<ILogManager> MATSDK_SPEC Get(
const char * id,
status_t& status
);
Expand Down
6 changes: 3 additions & 3 deletions lib/include/public/NullObjects.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,10 @@ namespace MAT_NS_BEGIN
return STATUS_ENOSYS;
}

virtual ILogger * GetLogger(std::string const & /*tenantToken*/, std::string const & /*source*/ = std::string(), std::string const & /*experimentationProject*/ = std::string()) override
virtual std::shared_ptr<ILogger> GetLogger(std::string const & /*tenantToken*/, std::string const & /*source*/ = std::string(), std::string const & /*experimentationProject*/ = std::string()) override
{
static NullLogger nullLogger;
return &nullLogger;
static std::shared_ptr<ILogger> nullLogger(new NullLogger);
return nullLogger;
}

virtual void AddEventListener(DebugEventType /*type*/, DebugEventListener & /*listener*/) override {};
Expand Down
38 changes: 20 additions & 18 deletions tests/functests/AISendTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ class AISendTests : public ::testing::Test,
std::string serverAddress;
HttpServer server;

ILogConfiguration configuration;
std::unique_ptr<ILogManager> logManager;
ILogger* logger;

std::atomic<bool> isSetup;
Expand Down Expand Up @@ -147,7 +149,8 @@ class AISendTests : public ::testing::Test,
virtual void Initialize(DebugEventListener& debugListener, std::string const& path, bool compression)
{
receivedRequests.clear();
auto configuration = LogManager::GetLogConfiguration();
configuration = ILogConfiguration{};
configuration[CFG_STR_PRIMARY_TOKEN] = std::string{TEST_TOKEN};
configuration[CFG_INT_SDK_MODE] = SdkModeTypes_AI;
configuration[CFG_STR_COLLECTOR_URL] = (serverAddress + path).c_str();
configuration[CFG_MAP_HTTP][CFG_BOOL_HTTP_COMPRESSION] = compression;
Expand All @@ -167,32 +170,31 @@ class AISendTests : public ::testing::Test,
configuration["version"] = "1.0.0";
configuration["config"] = {{"host", __FILE__}}; // Host instance

LogManager::Initialize(TEST_TOKEN, configuration);
LogManager::SetLevelFilter(DIAG_LEVEL_DEFAULT, {DIAG_LEVEL_DEFAULT_MIN, DIAG_LEVEL_DEFAULT_MAX});
LogManager::ResumeTransmission();
logManager = LogManagerProvider::CreateLogManager(configuration);
logManager->SetLevelFilter(DIAG_LEVEL_DEFAULT, {DIAG_LEVEL_DEFAULT_MIN, DIAG_LEVEL_DEFAULT_MAX});
logManager->ResumeTransmission();

LogManager::AddEventListener(DebugEventType::EVT_HTTP_OK, debugListener);
LogManager::AddEventListener(DebugEventType::EVT_HTTP_ERROR, debugListener);
LogManager::AddEventListener(DebugEventType::EVT_HTTP_FAILURE, debugListener);
LogManager::AddEventListener(DebugEventType::EVT_HTTP_STATE, debugListener);
LogManager::AddEventListener(DebugEventType::EVT_ADDED, debugListener);
logManager->AddEventListener(DebugEventType::EVT_HTTP_OK, debugListener);
logManager->AddEventListener(DebugEventType::EVT_HTTP_ERROR, debugListener);
logManager->AddEventListener(DebugEventType::EVT_HTTP_FAILURE, debugListener);
logManager->AddEventListener(DebugEventType::EVT_HTTP_STATE, debugListener);
logManager->AddEventListener(DebugEventType::EVT_ADDED, debugListener);

logger = LogManager::GetLogger(TEST_TOKEN);
logger = logManager->GetLogger(TEST_TOKEN);
}

virtual void FlushAndTeardown(DebugEventListener& debugListener)
{
LogManager::Flush();
logManager->Flush();

LogManager::RemoveEventListener(DebugEventType::EVT_HTTP_OK, debugListener);
LogManager::RemoveEventListener(DebugEventType::EVT_HTTP_ERROR, debugListener);
LogManager::RemoveEventListener(DebugEventType::EVT_HTTP_FAILURE, debugListener);
LogManager::RemoveEventListener(DebugEventType::EVT_HTTP_STATE, debugListener);
LogManager::RemoveEventListener(DebugEventType::EVT_ADDED, debugListener);
logManager->RemoveEventListener(DebugEventType::EVT_HTTP_OK, debugListener);
logManager->RemoveEventListener(DebugEventType::EVT_HTTP_ERROR, debugListener);
logManager->RemoveEventListener(DebugEventType::EVT_HTTP_FAILURE, debugListener);
logManager->RemoveEventListener(DebugEventType::EVT_HTTP_STATE, debugListener);
logManager->RemoveEventListener(DebugEventType::EVT_ADDED, debugListener);

LogManager::FlushAndTeardown();
logManager->FlushAndTeardown();

auto &configuration = LogManager::GetLogConfiguration();
configuration[CFG_INT_SDK_MODE] = SdkModeTypes_CS;
}

Expand Down
Loading