Fix remove duplicate metrics from stats usage#1
Conversation
…-metrics-from-stats-usage
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaStats System: API Controllers: Updated Storage Metrics: Simplified Configuration: Reduced Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The split between Risks: The removal of real-time metric updates creates a dependency on the Merge StatusNOT MERGEABLE — PR Score 30/100, below threshold (50)
|
Greptile SummaryThis PR refactors how usage metrics are sourced and written: resource-count and storage metrics are now exclusively computed by the
Confidence Score: 3/5Not safe to merge as-is: the per-function usage endpoint returns wrong time-series data for The app/controllers/api/functions.php (wrong metric array indices at lines 723-724) and .env (polling interval set to 15 s instead of the original 3600 s) Important Files Changed
Reviews (1): Last reviewed commit: "improve stats fetching from db" | Re-trigger Greptile |
Code Quality Report — test-org-codity/appwrite · PR #1Scanned: 2026-05-20 18:41 UTC | Score: 33/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-002]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
.env |
0 | 0 | 0 | 1 | 1 |
app/controllers/api/databases.php |
0 | 2 | 3 | 19 | 24 |
app/controllers/api/functions.php |
0 | 0 | 0 | 19 | 19 |
app/controllers/api/project.php |
0 | 0 | 0 | 2 | 2 |
app/controllers/api/storage.php |
0 | 0 | 0 | 1 | 1 |
src/Appwrite/Platform/Workers/StatsResources.php |
0 | 0 | 0 | 2 | 2 |
src/Appwrite/Platform/Workers/StatsUsageDump.php |
0 | 0 | 0 | 6 | 6 |
src/Appwrite/Utopia/Response/Model/File.php |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaMetrics Pipeline: Consolidated all resource counting into Database Layer: Added Storage Tracking: Replaced complex Configuration: Reduced Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The split between Risks: The 15-second aggregation interval is intentionally aggressive for faster feedback, but may increase load on Merge StatusNOT MERGEABLE — PR Score 21/100, below threshold (50)
|
![codity-dm[bot]](https://avatars.githubusercontent.com/u/234452856?s=60&v=4){kind=small}
| Query::equal('metric', [$deploymentMetric]), | ||
| Query::equal('period', ['inf']) | ||
| ]); | ||
| $deploymentValue = $dbForProject->getDocument('stats', md5('_inf_' . $deploymentMetric)); |
There was a problem hiding this comment.
Project usage now reads function deployment storage from the project DB even though this PR moved deployment/build storage metrics to the logs DB, so calling the project usage endpoint after creating a function deployment returns 0 for that function's storage.
Also reported at: app/controllers/api/project.php L212
Read both deployment and build storage metrics from $dbForLogs here (and use getAttribute('value', 0) consistently), because StatsUsageDump/StatsResources now populate those metrics in the logs DB instead of the project DB.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/project.php
Lines: 225-225
Issue Type: functional-high
Severity: high
Issue Description:
Project usage now reads function deployment storage from the project DB even though this PR moved deployment/build storage metrics to the logs DB, so calling the project usage endpoint after creating a function deployment returns 0 for that function's storage.
_Also reported at: `app/controllers/api/project.php` L212_
Current Code:
$deploymentValue = $dbForProject->getDocument('stats', md5('_inf_' . $deploymentMetric));
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| ->action(function (string $databaseId, string $range, string $collectionId, Response $response, Database $dbForProject, Database $dbForLogs) { | ||
|
|
||
| $database = $dbForProject->getDocument('databases', $databaseId); |
There was a problem hiding this comment.
The collection usage endpoint dereferences $database->getInternalId() before verifying the database exists, so add a database emptiness check before building collection paths.
Suggested fix
->action(function (string $databaseId, string $range, string $collectionId, Response $response, Database $dbForProject, Database $dbForLogs) {
$database = $dbForProject->getDocument('databases', $databaseId);
if ($database->isEmpty()) {
throw new Exception(Exception::DATABASE_NOT_FOUND);
}
$collectionDocument = $dbForProject->getDocument('database_' . $database->getInternalId(), $collectionId);Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/databases.php
Lines: 4426-4428
Issue Type: functional-high
Severity: high
Issue Description:
The collection usage endpoint dereferences `$database->getInternalId()` before verifying the database exists, so add a database emptiness check before building collection paths.
Current Code:
->action(function (string $databaseId, string $range, string $collectionId, Response $response, Database $dbForProject, Database $dbForLogs) {
$database = $dbForProject->getDocument('databases', $databaseId);
$collectionDocument = $dbForProject->getDocument('database_' . $database->getInternalId(), $collectionId);
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| 'builds' => $usage[$metrics[2]]['data'], | ||
| 'buildsStorage' => $usage[$metrics[3]]['data'], |
There was a problem hiding this comment.
The per-function builds and buildsStorage fields now point at execution and MB-seconds metrics, so map them to the logsDBMetrics entries introduced above.
Suggested fix
'builds' => $usage[$logsDBMetrics[2]]['data'],
'buildsStorage' => $usage[$logsDBMetrics[3]]['data'],Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/functions.php
Lines: 723-724
Issue Type: functional-high
Severity: high
Issue Description:
The per-function builds and buildsStorage fields now point at execution and MB-seconds metrics, so map them to the logsDBMetrics entries introduced above.
Current Code:
'builds' => $usage[$metrics[2]]['data'],
'buildsStorage' => $usage[$metrics[3]]['data'],
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| Authorization::skip(function () use ($dbForLogs, $bucket, $days, $metrics, &$stats) { | ||
| foreach ($metrics as $metric) { | ||
| $db = ($metric === str_replace('{bucketInternalId}', $bucket->getInternalId(), METRIC_BUCKET_ID_FILES_IMAGES_TRANSFORMED)) | ||
| ? $dbForLogs | ||
| : $dbForProject; | ||
|
|
||
| $result = $db->findOne('stats', [ | ||
| Query::equal('metric', [$metric]), | ||
| Query::equal('period', ['inf']) | ||
| ]); | ||
|
|
||
| $result = $dbForLogs->getDocument('stats', md5('_inf_' . $metric)); | ||
| $stats[$metric]['total'] = $result['value'] ?? 0; | ||
|
|
||
| $limit = $days['limit']; | ||
| $period = $days['period']; | ||
| $results = $db->find('stats', [ | ||
| $results = $dbForLogs->find('stats', [ |
There was a problem hiding this comment.
This route now reads bucket file and storage metrics from the logs database, but those metrics are still produced in the project database here, so keep using the original split database selection for bucket metrics.
Suggested fix
Authorization::skip(function () use ($dbForProject, $dbForLogs, $bucket, $days, $metrics, &$stats) {
foreach ($metrics as $metric) {
$db = ($metric === str_replace('{bucketInternalId}', $bucket->getInternalId(), METRIC_BUCKET_ID_FILES_IMAGES_TRANSFORMED))
? $dbForLogs
: $dbForProject;
$result = $db->getDocument('stats', md5('_inf_' . $metric));
$stats[$metric]['total'] = $result['value'] ?? 0;
$limit = $days['limit'];
$period = $days['period'];
$results = $db->find('stats', [Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/storage.php
Lines: 1890-1897
Issue Type: functional-high
Severity: high
Issue Description:
This route now reads bucket file and storage metrics from the logs database, but those metrics are still produced in the project database here, so keep using the original split database selection for bucket metrics.
Current Code:
Authorization::skip(function () use ($dbForLogs, $bucket, $days, $metrics, &$stats) {
foreach ($metrics as $metric) {
$result = $dbForLogs->getDocument('stats', md5('_inf_' . $metric));
$stats[$metric]['total'] = $result['value'] ?? 0;
$limit = $days['limit'];
$period = $days['period'];
$results = $dbForLogs->find('stats', [
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 53.7sSecurity scan powered by Codity.ai |
Dependency vulnerability scanning
View vulnerability details (8 items)1. google/protobuf 4.29.3 CVE: GHSA-p2gh-cfq4-4wjc
2. phpseclib/phpseclib 3.0.43 CVE: GHSA-3qpq-r242-jqj7
3. phpseclib/phpseclib 3.0.43 CVE: GHSA-94g3-g5v7-q4jg
4. phpseclib/phpseclib 3.0.43 CVE: GHSA-r854-jrxh-36qx
5. webonyx/graphql-php 14.11.10 CVE: GHSA-68jq-c3rv-pcrr
6. webonyx/graphql-php 14.11.10 CVE: GHSA-fc86-6rv6-2jpm
7. webonyx/graphql-php 14.11.10 CVE: GHSA-r7cg-qjjm-xhqq
8. phpunit/phpunit 9.6.22 CVE: GHSA-vvj3-c3rp-c85p
Powered by Codity.ai · Docs |
License Compliance Scan
All licenses are low-risk and compliant Medium Risk Licenses - 3 packagesLGPL-2.1-only (1 packages):
LGPL-2.1-or-later (1 packages):
LGPL-3.0-or-later (1 packages):
Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/appwrite · PR #1Scanned: 2026-05-20 19:21 UTC | Score: 35/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-002]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
.env |
0 | 0 | 0 | 1 | 1 |
app/controllers/api/databases.php |
0 | 1 | 3 | 19 | 23 |
app/controllers/api/functions.php |
0 | 0 | 0 | 19 | 19 |
app/controllers/api/project.php |
0 | 0 | 0 | 2 | 2 |
app/controllers/api/storage.php |
0 | 0 | 0 | 1 | 1 |
src/Appwrite/Platform/Workers/StatsResources.php |
0 | 0 | 0 | 2 | 2 |
src/Appwrite/Platform/Workers/StatsUsageDump.php |
0 | 0 | 0 | 6 | 6 |
src/Appwrite/Utopia/Response/Model/File.php |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaConfiguration: Reduced API Controllers: Updated usage endpoints in Workers: Removed duplicate metric recording from Data Model: Added Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The split between Risks:
Merge StatusNOT MERGEABLE — PR Score 20/100, below threshold (50)
|
| _APP_MAINTENANCE_RETENTION_AUDIT=1209600 | ||
| _APP_USAGE_AGGREGATION_INTERVAL=30 | ||
| _APP_STATS_RESOURCES_INTERVAL=3600 | ||
| _APP_STATS_RESOURCES_INTERVAL=15 |
There was a problem hiding this comment.
_APP_STATS_RESOURCES_INTERVAL was changed from 3600 (1 hour) to 15 (15 seconds). This is clearly a debug value that was not reverted before merging. In production, the StatsResources worker will now run every 15 seconds instead of every hour, causing a massive and continuous load on every project database. The CI workflow correctly overrides this to 5 for tests, but that override does not affect production deployments which read from this file.
| _APP_STATS_RESOURCES_INTERVAL=15 | |
| _APP_STATS_RESOURCES_INTERVAL=3600 |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert ini developer with deep knowledge of security, performance, and best practices.
### Context
File: .env
Lines: 90-90
Issue Type: performance-critical
Severity: critical
Issue Description:
`_APP_STATS_RESOURCES_INTERVAL` was changed from `3600` (1 hour) to `15` (15 seconds). This is clearly a debug value that was not reverted before merging. In production, the `StatsResources` worker will now run every 15 seconds instead of every hour, causing a massive and continuous load on every project database. The CI workflow correctly overrides this to `5` for tests, but that override does not affect production deployments which read from this file.
Current Code:
_APP_STATS_RESOURCES_INTERVAL=15
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow ini best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| $dbForLogs->createOrUpdateDocumentsWithIncrease( | ||
| 'stats', | ||
| 'value', | ||
| [$document] | ||
| $documents | ||
| ); | ||
| Console::success('Usage logs pushed to Logs DB'); | ||
| } catch (\Throwable $th) { |
There was a problem hiding this comment.
The previous implementation of writeToLogsDB filtered out resource-level metrics (databases, collections, documents, buckets, files, deployments, builds, storage) before writing to logsDB, because those were handled by the periodic StatsResources worker. This PR removes all filtering, meaning any residual metric from the queue (e.g. leftover keys from in-flight messages during a rolling deploy) will now be double-written to logsDB, potentially corrupting cumulative counts. The skip-list was a deliberate guard and should either be retained or replaced with an explicit allowlist.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: src/Appwrite/Platform/Workers/StatsUsageDump.php
Lines: 142-148
Issue Type: robustness-medium
Severity: medium
Issue Description:
The previous implementation of `writeToLogsDB` filtered out resource-level metrics (databases, collections, documents, buckets, files, deployments, builds, storage) before writing to logsDB, because those were handled by the periodic `StatsResources` worker. This PR removes all filtering, meaning any residual metric from the queue (e.g. leftover keys from in-flight messages during a rolling deploy) will now be double-written to logsDB, potentially corrupting cumulative counts. The skip-list was a deliberate guard and should either be retained or replaced with an explicit allowlist.
Current Code:
/** @var \Utopia\Database\Database $dbForLogs*/
$dbForLogs = call_user_func($this->getLogsDB, $project);
try {
$dbForLogs->createOrUpdateDocumentsWithIncrease(
'stats',
'value',
$documents
);
Console::success('Usage logs pushed to Logs DB');
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
In the per-function usage response, 'builds' is mapped to $usage[$metrics[2]]['data'] and 'buildsStorage' to $usage[$metrics[3]]['data']. After the refactor, $metrics only contains compute/execution metrics: [0]=BUILDS_COMPUTE, [1]=EXECUTIONS, [2]=EXECUTIONS_COMPUTE, [3]=BUILDS_MB_SECONDS. So $metrics[2] is execution-compute data and $metrics[3] is builds-MB-seconds data — both completely wrong fields. The build count and build storage time-series data should be sourced from $logsDBMetrics[2] (METRIC_RESOURCE_TYPE_ID_BUILDS) and $logsDBMetrics[3] (METRIC_RESOURCE_TYPE_ID_BUILDS_STORAGE) respectively, exactly as the *Total counterparts above them correctly do.
Suggested fix
'builds' => $usage[$logsDBMetrics[2]]['data'],
'buildsStorage' => $usage[$logsDBMetrics[3]]['data'],Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/functions.php
Lines: 734-735
Issue Type: functional-high
Severity: high
Issue Description:
In the per-function usage response, `'builds'` is mapped to `$usage[$metrics[2]]['data']` and `'buildsStorage'` to `$usage[$metrics[3]]['data']`. After the refactor, `$metrics` only contains compute/execution metrics: `[0]=BUILDS_COMPUTE, [1]=EXECUTIONS, [2]=EXECUTIONS_COMPUTE, [3]=BUILDS_MB_SECONDS`. So `$metrics[2]` is execution-compute data and `$metrics[3]` is builds-MB-seconds data — both completely wrong fields. The build count and build storage time-series data should be sourced from `$logsDBMetrics[2]` (`METRIC_RESOURCE_TYPE_ID_BUILDS`) and `$logsDBMetrics[3]` (`METRIC_RESOURCE_TYPE_ID_BUILDS_STORAGE`) respectively, exactly as the `*Total` counterparts above them correctly do.
Current Code:
'builds' => $usage[$metrics[2]]['data'],
'buildsStorage' => $usage[$metrics[3]]['data'],
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
The per-function deployment and build storage breakdown still reads from $dbForProject via getDocument. However, $metricsFromLogsDB explicitly lists METRIC_DEPLOYMENTS_STORAGE and METRIC_BUILDS_STORAGE as metrics that now live in $dbForLogs, and StatsResources::countForFunctions had its $dbForLogs parameter removed — suggesting these per-function storage metrics are now written to logsDB by the periodic worker. If that is the case, reading them from $dbForProject will always return 0, silently producing a broken breakdown table.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/project.php
Lines: 225-232
Issue Type: functional-medium
Severity: medium
Issue Description:
The per-function deployment and build storage breakdown still reads from `$dbForProject` via `getDocument`. However, `$metricsFromLogsDB` explicitly lists `METRIC_DEPLOYMENTS_STORAGE` and `METRIC_BUILDS_STORAGE` as metrics that now live in `$dbForLogs`, and `StatsResources::countForFunctions` had its `$dbForLogs` parameter removed — suggesting these per-function storage metrics are now written to logsDB by the periodic worker. If that is the case, reading them from `$dbForProject` will always return 0, silently producing a broken breakdown table.
Current Code:
$deploymentValue = $dbForProject->getDocument('stats', md5('_inf_' . $deploymentMetric));
$buildMetric = str_replace(['{functionInternalId}'], [$function->getInternalId()], METRIC_FUNCTION_ID_BUILDS_STORAGE);
$buildValue = $dbForProject->getDocument('stats', md5('_inf_' . $buildMetric));
$value = ($buildValue['value'] ?? 0) + ($deploymentValue['value'] ?? 0);
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 46.1sSecurity scan powered by Codity.ai |
Dependency vulnerability scanning
Top 10 Vulnerabilities (13 total found)1. google/protobuf 4.29.3 CVE: GHSA-p2gh-cfq4-4wjc
2. phpseclib/phpseclib 3.0.43 CVE: GHSA-3qpq-r242-jqj7
3. phpseclib/phpseclib 3.0.43 CVE: GHSA-94g3-g5v7-q4jg
4. phpseclib/phpseclib 3.0.43 CVE: GHSA-r854-jrxh-36qx
5. webonyx/graphql-php 14.11.10 CVE: GHSA-68jq-c3rv-pcrr
6. webonyx/graphql-php 14.11.10 CVE: GHSA-fc86-6rv6-2jpm
7. webonyx/graphql-php 14.11.10 CVE: GHSA-r7cg-qjjm-xhqq
8. phpunit/phpunit 9.6.22 CVE: GHSA-vvj3-c3rp-c85p
9. twig/twig 3.14.2 CVE: GHSA-24x9-r6q4-q93w
10. twig/twig 3.14.2 CVE: GHSA-4j38-f5cw-54h7
3 more vulnerabilities not shown. Update dependencies to fix these issues. Powered by Codity.ai · Docs |
License Compliance Scan
All licenses are low-risk and compliant Medium Risk Licenses - 3 packagesLGPL-2.1-only (1 packages):
LGPL-2.1-or-later (1 packages):
LGPL-3.0-or-later (1 packages):
Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/appwrite · PR #1Scanned: 2026-05-24 10:34 UTC | Score: 33/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-002]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
.env |
0 | 0 | 0 | 1 | 1 |
app/controllers/api/databases.php |
0 | 2 | 3 | 19 | 24 |
app/controllers/api/functions.php |
0 | 0 | 0 | 19 | 19 |
app/controllers/api/project.php |
0 | 0 | 0 | 2 | 2 |
app/controllers/api/storage.php |
0 | 0 | 0 | 1 | 1 |
src/Appwrite/Platform/Workers/StatsResources.php |
0 | 0 | 0 | 2 | 2 |
src/Appwrite/Platform/Workers/StatsUsageDump.php |
0 | 0 | 0 | 6 | 6 |
src/Appwrite/Utopia/Response/Model/File.php |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaStats System: Replaced scattered metric increments with centralized collection in Database API: Updated usage endpoints to query correct database per metric type. Removed inline storage metric tracking from create/delete operations. Functions API: Separated deployment/builds/storage metrics (logs DB) from compute/execution metrics (project DB). File Model: Added Configuration: Reduced Files Changed
Review Focus Areas
ArchitectureDesign Decisions: The split between Scalability & Extensibility: Batch processing in Risks (Intentional): Eventual consistency for resource counts (acceptable for stats). Risks (Unintentional): Missing metric coverage if Merge StatusNOT MERGEABLE — PR Score 21/100, below threshold (50)
|
| _APP_USAGE_AGGREGATION_INTERVAL=30 | ||
| _APP_STATS_RESOURCES_INTERVAL=3600 | ||
| _APP_STATS_RESOURCES_INTERVAL=15 | ||
| _APP_MAINTENANCE_RETENTION_USAGE_HOURLY=8640000 |
There was a problem hiding this comment.
_APP_STATS_RESOURCES_INTERVAL was changed from 3600 (1 hour) to 15 seconds in the default .env shipped with the repository. The CI workflow correctly overrides this to 5 for tests, but any deployment that sources the bundled .env as a base will run the resource-counting worker every 15 seconds, causing severe unnecessary database load in production. The original value of 3600 should be restored.
| _APP_MAINTENANCE_RETENTION_USAGE_HOURLY=8640000 | |
| _APP_STATS_RESOURCES_INTERVAL=3600 |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: .env
Lines: 91-91
Issue Type: functional-high
Severity: high
Issue Description:
`_APP_STATS_RESOURCES_INTERVAL` was changed from `3600` (1 hour) to `15` seconds in the default `.env` shipped with the repository. The CI workflow correctly overrides this to `5` for tests, but any deployment that sources the bundled `.env` as a base will run the resource-counting worker every 15 seconds, causing severe unnecessary database load in production. The original value of `3600` should be restored.
Current Code:
_APP_STATS_RESOURCES_INTERVAL=15
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| 'deploymentsStorage' => $usage[$logsDBMetrics[1]]['data'], | ||
| 'builds' => $usage[$metrics[2]]['data'], |
There was a problem hiding this comment.
In the per-function usage endpoint the 'builds' and 'buildsStorage' data fields were not updated to reference $logsDBMetrics after the metric arrays were split. They still reference $metrics[2] (now METRIC_FUNCTION_ID_EXECUTIONS_COMPUTE) and $metrics[3] (now METRIC_FUNCTION_ID_BUILDS_MB_SECONDS) respectively, so the API returns execution-compute time series where callers expect build-count data, and MB-seconds time series where callers expect build-storage data. Both fields will silently return completely unrelated numbers.
Suggested fix
'builds' => $usage[$logsDBMetrics[2]]['data'],
'buildsStorage' => $usage[$logsDBMetrics[3]]['data'],Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/functions.php
Lines: 722-723
Issue Type: functional-critical
Severity: critical
Issue Description:
In the per-function usage endpoint the `'builds'` and `'buildsStorage'` data fields were not updated to reference `$logsDBMetrics` after the metric arrays were split. They still reference `$metrics[2]` (now `METRIC_FUNCTION_ID_EXECUTIONS_COMPUTE`) and `$metrics[3]` (now `METRIC_FUNCTION_ID_BUILDS_MB_SECONDS`) respectively, so the API returns execution-compute time series where callers expect build-count data, and MB-seconds time series where callers expect build-storage data. Both fields will silently return completely unrelated numbers.
Current Code:
'builds' => $usage[$metrics[2]]['data'],
'buildsStorage' => $usage[$metrics[3]]['data'],
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| $functionsStorageBreakdown = array_map(function ($function) use ($dbForProject) { | ||
| $id = $function->getId(); | ||
| $name = $function->getAttribute('name'); | ||
| $deploymentMetric = str_replace(['{resourceType}', '{resourceInternalId}'], ['functions', $function->getInternalId()], METRIC_FUNCTION_ID_DEPLOYMENTS_STORAGE); | ||
| $deploymentValue = $dbForProject->findOne('stats', [ | ||
| Query::equal('metric', [$deploymentMetric]), | ||
| Query::equal('period', ['inf']) | ||
| ]); | ||
| $deploymentValue = $dbForProject->getDocument('stats', md5('_inf_' . $deploymentMetric)); | ||
|
|
||
| $buildMetric = str_replace(['{functionInternalId}'], [$function->getInternalId()], METRIC_FUNCTION_ID_BUILDS_STORAGE); | ||
| $buildValue = $dbForProject->findOne('stats', [ | ||
| Query::equal('metric', [$buildMetric]), | ||
| Query::equal('period', ['inf']) | ||
| ]); | ||
| $buildValue = $dbForProject->getDocument('stats', md5('_inf_' . $buildMetric)); | ||
|
|
||
| $value = ($buildValue['value'] ?? 0) + ($deploymentValue['value'] ?? 0); |
There was a problem hiding this comment.
The per-function deployment+build storage breakdown reads both METRIC_FUNCTION_ID_DEPLOYMENTS_STORAGE and METRIC_FUNCTION_ID_BUILDS_STORAGE from $dbForProject. These are storage metrics that the rest of the PR migrates to $dbForLogs (the functions usage endpoint moves them to $logsDBMetrics). Additionally, the value is computed with raw array access $buildValue['value'] ?? 0 on Document objects that now come from getDocument — while Document's ArrayAccess implementation does delegate to getAttribute, the pattern is inconsistent and masks the primary bug: both lookups should target $dbForLogs. The result is that all per-function storage totals in the project overview will be 0.
Suggested fix
$deploymentValue = $dbForLogs->getDocument('stats', md5('_inf_' . $deploymentMetric));
$buildMetric = str_replace(['{functionInternalId}'], [$function->getInternalId()], METRIC_FUNCTION_ID_BUILDS_STORAGE);
$buildValue = $dbForLogs->getDocument('stats', md5('_inf_' . $buildMetric));
$value = $buildValue->getAttribute('value', 0) + $deploymentValue->getAttribute('value', 0);Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/project.php
Lines: 221-230
Issue Type: functional-high
Severity: high
Issue Description:
The per-function deployment+build storage breakdown reads both `METRIC_FUNCTION_ID_DEPLOYMENTS_STORAGE` and `METRIC_FUNCTION_ID_BUILDS_STORAGE` from `$dbForProject`. These are storage metrics that the rest of the PR migrates to `$dbForLogs` (the functions usage endpoint moves them to `$logsDBMetrics`). Additionally, the value is computed with raw array access `$buildValue['value'] ?? 0` on Document objects that now come from `getDocument` — while Document's `ArrayAccess` implementation does delegate to `getAttribute`, the pattern is inconsistent and masks the primary bug: both lookups should target `$dbForLogs`. The result is that all per-function storage totals in the project overview will be `0`.
Current Code:
$deploymentValue = $dbForProject->getDocument('stats', md5('_inf_' . $deploymentMetric));
$buildMetric = str_replace(['{functionInternalId}'], [$function->getInternalId()], METRIC_FUNCTION_ID_BUILDS_STORAGE);
$buildValue = $dbForProject->getDocument('stats', md5('_inf_' . $buildMetric));
$value = ($buildValue['value'] ?? 0) + ($deploymentValue['value'] ?? 0);
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
There was a problem hiding this comment.
The per-database storage breakdown in the project usage action still queries $dbForProject for METRIC_DATABASE_ID_STORAGE, but this metric was migrated to $dbForLogs throughout this PR (e.g. the databases usage endpoint now reads it exclusively from $dbForLogs). Querying the wrong database will always return an empty Document, so every database's storage contribution is reported as 0 in the project-level storage breakdown.
Suggested fix
$result = $dbForLogs->getDocument('stats', md5('_inf_' . $metric));
return [
'resourceId' => $id,
'name' => $name,
'value' => $result->getAttribute('value', 0),
];Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert php developer with deep knowledge of security, performance, and best practices.
### Context
File: app/controllers/api/project.php
Lines: 207-212
Issue Type: functional-high
Severity: high
Issue Description:
The per-database storage breakdown in the project usage action still queries `$dbForProject` for `METRIC_DATABASE_ID_STORAGE`, but this metric was migrated to `$dbForLogs` throughout this PR (e.g. the databases usage endpoint now reads it exclusively from `$dbForLogs`). Querying the wrong database will always return an empty Document, so every database's storage contribution is reported as `0` in the project-level storage breakdown.
Current Code:
$result = $dbForProject->getDocument('stats', md5('_inf_' . $metric));
return [
'resourceId' => $id,
'name' => $name,
'value' => $result->getAttribute('value', 0),
];
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow php best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
No critical security issues detected Scan completed in 38.9sSecurity scan powered by Codity.ai |
Dependency vulnerability scanning
Top 10 Vulnerabilities (13 total found)1. google/protobuf 4.29.3 CVE: GHSA-p2gh-cfq4-4wjc
2. phpseclib/phpseclib 3.0.43 CVE: GHSA-3qpq-r242-jqj7
3. phpseclib/phpseclib 3.0.43 CVE: GHSA-94g3-g5v7-q4jg
4. phpseclib/phpseclib 3.0.43 CVE: GHSA-r854-jrxh-36qx
5. webonyx/graphql-php 14.11.10 CVE: GHSA-68jq-c3rv-pcrr
6. webonyx/graphql-php 14.11.10 CVE: GHSA-fc86-6rv6-2jpm
7. webonyx/graphql-php 14.11.10 CVE: GHSA-r7cg-qjjm-xhqq
8. phpunit/phpunit 9.6.22 CVE: GHSA-vvj3-c3rp-c85p
9. twig/twig 3.14.2 CVE: GHSA-24x9-r6q4-q93w
10. twig/twig 3.14.2 CVE: GHSA-4j38-f5cw-54h7
3 more vulnerabilities not shown. Update dependencies to fix these issues. Powered by Codity.ai · Docs |
License Compliance Scan
All licenses are low-risk and compliant Medium Risk Licenses - 3 packagesLGPL-2.1-only (1 packages):
LGPL-2.1-or-later (1 packages):
LGPL-3.0-or-later (1 packages):
Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/appwrite · PR #1Scanned: 2026-05-24 10:51 UTC | Score: 35/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-002]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
.env |
0 | 0 | 0 | 1 | 1 |
app/controllers/api/databases.php |
0 | 1 | 3 | 19 | 23 |
app/controllers/api/functions.php |
0 | 0 | 0 | 19 | 19 |
app/controllers/api/project.php |
0 | 0 | 0 | 2 | 2 |
app/controllers/api/storage.php |
0 | 0 | 0 | 1 | 1 |
src/Appwrite/Platform/Workers/StatsResources.php |
0 | 0 | 0 | 2 | 2 |
src/Appwrite/Platform/Workers/StatsUsageDump.php |
0 | 0 | 0 | 6 | 6 |
src/Appwrite/Utopia/Response/Model/File.php |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
2 participants
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist