Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[WIP] Modern lifecycle + Ownership #1089

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft

[WIP] Modern lifecycle + Ownership #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6764de9
initial changes
lalitb Jan 12, 2023
5498486
changes
lalitb Jan 20, 2023
6330193
more changes
lalitb Jan 20, 2023
a14fecf
more changes
lalitb Jan 24, 2023
1057c4b
Fix tests
lalitb Jan 26, 2023
dd1483a
Merge branch 'main' into v4-refactor
lalitb Jan 26, 2023
8046551
fix
lalitb Jan 26, 2023
79a7ca6
Merge branch 'v4-refactor' of github.com:microsoft/cpp_client_telemet…
lalitb Jan 26, 2023
ad8dafd
fix
lalitb Jan 27, 2023
c39662f
Revert "fix"
lalitb Jan 30, 2023
a127dba
fix ios wrapper
lalitb Mar 14, 2023
69d97ff
fix
lalitb Mar 14, 2023
11f1ac6
fix
lalitb Mar 14, 2023
51a4d68
Fix
lalitb Mar 14, 2023
98ddbc3
fix
lalitb Mar 14, 2023
6ab3d19
fix
lalitb Mar 14, 2023
43ad075
fix
lalitb Mar 14, 2023
6076691
revert
lalitb Mar 14, 2023
7c9941c
Merge branch 'main' into v4-refactor
lalitb Mar 14, 2023
5f57128
fix
lalitb Mar 14, 2023
4c147e9
fix
lalitb Mar 14, 2023
e081844
fix
lalitb Mar 14, 2023
79f2f69
fix
lalitb Mar 14, 2023
4fc289d
fix android build
lalitb Mar 14, 2023
eb18ec6
fix
lalitb Mar 14, 2023
8eb7153
fix
lalitb Mar 14, 2023
811586b
fix android
lalitb Mar 14, 2023
b81042c
try to fix test
lalitb Mar 14, 2023
72d692b
fix
lalitb Mar 14, 2023
7165385
Merge branch 'main' into v4-refactor
lalitb Mar 28, 2023
dcf10dc
fix
lalitb Mar 28, 2023
3db9adc
fix
lalitb Mar 28, 2023
d64b271
fix
lalitb Mar 28, 2023
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
2 changes: 1 addition & 1 deletion lib/api/LogManagerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ namespace MAT_NS_BEGIN
return m_logConfiguration;
}

ILogger* LogManagerImpl::GetLogger(const std::string& tenantToken, const std::string& source, const std::string& scope)
ILogger* LogManagerImpl::GetLogger(const std::string& tenantToken, const std::string& source, const std::string& scope)
{
{
LOCKGUARD(m_lock);
Expand Down
10 changes: 5 additions & 5 deletions lib/api/LogManagerProvider.cpp
View file
Open in desktop
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
View file
Open in desktop
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);
ILogger* logger = client->logmanager->GetLogger(token, source, scope);
if (logger == nullptr)
{
ctx->result = EFAULT; /* invalid address */
Expand Down
3 changes: 1 addition & 2 deletions lib/include/public/LogManager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace MAT_NS_BEGIN {
class LogManager : public LogManagerBase<ModuleLogConfiguration> {};
} MAT_NS_END

// Singleton LogManager Instance is deprecated.
#define LOGMANAGER_INSTANCE namespace MAT_NS_BEGIN { DEFINE_LOGMANAGER(LogManager, ModuleLogConfiguration); } MAT_NS_END

#endif

10 changes: 5 additions & 5 deletions lib/include/public/LogManagerBase.hpp
View file
Open in desktop
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 @@ -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 •
edited
Loading

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,16 +743,16 @@ 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 <> \
__attribute__((weak)) ILogManager* LogManagerBase<LogConfigurationClass>::instance{};
__attribute__((weak)) std::unique_ptr<ILogManager> LogManagerBase<LogConfigurationClass>::instance{};
#else
// ISO C++ -compliant declaration
#define DEFINE_LOGMANAGER(LogManagerClass, LogConfigurationClass) \
template <> \
ILogManager* LogManagerBase<LogConfigurationClass>::instance{};
std::unique_ptr<ILogManager> LogManagerBase<LogConfigurationClass>::instance{};
#endif

}
Expand Down
16 changes: 11 additions & 5 deletions lib/include/public/LogManagerProvider.hpp
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion lib/include/public/NullObjects.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ 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 ILogger* GetLogger(std::string const & /*tenantToken*/, std::string const & /*source*/ = std::string(), std::string const & /*experimentationProject*/ = std::string()) override
{
static NullLogger nullLogger;
return &nullLogger;
Expand Down
4 changes: 2 additions & 2 deletions lib/jni/LogManager_jni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
using namespace MAT;

template <>
ILogManager* LogManagerBase<WrapperConfig>::instance{};
std::unique_ptr<ILogManager> LogManagerBase<WrapperConfig>::instance{};

extern "C"
{
Expand Down Expand Up @@ -857,7 +857,7 @@ Java_com_microsoft_applications_events_LogManagerProvider_nativeCreateLogManager
status_t status = status_t::STATUS_SUCCESS;
mcPointer->manager = MAT::LogManagerProvider::CreateLogManager(
mcPointer->config,
status);
status).release();
if (status == status_t::STATUS_SUCCESS && !!mcPointer->manager)
{
std::lock_guard<std::mutex> lock(jniManagersMutex);
Expand Down
38 changes: 20 additions & 18 deletions tests/functests/AISendTests.cpp
View file
Open in desktop
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