From 67aa1888309e83648be62988b10e757c14a2e2a9 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sat, 23 Feb 2019 09:25:21 -0500 Subject: [PATCH] Improve Logger calls - Add context in various calls - Remove deprecated Logger::log call in Processor --- mod/item.php | 2 +- src/Database/DBA.php | 2 +- src/Model/Item.php | 32 +++++++++++++++++++------- src/Model/Term.php | 2 ++ src/Protocol/ActivityPub/Processor.php | 2 +- 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/mod/item.php b/mod/item.php index 122f62c4e5..b126c4825b 100644 --- a/mod/item.php +++ b/mod/item.php @@ -1127,4 +1127,4 @@ function item_add_implicit_mentions(array $tags, array $thread_parent_contact, $ } return $tags; -} \ No newline at end of file +} diff --git a/src/Database/DBA.php b/src/Database/DBA.php index 326fb0771d..832f0a444f 100644 --- a/src/Database/DBA.php +++ b/src/Database/DBA.php @@ -425,7 +425,7 @@ class DBA if ((substr_count($sql, '?') != count($args)) && (count($args) > 0)) { // Question: Should we continue or stop the query here? - Logger::error('Parameter mismatch. Query "'.$sql.'" - Parameters '.print_r($args, true)); + Logger::warning('Query parameters mismatch.', ['query' => $sql, 'args' => $args, 'callstack' => System::callstack()]); } $sql = self::cleanQuery($sql); diff --git a/src/Model/Item.php b/src/Model/Item.php index e4d18c9a74..16a3b07b53 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -1336,7 +1336,11 @@ class Item extends BaseObject $expire_date = time() - ($expire_interval * 86400); $created_date = strtotime($item['created']); if ($created_date < $expire_date) { - Logger::notice('item-store: item created ('.date('c', $created_date).') before expiration time ('.date('c', $expire_date).'). ignored. ' . print_r($item,true)); + Logger::notice('Item created before expiration interval.', [ + 'created' => date('c', $created_date), + 'expired' => date('c', $expire_date), + '$item' => $item + ]); return 0; } } @@ -1354,7 +1358,13 @@ class Item extends BaseObject if (DBA::isResult($existing)) { // We only log the entries with a different user id than 0. Otherwise we would have too many false positives if ($uid != 0) { - Logger::notice("Item with uri ".$item['uri']." already existed for user ".$uid." with id ".$existing["id"]." target network ".$existing["network"]." - new network: ".$item['network']); + Logger::notice('Item already existed for user', [ + 'uri' => $item['uri'], + 'uid' => $uid, + 'network' => $item['network'], + 'existing_id' => $existing["id"], + 'existing_network' => $existing["network"] + ]); } return $existing["id"]; @@ -1432,7 +1442,7 @@ class Item extends BaseObject $item['author-id'] = defaults($item, 'author-id', Contact::getIdForURL($item["author-link"], 0, false, $default)); if (Contact::isBlocked($item["author-id"])) { - Logger::notice('Contact '.$item["author-id"].' is blocked, item '.$item["uri"].' will not be stored'); + Logger::notice('Author is blocked node-wide', ['author-link' => $item["author-link"], 'item-uri' => $item["uri"]]); return 0; } @@ -1442,21 +1452,27 @@ class Item extends BaseObject $item['owner-id'] = defaults($item, 'owner-id', Contact::getIdForURL($item["owner-link"], 0, false, $default)); if (Contact::isBlocked($item["owner-id"])) { - Logger::notice('Contact '.$item["owner-id"].' is blocked, item '.$item["uri"].' will not be stored'); + Logger::notice('Owner is blocked node-wide', ['owner-link' => $item["owner-link"], 'item-uri' => $item["uri"]]); return 0; } if ($item['network'] == Protocol::PHANTOM) { - Logger::notice('Missing network. Called by: '.System::callstack(), Logger::DEBUG); - $item['network'] = Protocol::DFRN; - Logger::notice("Set network to " . $item["network"] . " for " . $item["uri"], Logger::DEBUG); + Logger::notice('Missing network, setting to {network}.', [ + 'uri' => $item["uri"], + 'network' => $item['network'], + 'callstack' => System::callstack() + ]); } // Checking if there is already an item with the same guid $condition = ['guid' => $item['guid'], 'network' => $item['network'], 'uid' => $item['uid']]; if (self::exists($condition)) { - Logger::notice('Found already existing item with guid '.$item['guid'].' for user '.$item['uid'].' on network '.$item['network']); + Logger::notice('Found already existing item', [ + 'guid' => $item['guid'], + 'uid' => $item['uid'], + 'network' => $item['network'] + ]); return 0; } diff --git a/src/Model/Term.php b/src/Model/Term.php index 5266627aca..36876368df 100644 --- a/src/Model/Term.php +++ b/src/Model/Term.php @@ -204,6 +204,8 @@ class Term $type = self::HASHTAG; $term = $tag; $link = ''; + + Logger::notice('Unknown term type', ['tag' => $tag]); } if (DBA::exists('term', ['uid' => $item['uid'], 'otype' => self::OBJECT_TYPE_POST, 'oid' => $item_id, 'term' => $term, 'type' => $type])) { diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index b77df1fae8..2661444e0c 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -276,7 +276,7 @@ class Processor /// @todo What to do with $activity['context']? if (($item['gravity'] != GRAVITY_PARENT) && !Item::exists(['uri' => $item['thr-parent']])) { - Logger::log('Parent ' . $item['thr-parent'] . ' not found, message will be discarded.', Logger::DEBUG); + Logger::info('Parent not found, message will be discarded.', ['thr-parent' => $item['thr-parent']]); return; } -- 2.39.5