diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 9f2787714d..e6a8053646 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -267,7 +267,7 @@ if( KICAD_USE_SENTRY ) target_link_libraries( kicommon sentry ) - set_property(SOURCE pgm_base.cpp APPEND PROPERTY COMPILE_DEFINITIONS KICAD_SENTRY_DSN="${KICAD_SENTRY_DSN}") + set_property(SOURCE app_monitor.cpp APPEND PROPERTY COMPILE_DEFINITIONS KICAD_SENTRY_DSN="${KICAD_SENTRY_DSN}") endif() if( KICAD_IPC_API ) diff --git a/common/app_monitor.cpp b/common/app_monitor.cpp index 7ad0ddd3ff..d31b433c20 100644 --- a/common/app_monitor.cpp +++ b/common/app_monitor.cpp @@ -24,6 +24,15 @@ #include #include #include +#include +#include +#include +#include +#include + +#include +#include +#include #ifdef KICAD_USE_SENTRY #include @@ -31,6 +40,231 @@ using namespace APP_MONITOR; +SENTRY::SENTRY() : + m_isOptedIn( false ) +{ +} + + +void SENTRY::Init() +{ +#ifdef KICAD_USE_SENTRY + sentryInit(); +#endif +} + + +void SENTRY::Cleanup() +{ +#ifdef KICAD_USE_SENTRY + sentry_close(); +#endif +} + + +void SENTRY::AddTag( const wxString& aKey, const wxString& aValue ) +{ +#ifdef KICAD_USE_SENTRY + sentry_set_tag( aKey.c_str(), aValue.c_str() ); +#endif +} + + +void SENTRY::SetSentryOptIn( bool aOptIn ) +{ +#ifdef KICAD_USE_SENTRY + if( aOptIn ) + { + // Opting in (when not opted in before) may require reading the uid off disk + // as it was skipped previously. + readOrCreateUid(); + + if( !m_sentry_optin_fn.Exists() ) + { + wxFFile sentryInitFile( m_sentry_optin_fn.GetFullPath(), "w" ); + sentryInitFile.Write( "" ); + sentryInitFile.Close(); + } + + m_isOptedIn = true; + } + else + { + if( m_sentry_optin_fn.Exists() ) + { + wxRemoveFile( m_sentry_optin_fn.GetFullPath() ); + sentry_close(); + } + + m_isOptedIn = false; + } +#else + m_isOptedIn = false; +#endif +} + + +wxString SENTRY::sentryCreateUid() +{ + boost::uuids::uuid uuid = boost::uuids::random_generator()(); + wxString userGuid = boost::uuids::to_string( uuid ); + + wxFFile sentryInitFile( m_sentry_uid_fn.GetFullPath(), "w" ); + sentryInitFile.Write( userGuid ); + sentryInitFile.Close(); + + return userGuid; +} + + +void SENTRY::ResetSentryId() +{ + m_sentryUid = sentryCreateUid(); +} + + +const wxString& SENTRY::GetSentryId() +{ + return m_sentryUid; +} + + +void SENTRY::readOrCreateUid() +{ + wxFFile sentryInitFile( m_sentry_uid_fn.GetFullPath() ); + sentryInitFile.ReadAll( &m_sentryUid ); + sentryInitFile.Close(); + + if( m_sentryUid.IsEmpty() || m_sentryUid.length() != 36 ) + { + ResetSentryId(); + } +} + + +void SENTRY::sentryInit() +{ +#ifdef KICAD_USE_SENTRY + m_sentry_optin_fn = wxFileName( PATHS::GetUserCachePath(), "sentry-opt-in" ); + m_sentry_uid_fn = wxFileName( PATHS::GetUserCachePath(), "sentry-uid" ); + + if( isConfiguredOptedIn() ) + { + m_isOptedIn = true; + + readOrCreateUid(); + + sentry_options_t* options = sentry_options_new(); + +#ifndef KICAD_SENTRY_DSN +#error "Project configuration error, missing KICAD_SENTRY_DSN" +#endif + // only capture 5% of transactions + sentry_options_set_traces_sample_rate( options, 0.05 ); + sentry_options_set_dsn( options, KICAD_SENTRY_DSN ); + + wxFileName tmp; + tmp.AssignDir( PATHS::GetUserCachePath() ); + tmp.AppendDir( "sentry" ); + +#ifdef __WINDOWS__ + sentry_options_set_database_pathw( options, tmp.GetPathWithSep().wc_str() ); +#else + sentry_options_set_database_path( options, tmp.GetPathWithSep().c_str() ); +#endif + sentry_options_set_symbolize_stacktraces( options, true ); + sentry_options_set_auto_session_tracking( options, false ); + + sentry_options_set_release( options, GetCommitHash().ToStdString().c_str() ); + + // This just gives us more filtering within sentry, issues still get grouped across + // environments. + sentry_options_set_environment( options, GetMajorMinorVersion().c_str() ); + + sentry_init( options ); + + sentry_value_t user = sentry_value_new_object(); + sentry_value_set_by_key( user, "id", sentry_value_new_string( m_sentryUid.c_str() ) ); + sentry_set_user( user ); + + sentry_set_tag( "kicad.version", GetBuildVersion().ToStdString().c_str() ); + } +#endif +} + + +bool SENTRY::isConfiguredOptedIn() +{ + // policy takes precedence + KIPLATFORM::POLICY::PBOOL policyState = KIPLATFORM::POLICY::GetPolicyBool( POLICY_KEY_DATACOLLECTION ); + if( policyState != KIPLATFORM::POLICY::PBOOL::NOT_CONFIGURED ) + { + return policyState == KIPLATFORM::POLICY::PBOOL::ENABLED; + } + + return m_sentry_optin_fn.Exists(); +} + + +bool SENTRY::IsOptedIn() +{ +#ifdef KICAD_USE_SENTRY + // always override with policy, just in case + KIPLATFORM::POLICY::PBOOL policyState = KIPLATFORM::POLICY::GetPolicyBool( POLICY_KEY_DATACOLLECTION ); + if( policyState != KIPLATFORM::POLICY::PBOOL::NOT_CONFIGURED ) + { + return policyState == KIPLATFORM::POLICY::PBOOL::ENABLED; + } + + return m_isOptedIn; +#else + return false; +#endif +} + + +void SENTRY::LogAssert( const ASSERT_CACHE_KEY& aKey, const wxString& aAssertMsg ) +{ +#ifdef KICAD_USE_SENTRY + if( !APP_MONITOR::SENTRY::Instance()->IsOptedIn() ) + { + return; + } + + // We use an assert cache to avoid collecting too many events + // Because they can come from functions that are called hundreds of times in loops + if( m_assertCache.find( aKey ) == m_assertCache.end() ) + { + sentry_value_t exc = sentry_value_new_exception( "assert", aAssertMsg.c_str() ); + sentry_value_set_stacktrace( exc, NULL, 0 ); + + sentry_value_t sentryEvent = sentry_value_new_event(); + sentry_event_add_exception( sentryEvent, exc ); + sentry_capture_event( sentryEvent ); + m_assertCache.insert( aKey ); + } +#endif +} + + +void SENTRY::LogException( const wxString& aMsg ) +{ +#ifdef KICAD_USE_SENTRY + if( !APP_MONITOR::SENTRY::Instance()->IsOptedIn() ) + { + return; + } + + sentry_value_t exc = sentry_value_new_exception( "exception", aMsg.c_str() ); + sentry_value_set_stacktrace( exc, NULL, 0 ); + + sentry_value_t sentryEvent = sentry_value_new_event(); + sentry_event_add_exception( sentryEvent, exc ); + sentry_capture_event( sentryEvent ); +#endif +} + + static std::string GetSentryBreadCrumbType( BREADCRUMB_TYPE aType ) { // the only special case due to collisions with defines @@ -74,10 +308,19 @@ static std::string GetSentryBreadCrumbLevel( BREADCRUMB_LEVEL aLevel ) namespace APP_MONITOR { +SENTRY* SENTRY::m_instance = nullptr; + + +bool operator<( const ASSERT_CACHE_KEY& aKey1, const ASSERT_CACHE_KEY& aKey2 ) +{ + return aKey1.file < aKey2.file || aKey1.line < aKey2.line || aKey1.func < aKey2.func || aKey1.cond < aKey2.cond; +} + + void AddBreadcrumb( BREADCRUMB_TYPE aType, const wxString& aMsg, const wxString& aCategory, BREADCRUMB_LEVEL aLevel ) { #ifdef KICAD_USE_SENTRY - if( !Pgm().IsSentryOptedIn() ) + if( !SENTRY::Instance()->IsOptedIn() ) { return; } @@ -176,7 +419,7 @@ private: TRANSACTION::TRANSACTION( const std::string& aName, const std::string& aOperation ) { #ifdef KICAD_USE_SENTRY - if( Pgm().IsSentryOptedIn() ) + if( SENTRY::Instance()->IsOptedIn() ) { m_impl = new TRANSACTION_IMPL( aName, aOperation ); } diff --git a/common/dialogs/panel_data_collection.cpp b/common/dialogs/panel_data_collection.cpp index 99900950ee..282fb1ea62 100644 --- a/common/dialogs/panel_data_collection.cpp +++ b/common/dialogs/panel_data_collection.cpp @@ -24,12 +24,12 @@ #include #include +#include #include #include #include #include #include -#include #include #include #include @@ -54,7 +54,7 @@ bool PANEL_DATA_COLLECTION::TransferDataToWindow() Disable(); } - m_sentryUid->SetValue( Pgm().GetSentryId() ); + m_sentryUid->SetValue( APP_MONITOR::SENTRY::Instance()->GetSentryId() ); return true; } @@ -62,7 +62,7 @@ bool PANEL_DATA_COLLECTION::TransferDataToWindow() bool PANEL_DATA_COLLECTION::TransferDataFromWindow() { - Pgm().SetSentryOptIn( m_cbOptIn->GetValue() ); + APP_MONITOR::SENTRY::Instance()->SetSentryOptIn( m_cbOptIn->GetValue() ); return true; } @@ -76,12 +76,12 @@ void PANEL_DATA_COLLECTION::ResetPanel() void PANEL_DATA_COLLECTION::applySettingsToPanel() { - m_cbOptIn->SetValue( Pgm().IsSentryOptedIn() ); + m_cbOptIn->SetValue( APP_MONITOR::SENTRY::Instance()->IsOptedIn() ); } void PANEL_DATA_COLLECTION::OnResetIdClick( wxCommandEvent& aEvent ) { - Pgm().ResetSentryId(); - m_sentryUid->SetValue( Pgm().GetSentryId() ); + APP_MONITOR::SENTRY::Instance()->ResetSentryId(); + m_sentryUid->SetValue( APP_MONITOR::SENTRY::Instance()->GetSentryId() ); } diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp index 0e5d163c6f..03b093a201 100644 --- a/common/pgm_base.cpp +++ b/common/pgm_base.cpp @@ -44,6 +44,7 @@ #include #include +#include #include #include #include @@ -68,13 +69,6 @@ #include -#ifdef KICAD_USE_SENTRY -#include -#include -#include -#include -#endif - #ifdef KICAD_IPC_API #include #include @@ -171,9 +165,7 @@ void PGM_BASE::Destroy() { KICAD_CURL::Cleanup(); -#ifdef KICAD_USE_SENTRY - sentry_close(); -#endif + APP_MONITOR::SENTRY::Instance()->Cleanup(); m_pgm_checker.reset(); } @@ -251,125 +243,6 @@ const wxString PGM_BASE::AskUserForPreferredEditor( const wxString& aDefaultEdit #ifdef KICAD_USE_SENTRY -bool PGM_BASE::IsSentryOptedIn() -{ - KIPLATFORM::POLICY::PBOOL policyState = - KIPLATFORM::POLICY::GetPolicyBool( POLICY_KEY_DATACOLLECTION ); - if( policyState != KIPLATFORM::POLICY::PBOOL::NOT_CONFIGURED ) - { - return policyState == KIPLATFORM::POLICY::PBOOL::ENABLED; - } - - return m_sentry_optin_fn.Exists(); -} - - -void PGM_BASE::SetSentryOptIn( bool aOptIn ) -{ - if( aOptIn ) - { - if( !m_sentry_uid_fn.Exists() ) - { - sentryCreateUid(); - } - - if( !m_sentry_optin_fn.Exists() ) - { - wxFFile sentryInitFile( m_sentry_optin_fn.GetFullPath(), "w" ); - sentryInitFile.Write( "" ); - sentryInitFile.Close(); - } - } - else - { - if( m_sentry_optin_fn.Exists() ) - { - wxRemoveFile( m_sentry_optin_fn.GetFullPath() ); - sentry_close(); - } - } -} - - -wxString PGM_BASE::sentryCreateUid() -{ - boost::uuids::uuid uuid = boost::uuids::random_generator()(); - wxString userGuid = boost::uuids::to_string( uuid ); - - wxFFile sentryInitFile( m_sentry_uid_fn.GetFullPath(), "w" ); - sentryInitFile.Write( userGuid ); - sentryInitFile.Close(); - - return userGuid; -} - - -void PGM_BASE::ResetSentryId() -{ - m_sentryUid = sentryCreateUid(); -} - - -const wxString& PGM_BASE::GetSentryId() -{ - return m_sentryUid; -} - - -void PGM_BASE::sentryInit() -{ - m_sentry_optin_fn = wxFileName( PATHS::GetUserCachePath(), "sentry-opt-in" ); - m_sentry_uid_fn = wxFileName( PATHS::GetUserCachePath(), "sentry-uid" ); - - if( IsSentryOptedIn() ) - { - wxFFile sentryInitFile( m_sentry_uid_fn.GetFullPath() ); - sentryInitFile.ReadAll( &m_sentryUid ); - sentryInitFile.Close(); - - if( m_sentryUid.IsEmpty() || m_sentryUid.length() != 36 ) - { - m_sentryUid = sentryCreateUid(); - } - - sentry_options_t* options = sentry_options_new(); - -#ifndef KICAD_SENTRY_DSN -# error "Project configuration error, missing KICAD_SENTRY_DSN" -#endif - // only capture 5% of transactions - sentry_options_set_traces_sample_rate( options, 0.05 ); - sentry_options_set_dsn( options, KICAD_SENTRY_DSN ); - - wxFileName tmp; - tmp.AssignDir( PATHS::GetUserCachePath() ); - tmp.AppendDir( "sentry" ); - -#ifdef __WINDOWS__ - sentry_options_set_database_pathw( options, tmp.GetPathWithSep().wc_str() ); -#else - sentry_options_set_database_path( options, tmp.GetPathWithSep().c_str() ); -#endif - sentry_options_set_symbolize_stacktraces( options, true ); - sentry_options_set_auto_session_tracking( options, false ); - - sentry_options_set_release( options, GetCommitHash().ToStdString().c_str() ); - - // This just gives us more filtering within sentry, issues still get grouped across - // environments. - sentry_options_set_environment( options, GetMajorMinorVersion().c_str() ); - - sentry_init( options ); - - sentry_value_t user = sentry_value_new_object(); - sentry_value_set_by_key( user, "id", sentry_value_new_string( m_sentryUid.c_str() ) ); - sentry_set_user( user ); - - sentry_set_tag( "kicad.version", GetBuildVersion().ToStdString().c_str() ); - } -} - - void PGM_BASE::sentryPrompt() { if( !IsGUI() ) @@ -394,12 +267,11 @@ void PGM_BASE::sentryPrompt() if( result == wxID_YES ) { - SetSentryOptIn( true ); - sentryInit(); + APP_MONITOR::SENTRY::Instance()->SetSentryOptIn( true ); } else { - SetSentryOptIn( false ); + APP_MONITOR::SENTRY::Instance()->SetSentryOptIn( false ); } m_settings_manager->GetCommonSettings()->m_DoNotShowAgain.data_collection_prompt = true; @@ -468,9 +340,7 @@ bool PGM_BASE::InitPgm( bool aHeadless, bool aSkipPyInit, bool aIsUnitTest ) KICAD_CURL::Init(); -#ifdef KICAD_USE_SENTRY - sentryInit(); -#endif + APP_MONITOR::SENTRY::Instance()->Init(); // Initialize the singleton instance m_singleton.Init(); @@ -483,9 +353,7 @@ bool PGM_BASE::InitPgm( bool aHeadless, bool aSkipPyInit, bool aIsUnitTest ) else pgm_name = wxFileName( App().argv[0] ).GetName().Lower(); -#ifdef KICAD_USE_SENTRY - sentry_set_tag( "kicad.app", pgm_name.c_str() ); -#endif + APP_MONITOR::SENTRY::Instance()->AddTag( "kicad.app", pgm_name ); wxInitAllImageHandlers(); @@ -959,17 +827,7 @@ void PGM_BASE::HandleException( std::exception_ptr aPtr ) } catch( const std::exception& e ) { -#ifdef KICAD_USE_SENTRY - if( IsSentryOptedIn() ) - { - sentry_value_t exc = sentry_value_new_exception( "exception", e.what() ); - sentry_value_set_stacktrace( exc, NULL, 0 ); - - sentry_value_t sentryEvent = sentry_value_new_event(); - sentry_event_add_exception( sentryEvent, exc ); - sentry_capture_event( sentryEvent ); - } -#endif + APP_MONITOR::SENTRY::Instance()->LogException( e.what() ); wxLogError( wxT( "Unhandled exception class: %s what: %s" ), From_UTF8( typeid( e ).name() ), From_UTF8( e.what() ) ); @@ -981,28 +839,6 @@ void PGM_BASE::HandleException( std::exception_ptr aPtr ) } -#ifdef KICAD_USE_SENTRY -struct SENTRY_ASSERT_CACHE_KEY -{ - wxString file; - int line; - wxString func; - wxString cond; - wxString msg; -}; - - -bool operator<( const SENTRY_ASSERT_CACHE_KEY& aKey1, const SENTRY_ASSERT_CACHE_KEY& aKey2 ) -{ - return aKey1.file < aKey2.file || - aKey1.line < aKey2.line || - aKey1.func < aKey2.func || - aKey1.cond < aKey2.cond || - aKey1.msg < aKey2.msg; -} -#endif - - void PGM_BASE::HandleAssert( const wxString& aFile, int aLine, const wxString& aFunc, const wxString& aCond, const wxString& aMsg ) { @@ -1025,23 +861,8 @@ void PGM_BASE::HandleAssert( const wxString& aFile, int aLine, const wxString& a #endif #ifdef KICAD_USE_SENTRY - if( IsSentryOptedIn() ) - { - static std::set assertCache; - - SENTRY_ASSERT_CACHE_KEY key = { aFile, aLine, aFunc, aCond }; - - if( assertCache.find( key ) == assertCache.end() ) - { - sentry_value_t exc = sentry_value_new_exception( "assert", assertStr ); - sentry_value_set_stacktrace( exc, NULL, 0 ); - - sentry_value_t sentryEvent = sentry_value_new_event(); - sentry_event_add_exception( sentryEvent, exc ); - sentry_capture_event( sentryEvent ); - assertCache.insert( key ); - } - } + APP_MONITOR::ASSERT_CACHE_KEY key = { aFile, aLine, aFunc, aCond }; + APP_MONITOR::SENTRY::Instance()->LogAssert( key, assertStr ); #endif } diff --git a/include/app_monitor.h b/include/app_monitor.h index 2f36796c7f..7768712d1e 100644 --- a/include/app_monitor.h +++ b/include/app_monitor.h @@ -25,7 +25,9 @@ #include #include +#include #include +#include namespace APP_MONITOR { @@ -53,6 +55,11 @@ namespace APP_MONITOR class TRANSACTION_IMPL; + /** + * This represents a sentry transaction which is used for time-performance metrics + * You start a transaction and can denote "spans" inside the transaction for specific + * portions of the transaction. + */ class KICOMMON_API TRANSACTION { public: @@ -71,11 +78,90 @@ namespace APP_MONITOR #endif }; + /** + * This struct represents a key being used for the std::set that deduplicates asserts + * during this running session. + * + * The elements are meant to make the assert sufficiently unique but at the same time + * avoid unnecessary noise. One notable issue was we used to use msg as a key but + * asserts with args in loops would defeat it and cause noise + */ + struct KICOMMON_API ASSERT_CACHE_KEY + { + wxString file; + int line; + wxString func; + wxString cond; + }; + + KICOMMON_API + bool operator<( const ASSERT_CACHE_KEY& aKey1, const ASSERT_CACHE_KEY& aKey2 ); + + /** + * This is a singleton class intended to manage sentry + * + * The inards of the api in this class are meant to be compiled out when KICAD_USE_SENTRY + * is not defined and become "inert" in order to reduce the need to sprinkle #ifdef checks + * everywhere. + */ + class KICOMMON_API SENTRY + { + public: + SENTRY( const SENTRY& obj ) = delete; + + static SENTRY* Instance() + { + if( m_instance == nullptr ) + m_instance = new SENTRY(); + + return m_instance; + } + + void Init(); + void Cleanup(); + + bool IsOptedIn(); + void AddTag( const wxString& aKey, const wxString& aValue ); + void SetSentryOptIn( bool aOptIn ); + const wxString& GetSentryId(); + void ResetSentryId(); + + void LogAssert( const ASSERT_CACHE_KEY& aKey, const wxString& aMsg ); + void LogException( const wxString& aMsg ); + + private: + SENTRY(); + + bool isConfiguredOptedIn(); + void sentryInit(); + wxString sentryCreateUid(); + void readOrCreateUid(); + + static SENTRY* m_instance; + + bool m_isOptedIn; + wxFileName m_sentry_optin_fn; + wxFileName m_sentry_uid_fn; + wxString m_sentryUid; + + std::set m_assertCache; + }; + + /** + * Add a sentry breadcrumb + */ KICOMMON_API void AddBreadcrumb( BREADCRUMB_TYPE aType, const wxString& aMsg, const wxString& aCategory, BREADCRUMB_LEVEL aLevel = BREADCRUMB_LEVEL::INFO ); + /** + * Add a navigation breadcrumb + */ KICOMMON_API void AddNavigationBreadcrumb( const wxString& aMsg, const wxString& aCategory ); + + /** + * Add a transaction breadcrumb + */ KICOMMON_API void AddTransactionBreadcrumb( const wxString& aMsg, const wxString& aCategory ); } \ No newline at end of file diff --git a/include/pgm_base.h b/include/pgm_base.h index b09c6d16d6..5bbd2e052d 100644 --- a/include/pgm_base.h +++ b/include/pgm_base.h @@ -305,31 +305,6 @@ public: */ void SaveCommonSettings(); -#ifdef KICAD_USE_SENTRY - /** - * @return True if the user agreed to sentry data collection - */ - bool IsSentryOptedIn(); - - /** - * Set the Sentry opt in state, this will also terminate sentry - * immediately if needed, however it will not init sentry if opted in. - * - * @param aOptIn True/false to agreeing to the use of sentry. - */ - void SetSentryOptIn( bool aOptIn ); - - /** - * Generate and stores a new sentry id at random using the boost uuid generator. - */ - void ResetSentryId(); - - /** - * Get the current id string being used as "user id" in sentry reports. - */ - const wxString& GetSentryId(); -#endif - /** * A exception handler to be used at the top level if exceptions bubble up that for. *