Skip to content

Add realtime logger#10871

Merged
abnegate merged 2 commits into1.8.xfrom
feat-custom-realtime-logger
Nov 27, 2025
Merged

Add realtime logger#10871
abnegate merged 2 commits into1.8.xfrom
feat-custom-realtime-logger

Conversation

@abnegate
Copy link
Copy Markdown
Member

@abnegate abnegate commented Nov 26, 2025

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

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 26, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

A new registry entry realtimeLogger is added in app/init/registers.php that creates a separate logger instance using _APP_LOGGING_CONFIG_REALTIME with fallback to _APP_LOGGING_CONFIG. It constructs provider DSNs, maps provider-specific configuration, validates the provider, instantiates provider adapters (sentry, logowl, raygun, appsignal), and returns a Logger or handles adapter creation failures. app/realtime.php now retrieves the logger from realtimeLogger instead of logger. An environment variable _APP_LOGGING_CONFIG_REALTIME was added to .env and to the realtime service in docker-compose.yml.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect app/init/registers.php realtimeLogger block for parity with the existing logger flow, correct DSN parsing, and consistent error handling.
  • Verify fallback to _APP_LOGGING_CONFIG when _APP_LOGGING_CONFIG_REALTIME is unset.
  • Check each adapter branch (sentry, logowl, raygun, appsignal) for correct instantiation and failure handling.
  • Confirm app/realtime.php uses realtimeLogger only where intended and no other places still reference logger.
  • Validate .env and docker-compose.yml entries correctly expose _APP_LOGGING_CONFIG_REALTIME to the realtime service.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description consists entirely of the default contribution template with empty placeholder sections, providing no substantive information about the changeset or its rationale. Complete the template sections, particularly 'What does this PR do?' and 'Test Plan', to explain the implementation details, motivation, and verification steps for this change.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add realtime logger' directly and clearly describes the main change—adding a new realtime logger service across multiple files (registers.php, realtime.php, .env, docker-compose.yml).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-custom-realtime-logger
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04dd0f8 and 63652cc.

📒 Files selected for processing (2)
  • .env (1 hunks)
  • docker-compose.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T01:20:14.364Z 
Learnt from: stnguyen90
Repo: appwrite/appwrite PR: 10119
File: app/controllers/api/account.php:1226-1232
Timestamp: 2025-07-08T01:20:14.364Z
Learning: In Appwrite, `_APP_DOMAIN` is a required environment variable that must always be set for the system to function properly.

Applied to files:

  • .env
  • docker-compose.yml
🪛 dotenv-linter (4.0.0)
.env

[warning] 106-106: [UnorderedKey] The _APP_LOGGING_CONFIG_REALTIME key should go before the _APP_MAINTENANCE_INTERVAL key

(UnorderedKey)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Health)
  • GitHub Check: E2E Service Test (Tokens)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (VCS)
  • GitHub Check: E2E Service Test (Teams)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (GraphQL)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Avatars)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: Unit Test
  • GitHub Check: Benchmark
  • GitHub Check: scan
🔇 Additional comments (2)
.env (1)

106-106: Verify alphabetical ordering of environment variables.

The static analysis tool (dotenv-linter) flagged that _APP_LOGGING_CONFIG_REALTIME should be positioned before _APP_MAINTENANCE_INTERVAL to maintain alphabetical key ordering. While the logical grouping with _APP_LOGGING_CONFIG at line 105 makes sense, the current placement may violate the file's ordering convention.

Please verify if the .env file should follow strict alphabetical ordering and, if so, consider repositioning the new variable.

docker-compose.yml (1)

288-288: LGTM — environment variable correctly exposed to realtime service.

The addition of _APP_LOGGING_CONFIG_REALTIME to the appwrite-realtime service environment is correct. Placement after _APP_LOGGING_CONFIG maintains consistency, and targeting only the realtime service aligns with the PR's intent to configure separate logging for the realtime process.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 26, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
imagemagick 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-c++ 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-dev 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-heic 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-jpeg 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-jxl 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-libs 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-openexr 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-webp 7.1.2.3-r0 CVE-2025-62171 HIGH
libecpg 17.6-r0 CVE-2025-12818 HIGH
libecpg-dev 17.6-r0 CVE-2025-12818 HIGH
libpq 17.6-r0 CVE-2025-12818 HIGH
libpq-dev 17.6-r0 CVE-2025-12818 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
postgresql17-dev 17.6-r0 CVE-2025-12818 HIGH
github.com/containerd/containerd/v2 v2.0.2 CVE-2024-25621 HIGH
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH
stdlib 1.22.10 CVE-2025-58183 HIGH
stdlib 1.22.10 CVE-2025-58186 HIGH
stdlib 1.22.10 CVE-2025-58187 HIGH
stdlib 1.22.10 CVE-2025-58188 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

coderabbitai[bot]
coderabbitai bot reviewed Nov 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11b8742 and 04dd0f8.

📒 Files selected for processing (2)
  • app/init/registers.php (2 hunks)
  • app/realtime.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/init/registers.php (1)
src/Appwrite/Extend/Exception.php (1)
  • Exception (7-465)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (1)
app/realtime.php (1)

238-275: Wiring realtime to its dedicated logger looks correct

Switching $register->get('logger') to $register->get('realtimeLogger') inside $logError cleanly routes realtime errors through the new, separately configurable logger, while preserving the existing guard:

  • If no logger is configured, the if ($logger && ...) simply skips external logging and still logs to Console, so behavior degrades gracefully.
  • The only caveat is that any exception thrown while resolving realtimeLogger (e.g., from DSN parsing) will bubble out of $logError; this is addressed by ensuring the realtimeLogger registration handles bad/legacy configs without throwing (see comment in app/init/registers.php).

With that registration adjusted, this usage is sound.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 26, 2025 •
edited
Loading

✨ Benchmark results

  • Requests per second: 1,195
  • Requests with 200 status code: 215,187
  • P99 latency: 0.161785972

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,195 1,251
200 215,187 225,180
P99 0.161785972 0.161912767

@abnegate abnegate merged commit 885a48b into 1.8.x Nov 27, 2025
40 checks passed
@abnegate abnegate deleted the feat-custom-realtime-logger branch November 27, 2025 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@abnegate