Skip to content

Commit

Permalink
feat: add additional logging for database errors
Browse files Browse the repository at this point in the history
including the stack trace of the current database transaction

Signed-off-by: Robin Appelman <robin@icewind.nl>
  • Loading branch information
icewind1991 committed Jul 25, 2024
1 parent 4ec3ccd commit c95206a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 4 deletions.
51 changes: 47 additions & 4 deletions lib/private/DB/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ class Connection extends PrimaryReadReplicaConnection {
/** @var array<string, int> */
protected $tableDirtyWrites = [];

protected bool $logDbException = false;
private ?array $transactionBacktrace = null;

protected bool $logRequestId;
protected string $requestId;

Expand Down Expand Up @@ -124,6 +127,7 @@ public function __construct(
$this->logger = \OC::$server->get(LoggerInterface::class);

$this->logRequestId = $this->systemConfig->getValue('db.log_request_id', false);
$this->logDbException = $this->systemConfig->getValue('db.log_exceptions', false);
$this->requestId = Server::get(IRequestId::class)->getId();

/** @var \OCP\Profiler\IProfiler */
Expand Down Expand Up @@ -325,7 +329,12 @@ public function executeQuery(string $sql, array $params = [], $types = [], ?Quer
$sql = $this->finishQuery($sql);
$this->queriesExecuted++;
$this->logQueryToFile($sql);
return parent::executeQuery($sql, $params, $types, $qcp);
try {
return parent::executeQuery($sql, $params, $types, $qcp);
} catch (\Exception $e) {
$this->logDatabaseException($e);
throw $e;
}
}

/**
Expand Down Expand Up @@ -370,7 +379,12 @@ public function executeStatement($sql, array $params = [], array $types = []): i
$sql = $this->finishQuery($sql);
$this->queriesExecuted++;
$this->logQueryToFile($sql);
return (int)parent::executeStatement($sql, $params, $types);
try {
return (int)parent::executeStatement($sql, $params, $types);
} catch (\Exception $e) {
$this->logDatabaseException($e);
throw $e;
}
}

protected function logQueryToFile(string $sql): void {
Expand Down Expand Up @@ -437,11 +451,21 @@ public function realLastInsertId($seqName = null) {
* @deprecated 15.0.0 - use unique index and "try { $db->insert() } catch (UniqueConstraintViolationException $e) {}" instead, because it is more reliable and does not have the risk for deadlocks - see https://github.com/nextcloud/server/pull/12371
*/
public function insertIfNotExist($table, $input, ?array $compare = null) {
return $this->adapter->insertIfNotExist($table, $input, $compare);
try {
return $this->adapter->insertIfNotExist($table, $input, $compare);
} catch (\Exception $e) {
$this->logDatabaseException($e);
throw $e;
}
}

public function insertIgnoreConflict(string $table, array $values) : int {
return $this->adapter->insertIgnoreConflict($table, $values);
try {
return $this->adapter->insertIgnoreConflict($table, $values);
} catch (\Exception $e) {
$this->logDatabaseException($e);
throw $e;
}
}

private function getType($value) {
Expand Down Expand Up @@ -700,6 +724,7 @@ private function getMigrator() {

public function beginTransaction() {
if (!$this->inTransaction()) {
$this->transactionBacktrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
$this->transactionActiveSince = microtime(true);
}
return parent::beginTransaction();
Expand All @@ -709,6 +734,7 @@ public function commit() {
$result = parent::commit();
if ($this->getTransactionNestingLevel() === 0) {
$timeTook = microtime(true) - $this->transactionActiveSince;
$this->transactionBacktrace = null;
$this->transactionActiveSince = null;
if ($timeTook > 1) {
$this->logger->debug('Transaction took ' . $timeTook . 's', ['exception' => new \Exception('Transaction took ' . $timeTook . 's')]);
Expand All @@ -721,6 +747,7 @@ public function rollBack() {
$result = parent::rollBack();
if ($this->getTransactionNestingLevel() === 0) {
$timeTook = microtime(true) - $this->transactionActiveSince;
$this->transactionBacktrace = null;
$this->transactionActiveSince = null;
if ($timeTook > 1) {
$this->logger->debug('Transaction rollback took longer than 1s: ' . $timeTook, ['exception' => new \Exception('Long running transaction rollback')]);
Expand Down Expand Up @@ -750,4 +777,20 @@ private function reconnectIfNeeded(): void {
private function getConnectionName(): string {
return $this->isConnectedToPrimary() ? 'primary' : 'replica';
}

/**
* Log a database exception if enabled
*
* @param \Exception $exception
* @return void
*/
public function logDatabaseException(\Exception $exception): void {
if ($this->logDbException) {
if ($exception instanceof Exception\UniqueConstraintViolationException) {
$this->logger->info($exception->getMessage(), ['exception' => $exception, 'transaction' => $this->transactionBacktrace]);
} else {
$this->logger->error($exception->getMessage(), ['exception' => $exception, 'transaction' => $this->transactionBacktrace]);
}
}
}
}
4 changes: 4 additions & 0 deletions lib/private/DB/ConnectionAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,8 @@ public function getDatabaseProvider(): string {
throw new \Exception('Database ' . $platform::class . ' not supported');
}
}

public function logDatabaseException(\Exception $exception) {
$this->inner->logDatabaseException($exception);
}
}

0 comments on commit c95206a

Please sign in to comment.