From 6c639d2317b002d7a6ab7952a41d5e8cd58be2b3 Mon Sep 17 00:00:00 2001
From: Adrien Beraud <adrien.beraud@savoirfairelinux.com>
Date: Wed, 8 Jan 2025 13:35:51 -0500
Subject: [PATCH] logs: add accountId

Change-Id: Ia3ec3a96660821175d52c0120642c7c6fc946d1c
---
 src/account.h                   |  2 +-
 src/im/message_engine.cpp       | 16 +++----
 src/jamidht/account_manager.cpp | 82 ++++++++++++++++-----------------
 src/jamidht/account_manager.h   |  2 +-
 src/jamidht/conversation.cpp    |  6 +--
 src/jamidht/jamiaccount.cpp     | 19 ++++----
 src/sip/sipaccountbase.cpp      |  4 +-
 7 files changed, 65 insertions(+), 66 deletions(-)

diff --git a/src/account.h b/src/account.h
index a2887f642..5baf2bb48 100644
--- a/src/account.h
+++ b/src/account.h
@@ -145,7 +145,7 @@ public:
      * Get the account ID
      * @return constant account id
      */
-    const std::string& getAccountID() const { return accountID_; }
+    inline const std::string& getAccountID() const { return accountID_; }
 
     virtual std::string_view getAccountType() const = 0;
 
diff --git a/src/im/message_engine.cpp b/src/im/message_engine.cpp
index 5b6602372..b0ec019e9 100644
--- a/src/im/message_engine.cpp
+++ b/src/im/message_engine.cpp
@@ -114,7 +114,7 @@ MessageEngine::retrySend(const std::string& peer, const std::string& deviceId, b
     }
     // avoid locking while calling callback
     for (const auto& p : pending) {
-        JAMI_DEBUG("[message {:d}] Reattempt sending", p.token);
+        JAMI_DEBUG("[Account {:s}] [message {:d}] Reattempt sending", account_.getAccountID(), p.token);
         if (p.payloads.find("application/im-gitmessage-id") == p.payloads.end())
             emitSignal<libjami::ConfigurationSignal::AccountMessageStatusChanged>(
                 account_.getAccountID(),
@@ -145,13 +145,13 @@ MessageEngine::onMessageSent(const std::string& peer,
                              bool ok,
                              const std::string& deviceId)
 {
-    JAMI_DEBUG("[message {:d}] Message sent: {:s}", token, ok ? "success"sv : "failure"sv);
+    JAMI_DEBUG("[Account {:s}] [message {:d}] Message sent: {:s}", account_.getAccountID(), token, ok ? "success"sv : "failure"sv);
     std::lock_guard lock(messagesMutex_);
     auto& m = deviceId.empty() ? messages_ : messagesDevices_;
 
     auto p = m.find(deviceId.empty() ? peer : deviceId);
     if (p == m.end()) {
-        JAMI_WARNING("onMessageSent: Peer not found: id:{} device:{}", peer, deviceId);
+        JAMI_WARNING("[Account {:s}] onMessageSent: Peer not found: id:{} device:{}", account_.getAccountID(), peer, deviceId);
         return;
     }
 
@@ -164,7 +164,7 @@ MessageEngine::onMessageSent(const std::string& peer,
         if (f->status == MessageStatus::SENDING) {
             if (ok) {
                 f->status = MessageStatus::SENT;
-                JAMI_LOG("[message {:d}] Status changed to SENT", token);
+                JAMI_LOG("[Account {:s}] [message {:d}] Status changed to SENT", account_.getAccountID(), token);
                 if (emit)
                     emitSignal<libjami::ConfigurationSignal::AccountMessageStatusChanged>(
                         account_.getAccountID(),
@@ -176,7 +176,7 @@ MessageEngine::onMessageSent(const std::string& peer,
                 scheduleSave();
             } else if (f->retried >= MAX_RETRIES) {
                 f->status = MessageStatus::FAILURE;
-                JAMI_WARNING("[message {:d}] Status changed to FAILURE", token);
+                JAMI_WARNING("[Account {:s}] [message {:d}] Status changed to FAILURE", account_.getAccountID(), token);
                 if (emit)
                     emitSignal<libjami::ConfigurationSignal::AccountMessageStatusChanged>(
                         account_.getAccountID(),
@@ -188,13 +188,13 @@ MessageEngine::onMessageSent(const std::string& peer,
                 scheduleSave();
             } else {
                 f->status = MessageStatus::IDLE;
-                JAMI_DEBUG("[message {:d}] Status changed to IDLE", token);
+                JAMI_DEBUG("[Account {:s}] [message {:d}] Status changed to IDLE", account_.getAccountID(), token);
             }
         } else {
-            JAMI_DEBUG("[message {:d}] State is not SENDING", token);
+            JAMI_DEBUG("[Account {:s}] [message {:d}] State is not SENDING", account_.getAccountID(), token);
         }
     } else {
-        JAMI_DEBUG("[message {:d}] Unable to find message", token);
+        JAMI_DEBUG("[Account {:s}] [message {:d}] Unable to find message", account_.getAccountID(), token);
     }
 }
 
diff --git a/src/jamidht/account_manager.cpp b/src/jamidht/account_manager.cpp
index 90b5c9aea..437b27ec9 100644
--- a/src/jamidht/account_manager.cpp
+++ b/src/jamidht/account_manager.cpp
@@ -184,13 +184,13 @@ AccountManager::useIdentity(const std::string& accountId,
         return nullptr;
 
     if (not identity.first or not identity.second) {
-        JAMI_ERR("[Auth] no identity provided");
+        JAMI_ERROR("[Account {}] [Auth] no identity provided", accountId);
         return nullptr;
     }
 
     auto accountCertificate = identity.second->issuer;
     if (not accountCertificate) {
-        JAMI_ERR("[Auth] device certificate must be issued by the account certificate");
+        JAMI_ERROR("[Account {}] [Auth] device certificate must be issued by the account certificate", accountId);
         return nullptr;
     }
 
@@ -198,38 +198,38 @@ AccountManager::useIdentity(const std::string& accountId,
     auto contactList = std::make_unique<ContactList>(accountId, accountCertificate, path_, onChange);
     auto result = contactList->isValidAccountDevice(*identity.second);
     if (not result) {
-        JAMI_ERR("[Auth] unable to use identity: device certificate chain is unable to be verified: %s",
-                 result.toString().c_str());
+        JAMI_ERROR("[Account {}] [Auth] unable to use identity: device certificate chain is unable to be verified: {}", accountId,
+                 result.toString());
         return nullptr;
     }
 
     auto pk = accountCertificate->getSharedPublicKey();
-    JAMI_DBG("[Auth] checking device receipt for %s", pk->getId().toString().c_str());
+    JAMI_LOG("[Account {}] [Auth] checking device receipt for {}", accountId, pk->getId().toString());
     if (!pk->checkSignature({receipt.begin(), receipt.end()}, receiptSignature)) {
-        JAMI_ERR("[Auth] device receipt signature check failed");
+        JAMI_ERROR("[Auth] device receipt signature check failed");
         return nullptr;
     }
 
     auto root = announceFromReceipt(receipt);
     if (!root.isMember("announce")) {
-        JAMI_ERR() << this << " device receipt parsing error";
+        JAMI_ERROR("[Account {}] [Auth] device receipt parsing error", accountId);
         return nullptr;
     }
 
     auto dev_id = root["dev"].asString();
     if (dev_id != identity.second->getId().toString()) {
-        JAMI_ERR("[Auth] device ID mismatch between receipt and certificate");
+        JAMI_ERROR("[Account {}] [Auth] device ID mismatch between receipt and certificate", accountId);
         return nullptr;
     }
     auto id = root["id"].asString();
     if (id != pk->getId().toString()) {
-        JAMI_ERR("[Auth] account ID mismatch between receipt and certificate");
+        JAMI_ERROR("[Account {}] [Auth] account ID mismatch between receipt and certificate", accountId);
         return nullptr;
     }
 
     auto devicePk = identity.first->getSharedPublicKey();
     if (!devicePk) {
-        JAMI_ERR("[Auth] No device pk found");
+        JAMI_ERROR("[Account {}] [Auth] No device pk found", accountId);
         return nullptr;
     }
 
@@ -252,9 +252,8 @@ AccountManager::useIdentity(const std::string& accountId,
     info->username = username;
     info_ = std::move(info);
 
-    JAMI_DBG("[Auth] Device %s receipt checked successfully for account %s",
-             info_->deviceId.c_str(),
-             id.c_str());
+    JAMI_LOG("[Account {}] [Auth] Device {} receipt checked successfully for user {}", accountId,
+             info_->deviceId, id);
     return info_.get();
 }
 
@@ -273,7 +272,7 @@ AccountManager::announceFromReceipt(const std::string& receipt)
     Json::CharReaderBuilder rbuilder;
     auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader());
     if (!reader->parse(&receipt[0], &receipt[receipt.size()], &root, nullptr)) {
-        JAMI_ERR() << this << " device receipt parsing error";
+        JAMI_ERROR("[Account {}] error parsing device receipt", accountId_);
     }
     return root;
 }
@@ -288,9 +287,9 @@ AccountManager::startSync(const OnNewDeviceCb& cb, const OnDeviceAnnouncedCb& dc
             dht_->put(
                 h,
                 info_->announce,
-                [dcb = std::move(dcb), h](bool ok) {
+                [dcb = std::move(dcb), h, accountId=accountId_](bool ok) {
                     if (ok)
-                        JAMI_DEBUG("device announced at {}", h.toString());
+                        JAMI_DEBUG("[Account {}] device announced at {}", accountId, h.toString());
                     // We do not care about the status, it's a permanent put, if this fail,
                     // this means the DHT is disconnected but the put will be retried when connected.
                     if (dcb)
@@ -312,7 +311,7 @@ AccountManager::startSync(const OnNewDeviceCb& cb, const OnDeviceAnnouncedCb& dc
         });
         dht_->listen<dht::crypto::RevocationList>(h, [this](dht::crypto::RevocationList&& crl) {
             if (crl.isSignedBy(*info_->identity.second->issuer)) {
-                JAMI_DEBUG("Found CRL for account.");
+                JAMI_DEBUG("[Account {}] Found CRL for account.", accountId_);
                 certStore()
                     .pinRevocationList(info_->accountId,
                                        std::make_shared<dht::crypto::RevocationList>(
@@ -322,7 +321,7 @@ AccountManager::startSync(const OnNewDeviceCb& cb, const OnDeviceAnnouncedCb& dc
         });
         syncDevices();
     } else {
-        JAMI_WARNING("Unable to announce device: no announcement.");
+        JAMI_ERROR("[Account {}] Unable to announce device: no announcement.", accountId_);
     }
 
     auto inboxKey = dht::InfoHash::get("inbox:" + info_->devicePk->getId().toString());
@@ -336,7 +335,8 @@ AccountManager::startSync(const OnNewDeviceCb& cb, const OnDeviceAnnouncedCb& dc
             true,
             [this, v](const std::shared_ptr<dht::crypto::Certificate>&,
                       dht::InfoHash peer_account) mutable {
-                JAMI_WARNING("Got trust request (confirm: {}) from: {} / {}. ConversationId: {}",
+                JAMI_WARNING("[Account {}] Got trust request (confirm: {}) from: {} / {}. ConversationId: {}",
+                          accountId_,
                           v.confirm,
                           peer_account.toString(),
                           v.from.toString(),
@@ -403,8 +403,8 @@ AccountManager::getAccountDeviceName() const
 }
 
 bool
-AccountManager::foundPeerDevice(const std::shared_ptr<dht::crypto::Certificate>& crt,
-                                dht::InfoHash& account_id)
+AccountManager::foundPeerDevice(const std::string& accountId, const std::shared_ptr<dht::crypto::Certificate>& crt,
+                                dht::InfoHash& peer_id)
 {
     if (not crt)
         return false;
@@ -415,7 +415,7 @@ AccountManager::foundPeerDevice(const std::shared_ptr<dht::crypto::Certificate>&
 
     // Device certificate is unable to be self-signed
     if (top_issuer == crt) {
-        JAMI_WARN("Found invalid peer device: %s", crt->getLongId().toString().c_str());
+        JAMI_WARNING("[Account {}] Found invalid peer device: {}", accountId, crt->getLongId().toString());
         return false;
     }
 
@@ -424,21 +424,22 @@ AccountManager::foundPeerDevice(const std::shared_ptr<dht::crypto::Certificate>&
     dht::crypto::TrustList peer_trust;
     peer_trust.add(*top_issuer);
     if (not peer_trust.verify(*crt)) {
-        JAMI_WARN("Found invalid peer device: %s", crt->getLongId().toString().c_str());
+        JAMI_WARNING("[Account {}] Found invalid peer device: {}", accountId, crt->getLongId().toString());
         return false;
     }
 
     // Check cached OCSP response
     if (crt->ocspResponse and crt->ocspResponse->getCertificateStatus() != GNUTLS_OCSP_CERT_GOOD) {
-        JAMI_ERR("Certificate %s is disabled by cached OCSP response", crt->getLongId().to_c_str());
+        JAMI_WARNING("[Account {}] Certificate {} is disabled by cached OCSP response", accountId, crt->getLongId());
         return false;
     }
 
-    account_id = crt->issuer->getId();
-    JAMI_WARN("Found peer device: %s account:%s CA:%s",
-              crt->getLongId().toString().c_str(),
-              account_id.toString().c_str(),
-              top_issuer->getId().toString().c_str());
+    peer_id = crt->issuer->getId();
+    JAMI_WARNING("[Account {}] Found peer device: {} account:{} CA:{}",
+              accountId,
+              crt->getLongId().toString(),
+              peer_id.toString(),
+              top_issuer->getId().toString());
     return true;
 }
 
@@ -451,7 +452,7 @@ AccountManager::onPeerMessage(const dht::crypto::PublicKey& peer_device,
     // quick check in case we already explicilty banned this device
     auto trustStatus = getCertificateStatus(peer_device.toString());
     if (trustStatus == dhtnet::tls::TrustStore::PermissionStatus::BANNED) {
-        JAMI_WARN("[Auth] Discarding message from banned device %s", peer_device.toString().c_str());
+        JAMI_WARNING("[Account {}] [Auth] Discarding message from banned device {}", accountId_, peer_device.toString());
         return;
     }
 
@@ -471,14 +472,14 @@ AccountManager::onPeerCertificate(const std::shared_ptr<dht::crypto::Certificate
                                   dht::InfoHash& account_id)
 {
     dht::InfoHash peer_account_id;
-    if (not foundPeerDevice(cert, peer_account_id)) {
-        JAMI_WARN("[Auth] Discarding message from invalid peer certificate");
+    if (not foundPeerDevice(accountId_, cert, peer_account_id)) {
+        JAMI_WARNING("[Account {}] [Auth] Discarding message from invalid peer certificate", accountId_);
         return false;
     }
 
     if (not isAllowed(*cert, allowPublic)) {
-        JAMI_WARN("[Auth] Discarding message from unauthorized peer %s.",
-                  peer_account_id.toString().c_str());
+        JAMI_WARNING("[Account {}] [Auth] Discarding message from unauthorized peer {}.",
+                  accountId_, peer_account_id.toString());
         return false;
     }
 
@@ -489,11 +490,11 @@ AccountManager::onPeerCertificate(const std::shared_ptr<dht::crypto::Certificate
 bool
 AccountManager::addContact(const dht::InfoHash& uri, bool confirmed, const std::string& conversationId)
 {
-    JAMI_WARNING("AccountManager::addContact {}", confirmed);
     if (not info_) {
         JAMI_ERROR("addContact(): account not loaded");
         return false;
     }
+    JAMI_WARNING("[Account {}] addContact {}", accountId_, confirmed);
     if (info_->contacts->addContact(uri, confirmed, conversationId)) {
         syncDevices();
         return true;
@@ -506,11 +507,11 @@ AccountManager::removeContact(const std::string& uri, bool banned)
 {
     dht::InfoHash h(uri);
     if (not h) {
-        JAMI_ERROR("removeContact: invalid contact URI");
+        JAMI_ERROR("[Account {}] removeContact: invalid contact URI", accountId_);
         return;
     }
     if (not info_) {
-        JAMI_ERROR("addContact(): account not loaded");
+        JAMI_ERROR("[Account {}] removeContact: account not loaded", accountId_);
         return;
     }
     if (info_->contacts->removeContact(h, banned))
@@ -723,9 +724,9 @@ AccountManager::sendTrustRequest(const std::string& to,
     forEachDevice(toH,
                   [this, toH, convId, payload](const std::shared_ptr<dht::crypto::PublicKey>& dev) {
                       auto to = toH.toString();
-                      JAMI_WARNING("Sending trust request to: {:s} / {:s}",
+                      JAMI_WARNING("Sending trust request to: {:s} / {:s} of size {:d}",
                                    to,
-                                   dev->getLongId().toString());
+                                   dev->getLongId().toString(), payload.size());
                       dht_->putEncrypted(dht::InfoHash::get("inbox:" + dev->getId().toString()),
                                          dev,
                                          dht::TrustRequest(DHT_TYPE_NS, convId, payload),
@@ -743,9 +744,8 @@ void
 AccountManager::sendTrustRequestConfirm(const dht::InfoHash& toH, const std::string& convId)
 {
     JAMI_WARN("AccountManager::sendTrustRequestConfirm");
-    dht::TrustRequest answer {DHT_TYPE_NS, ""};
+    dht::TrustRequest answer {DHT_TYPE_NS, convId};
     answer.confirm = true;
-    answer.conversationId = convId;
 
     if (!convId.empty() && info_)
         info_->contacts->acceptConversation(convId);
diff --git a/src/jamidht/account_manager.h b/src/jamidht/account_manager.h
index d1e1f6524..35f5fbf27 100644
--- a/src/jamidht/account_manager.h
+++ b/src/jamidht/account_manager.h
@@ -198,7 +198,7 @@ public:
      * Returns true only if the device certificate is a valid device certificate.
      * In that case (true is returned) the account_id parameter is set to the peer account ID.
      */
-    static bool foundPeerDevice(const std::shared_ptr<dht::crypto::Certificate>& crt,
+    static bool foundPeerDevice(const std::string& accoundId, const std::shared_ptr<dht::crypto::Certificate>& crt,
                                 dht::InfoHash& account_id);
 
     // Contact requests
diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index 03ca7cbdc..5b25af67a 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -228,9 +228,9 @@ public:
                         peer = member;
                     }
                 }
-                fmtStr_ = fmt::format("[Conversation (1:1) {}]", peer);
+                fmtStr_ = fmt::format("[Account {}] [Conversation (1:1) {}]", accountId_, peer);
             } else {
-                fmtStr_ = fmt::format("[Conversation {}]", repository_->id());
+                fmtStr_ = fmt::format("[Account {}] [Conversation {}]", accountId_, repository_->id());
             }
         }
         return fmtStr_;
@@ -243,7 +243,7 @@ public:
             if (fallbackTimer_)
                 fallbackTimer_->cancel();
         } catch (const std::exception& e) {
-            JAMI_ERROR("[Conversation {:s}] {:s}", toString(), e.what());
+            JAMI_ERROR("{:s} {:s}", toString(), e.what());
         }
     }
 
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index e47e809e4..a36ad6494 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -1672,15 +1672,14 @@ JamiAccount::trackBuddyPresence(const std::string& buddy_id, bool track)
     try {
         buddyUri = parseJamiUri(buddy_id);
     } catch (...) {
-        JAMI_ERR("[Account %s] Failed to track presence: invalid URI %s",
-                 getAccountID().c_str(),
-                 buddy_id.c_str());
+        JAMI_ERROR("[Account {:s}] Failed to track presence: invalid URI {:s}",
+                 getAccountID(), buddy_id);
         return;
     }
-    JAMI_DBG("[Account %s] %s presence for %s",
-             getAccountID().c_str(),
+    JAMI_LOG("[Account {:s}] {:s} presence for {:s}",
+             getAccountID(),
              track ? "Track" : "Untrack",
-             buddy_id.c_str());
+             buddy_id);
 
     auto h = dht::InfoHash(buddyUri);
     std::unique_lock lock(buddyInfoMtx);
@@ -1765,7 +1764,7 @@ void
 JamiAccount::onTrackedBuddyOnline(const dht::InfoHash& contactId)
 {
     std::string id(contactId.toString());
-    JAMI_DEBUG("Buddy {} online", id);
+    JAMI_DEBUG("[Account {:s}] Buddy {} online", getAccountID(), id);
     auto& state = presenceState_[id];
     if (state < PresenceState::AVAILABLE) {
         state = PresenceState::AVAILABLE;
@@ -1795,7 +1794,7 @@ JamiAccount::onTrackedBuddyOnline(const dht::InfoHash& contactId)
             }
 
             if (payload.size() >= 64000) {
-                JAMI_WARN() << "Trust request is too big, reset payload";
+                JAMI_WARNING("[Account {:s}] Trust request for contact {:s} is too big, reset payload", getAccountID(), id);
                 payload.clear();
             }
 
@@ -1808,11 +1807,11 @@ void
 JamiAccount::onTrackedBuddyOffline(const dht::InfoHash& contactId)
 {
     auto id = contactId.toString();
-    JAMI_DEBUG("Buddy {} offline", id);
+    JAMI_DEBUG("[Account {:s}] Buddy {} offline", getAccountID(), id);
     auto& state = presenceState_[id];
     if (state > PresenceState::DISCONNECTED) {
         if (state == PresenceState::CONNECTED) {
-            JAMI_WARNING("Buddy {} is not present on the DHT, but P2P connected", id);
+            JAMI_WARNING("[Account {:s}] Buddy {} is not present on the DHT, but P2P connected", getAccountID(), id);
         }
         state = PresenceState::DISCONNECTED;
         emitSignal<libjami::PresenceSignal::NewBuddyNotification>(getAccountID(),
diff --git a/src/sip/sipaccountbase.cpp b/src/sip/sipaccountbase.cpp
index ba7d3fdcd..630190a96 100644
--- a/src/sip/sipaccountbase.cpp
+++ b/src/sip/sipaccountbase.cpp
@@ -277,12 +277,12 @@ SIPAccountBase::onTextMessage(const std::string& id,
                               const std::string& /* deviceId */,
                               const std::map<std::string, std::string>& payloads)
 {
-    JAMI_DBG("Text message received from %s, %zu part(s)", from.c_str(), payloads.size());
+    JAMI_LOG("[Account {}] Text message received from {}, {:d} part(s)", accountID_, from, payloads.size());
     for (const auto& m : payloads) {
         if (!utf8_validate(m.first))
             return;
         if (!utf8_validate(m.second)) {
-            JAMI_WARN("Dropping invalid message with MIME type %s", m.first.c_str());
+            JAMI_WARNING("[Account {}] Dropping invalid message with MIME type {}", accountID_, m.first);
             return;
         }
         if (handleMessage(from, m))
-- 
GitLab