From 38a0cdb4abbb9de3285f272794e6f8bbdb5b8a16 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Wed, 14 Jun 2023 08:09:11 +0200 Subject: [PATCH] feat(GODT-2690): update sentry reporting in GUI for new log file naming. --- .../bridge-gui/bridge-gui/AppController.cpp | 20 ++++++++++++++++++ .../bridge-gui/bridge-gui/AppController.h | 13 +++++++----- .../bridge-gui/bridge-gui/CommandLine.cpp | 10 +++++---- .../bridge-gui/bridge-gui/CommandLine.h | 1 - .../bridge-gui/bridge-gui/LogUtils.cpp | 11 +++------- .../frontend/bridge-gui/bridge-gui/LogUtils.h | 2 +- .../bridge-gui/bridge-gui/QMLBackend.cpp | 4 ++-- .../frontend/bridge-gui/bridge-gui/main.cpp | 17 ++++++++------- .../bridgepp/bridgepp/GRPC/GRPCClient.cpp | 16 ++++++++------ .../bridgepp/bridgepp/GRPC/GRPCClient.h | 5 +++-- .../bridgepp/bridgepp/Log/LogUtils.cpp | 21 +++++++------------ .../bridgepp/bridgepp/Log/LogUtils.h | 2 +- 12 files changed, 71 insertions(+), 51 deletions(-) diff --git a/internal/frontend/bridge-gui/bridge-gui/AppController.cpp b/internal/frontend/bridge-gui/bridge-gui/AppController.cpp index 0f2ad890..e7ce8189 100644 --- a/internal/frontend/bridge-gui/bridge-gui/AppController.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/AppController.cpp @@ -117,7 +117,27 @@ void AppController::restart(bool isCrashing) { } +//**************************************************************************************************************************************************** +/// \param[in] launcher The launcher. +/// \param[in] args The launcher arguments. +//**************************************************************************************************************************************************** void AppController::setLauncherArgs(const QString &launcher, const QStringList &args) { launcher_ = launcher; launcherArgs_ = args; } + + +//**************************************************************************************************************************************************** +/// \param[in] sessionID The sessionID. +//**************************************************************************************************************************************************** +void AppController::setSessionID(const QString &sessionID) { + sessionID_ = sessionID; +} + + +//**************************************************************************************************************************************************** +/// \return The sessionID. +//**************************************************************************************************************************************************** +QString AppController::sessionID() { + return sessionID_; +} diff --git a/internal/frontend/bridge-gui/bridge-gui/AppController.h b/internal/frontend/bridge-gui/bridge-gui/AppController.h index d55e1f11..be9f9cfe 100644 --- a/internal/frontend/bridge-gui/bridge-gui/AppController.h +++ b/internal/frontend/bridge-gui/bridge-gui/AppController.h @@ -37,7 +37,7 @@ class Exception; /// \brief App controller class. //**************************************************************************************************************************************************** class AppController : public QObject { -Q_OBJECT + Q_OBJECT friend AppController &app(); public: // member functions. @@ -52,10 +52,12 @@ public: // member functions. std::unique_ptr &bridgeOverseer() { return bridgeOverseer_; }; ///< Returns a reference the bridge overseer bridgepp::ProcessMonitor *bridgeMonitor() const; ///< Return the bridge worker. Settings &settings();; ///< Return the application settings. - void setLauncherArgs(const QString &launcher, const QStringList &args); + void setLauncherArgs(const QString &launcher, const QStringList &args); ///< Set the launcher arguments. + void setSessionID(QString const &sessionID); ///< Set the sessionID. + QString sessionID(); ///< Get the sessionID. public slots: - void onFatalError(bridgepp::Exception const& e); ///< Handle fatal errors. + void onFatalError(bridgepp::Exception const &e); ///< Handle fatal errors. private: // member functions AppController(); ///< Default constructor. @@ -67,8 +69,9 @@ private: // data members std::unique_ptr log_; ///< The log. std::unique_ptr bridgeOverseer_; ///< The overseer for the bridge monitor worker. std::unique_ptr settings_; ///< The application settings. - QString launcher_; - QStringList launcherArgs_; + QString launcher_; ///< The launcher. + QStringList launcherArgs_; ///< The launcher arguments. + QString sessionID_; ///< The sessionID. }; diff --git a/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp b/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp index f01def14..1daf3a2b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp @@ -143,12 +143,14 @@ CommandLineOptions parseCommandLine(int argc, char *argv[]) { options.logLevel = parseLogLevel(argc, argv); - options.sessionID = parseGoCLIStringArgument(argc, argv, { "session-id" }); - if (options.sessionID.isEmpty()) { - options.sessionID = newSessionID(); + QString sessionID = parseGoCLIStringArgument(argc, argv, { "session-id" }); + if (sessionID.isEmpty()) { + // The session ID was not passed to us on the command-line -> create one and add to the command-line for bridge + sessionID = newSessionID(); options.bridgeArgs.append("--session-id"); - options.bridgeArgs.append(options.sessionID); + options.bridgeArgs.append(sessionID); } + app().setSessionID(sessionID); return options; } diff --git a/internal/frontend/bridge-gui/bridge-gui/CommandLine.h b/internal/frontend/bridge-gui/bridge-gui/CommandLine.h index 6668c9ef..9accfee8 100644 --- a/internal/frontend/bridge-gui/bridge-gui/CommandLine.h +++ b/internal/frontend/bridge-gui/bridge-gui/CommandLine.h @@ -34,7 +34,6 @@ struct CommandLineOptions { bridgepp::Log::Level logLevel { bridgepp::Log::defaultLevel }; ///< The log level bool noWindow { false }; ///< Should the application start without displaying the main window? bool useSoftwareRenderer { false }; ///< Should QML be renderer in software (i.e. without rendering hardware interface). - QString sessionID; ///< The sessionID. }; diff --git a/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp index b2544067..ceba82d8 100644 --- a/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp @@ -27,20 +27,15 @@ using namespace bridgepp; //**************************************************************************************************************************************************** /// \return A reference to the log. //**************************************************************************************************************************************************** -Log &initLog(QString const &sessionID) { +Log &initLog() { Log &log = app().log(); log.registerAsQtMessageHandler(); log.setEchoInConsole(true); - // remove old gui log files - QDir const logsDir(userLogsDir()); - for (QFileInfo const fileInfo: logsDir.entryInfoList({ "gui_v*.log" }, QDir::Filter::Files)) { // entryInfolist apparently only support wildcards, not regex. - QFile(fileInfo.absoluteFilePath()).remove(); - } - // create new GUI log file QString error; - if (!log.startWritingToFile(logsDir.absoluteFilePath(QString("%1_gui_000_v%2_%3.log").arg(sessionID, PROJECT_VER, PROJECT_TAG)), &error)) { + if (!log.startWritingToFile(QDir(userLogsDir()).absoluteFilePath(QString("%1_gui_000_v%2_%3.log").arg(app().sessionID(), + PROJECT_VER, PROJECT_TAG)), &error)) { log.error(error); } diff --git a/internal/frontend/bridge-gui/bridge-gui/LogUtils.h b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h index 34161fdc..b324301e 100644 --- a/internal/frontend/bridge-gui/bridge-gui/LogUtils.h +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h @@ -23,7 +23,7 @@ #include -bridgepp::Log &initLog(QString const &sessionID); ///< Initialize the application log. +bridgepp::Log &initLog(); ///< Initialize the application log. #endif //BRIDGE_GUI_LOG_UTILS_H diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp index f6471239..b7c884c2 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp @@ -61,7 +61,7 @@ void QMLBackend::init(GRPCConfig const &serviceConfig) { app().grpc().setLog(&log); this->connectGrpcEvents(); - app().grpc().connectToServer(bridgepp::userConfigDir(), serviceConfig, app().bridgeMonitor()); + app().grpc().connectToServer(app().sessionID(), bridgepp::userConfigDir(), serviceConfig, app().bridgeMonitor()); app().log().info("Connected to backend via gRPC service."); QString bridgeVer; @@ -640,7 +640,7 @@ void QMLBackend::login(QString const &username, QString const &password) const { HANDLE_EXCEPTION( if (username.compare("coco@bandicoot", Qt::CaseInsensitive) == 0) { throw Exception("User requested bridge-gui to crash by trying to log as coco@bandicoot", - "This error exists for test purposes and should be ignored.", __func__, tailOfLatestBridgeLog()); + "This error exists for test purposes and should be ignored.", __func__, tailOfLatestBridgeLog(app().sessionID())); } app().grpc().login(username, password); ) diff --git a/internal/frontend/bridge-gui/bridge-gui/main.cpp b/internal/frontend/bridge-gui/bridge-gui/main.cpp index c357562f..e93bb4e0 100644 --- a/internal/frontend/bridge-gui/bridge-gui/main.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/main.cpp @@ -287,7 +287,7 @@ int main(int argc, char *argv[]) { initQtApplication(); CommandLineOptions const cliOptions = parseCommandLine(argc, argv); - Log &log = initLog(cliOptions.sessionID); + Log &log = initLog(); QLockFile lock(bridgepp::userCacheDir() + "/" + bridgeGUILock); if (!checkSingleInstance(lock)) { @@ -306,22 +306,23 @@ int main(int argc, char *argv[]) { log.setLevel(cliOptions.logLevel); log.info(QString("New Sentry reporter - id: %1.").arg(getProtectedHostname())); - QString bridgeexec; + QString const &sessionID = app().sessionID(); + QString bridgeExe; if (!cliOptions.attach) { if (isBridgeRunning()) { throw Exception("An orphan instance of bridge is already running. Please terminate it and relaunch the application.", - QString(), __FUNCTION__, tailOfLatestBridgeLog()); + QString(), __FUNCTION__, tailOfLatestBridgeLog(sessionID)); } // before launching bridge, we remove any trailing service config file, because we need to make sure we get a newly generated one. FocusGRPCClient::removeServiceConfigFile(configDir); GRPCClient::removeServiceConfigFile(configDir); - bridgeexec = launchBridge(cliOptions.bridgeArgs); + bridgeExe = launchBridge(cliOptions.bridgeArgs); } log.info(QString("Retrieving gRPC service configuration from '%1'").arg(QDir::toNativeSeparators(grpcServerConfigPath(configDir)))); - app().backend().init(GRPCClient::waitAndRetrieveServiceConfig(configDir, cliOptions.attach ? 0 : grpcServiceConfigWaitDelayMs, - app().bridgeMonitor())); + app().backend().init(GRPCClient::waitAndRetrieveServiceConfig(sessionID, configDir, + cliOptions.attach ? 0 : grpcServiceConfigWaitDelayMs, app().bridgeMonitor())); if (!cliOptions.attach) { GRPCClient::removeServiceConfigFile(configDir); } @@ -379,9 +380,9 @@ int main(int argc, char *argv[]) { QStringList args = cliOptions.bridgeGuiArgs; args.append(waitFlag); args.append(mainexec); - if (!bridgeexec.isEmpty()) { + if (!bridgeExe.isEmpty()) { args.append(waitFlag); - args.append(bridgeexec); + args.append(bridgeExe); } app().setLauncherArgs(cliOptions.launcher, args); result = QGuiApplication::exec(); diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp index 34038c49..07f7ac13 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp @@ -57,11 +57,13 @@ void GRPCClient::removeServiceConfigFile(QString const &configDir) { //**************************************************************************************************************************************************** +/// \param[in] sessionID The sessionID. /// \param[in] timeoutMs The timeout in milliseconds /// \param[in] serverProcess An optional server process to monitor. If the process it, no need and retry, as connexion cannot be established. Ignored if null. /// \return The service config. //**************************************************************************************************************************************************** -GRPCConfig GRPCClient::waitAndRetrieveServiceConfig(QString const &configDir, qint64 timeoutMs, ProcessMonitor *serverProcess) { +GRPCConfig GRPCClient::waitAndRetrieveServiceConfig(QString const & sessionID, QString const &configDir, qint64 timeoutMs, + ProcessMonitor *serverProcess) { QString const path = grpcServerConfigPath(configDir); QFile file(path); @@ -71,7 +73,7 @@ GRPCConfig GRPCClient::waitAndRetrieveServiceConfig(QString const &configDir, qi while (true) { if (serverProcess && serverProcess->getStatus().ended) { throw Exception("Bridge application exited before providing a gRPC service configuration file.", QString(), __FUNCTION__, - tailOfLatestBridgeLog()); + tailOfLatestBridgeLog(sessionID)); } if (file.exists()) { @@ -85,7 +87,7 @@ GRPCConfig GRPCClient::waitAndRetrieveServiceConfig(QString const &configDir, qi } if (!found) { - throw Exception("Server did not provide gRPC service configuration in time.", QString(), __FUNCTION__, tailOfLatestBridgeLog()); + throw Exception("Server did not provide gRPC service configuration in time.", QString(), __FUNCTION__, tailOfLatestBridgeLog(sessionID)); } GRPCConfig sc; @@ -114,10 +116,12 @@ void GRPCClient::setLog(Log *log) { //**************************************************************************************************************************************************** +/// \param[in] sessionID The sessionID. +/// \param[in] configDir The configuration directory /// \param[in] serverProcess An optional server process to monitor. If the process it, no need and retry, as connexion cannot be established. Ignored if null. /// \return true iff the connection was successful. //**************************************************************************************************************************************************** -void GRPCClient::connectToServer(QString const &configDir, GRPCConfig const &config, ProcessMonitor *serverProcess) { +void GRPCClient::connectToServer(QString const &sessionID, QString const &configDir, GRPCConfig const &config, ProcessMonitor *serverProcess) { try { serverToken_ = config.token.toStdString(); QString address; @@ -147,7 +151,7 @@ void GRPCClient::connectToServer(QString const &configDir, GRPCConfig const &con while (true) { if (serverProcess && serverProcess->getStatus().ended) { throw Exception("Bridge application ended before gRPC connexion could be established.", QString(), __FUNCTION__, - tailOfLatestBridgeLog()); + tailOfLatestBridgeLog(sessionID)); } this->logInfo(QString("Connection to gRPC server at %1. attempt #%2").arg(address).arg(++i)); @@ -158,7 +162,7 @@ void GRPCClient::connectToServer(QString const &configDir, GRPCConfig const &con if (QDateTime::currentDateTime() > giveUpTime) { throw Exception("Connection to the gRPC server failed because of a timeout.", QString(), __FUNCTION__, - tailOfLatestBridgeLog()); + tailOfLatestBridgeLog(sessionID)); } } diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.h index 6485ce80..8095f178 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.h @@ -49,7 +49,8 @@ class GRPCClient : public QObject { Q_OBJECT public: // static member functions static void removeServiceConfigFile(QString const &configDir); ///< Delete the service config file. - static GRPCConfig waitAndRetrieveServiceConfig(QString const &configDir, qint64 timeoutMs, class ProcessMonitor *serverProcess); ///< Wait and retrieve the service configuration. + static GRPCConfig waitAndRetrieveServiceConfig(QString const &sessionID, QString const &configDir, qint64 timeoutMs, + class ProcessMonitor *serverProcess); ///< Wait and retrieve the service configuration. public: // member functions. GRPCClient() = default; ///< Default constructor. @@ -59,7 +60,7 @@ public: // member functions. GRPCClient &operator=(GRPCClient const &) = delete; ///< Disabled assignment operator. GRPCClient &operator=(GRPCClient &&) = delete; ///< Disabled move assignment operator. void setLog(Log *log); ///< Set the log for the client. - void connectToServer(QString const &configDir, GRPCConfig const &config, class ProcessMonitor *serverProcess); ///< Establish connection to the gRPC server. + void connectToServer(QString const &sessionID, QString const &configDir, GRPCConfig const &config, class ProcessMonitor *serverProcess); ///< Establish connection to the gRPC server. bool isConnected() const; ///< Check whether the gRPC client is connected to the server. grpc::Status checkTokens(QString const &clientConfigPath, QString &outReturnedClientToken); ///< Performs a token check. diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp index bca90da8..6920ec57 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp @@ -35,34 +35,29 @@ QString userLogsDir() { //**************************************************************************************************************************************************** /// \brief Return the path of the latest bridge log. +/// +/// \param[in] sessionID The sessionID. /// \return The path of the latest bridge log file. /// \return An empty string if no bridge log file was found. //**************************************************************************************************************************************************** -QString latestBridgeLogPath() { +QString latestBridgeLogPath(QString const &sessionID) { QDir const logsDir(userLogsDir()); if (logsDir.isEmpty()) { return QString(); } - QFileInfoList files = logsDir.entryInfoList({ "v*.log" }, QDir::Files); // could do sorting, but only by last modification time. we want to sort by creation time. - if (files.isEmpty()) { - return QString(); - } - - std::sort(files.begin(), files.end(), [](QFileInfo const &lhs, QFileInfo const &rhs) -> bool { - return lhs.birthTime() < rhs.birthTime(); - }); - return files.back().absoluteFilePath(); + QFileInfoList const files = logsDir.entryInfoList({ sessionID + "_bri_*.log" }, QDir::Files, QDir::Name); + return files.isEmpty() ? QString() : files.back().absoluteFilePath(); } //**************************************************************************************************************************************************** /// Return the maxSize last bytes of the latest bridge log. //**************************************************************************************************************************************************** -QByteArray tailOfLatestBridgeLog() { - QString path = latestBridgeLogPath(); +QByteArray tailOfLatestBridgeLog(QString const &sessionID) { + QString path = latestBridgeLogPath(sessionID); if (path.isEmpty()) { - return QByteArray(); + return QString("We could not find a bridge log file for the current session.").toLocal8Bit(); } QFile file(path); diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.h index 2918cd15..ab4f81d0 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.h @@ -24,7 +24,7 @@ namespace bridgepp { QString userLogsDir(); ///< Return the path of the user logs dir. -QByteArray tailOfLatestBridgeLog(); ///< Return the last bytes of the last bridge log. +QByteArray tailOfLatestBridgeLog(QString const &sessionID); ///< Return the last bytes of the last bridge log. } // namespace bridgepp