Skip to content

fix(api-server): Repair logging setup and enhance observability:#1695

Merged
LinZhihao-723 merged 8 commits into
y-scope:mainfrom
LinZhihao-723:api-server-logging-fix
Nov 29, 2025
Merged

fix(api-server): Repair logging setup and enhance observability:#1695
LinZhihao-723 merged 8 commits into
y-scope:mainfrom
LinZhihao-723:api-server-logging-fix

Conversation

@LinZhihao-723

@LinZhihao-723 LinZhihao-723 commented Nov 29, 2025

Copy link
Copy Markdown
Member
  • Ensure the logger’s worker guard remains alive for the duration of the server.
  • Add request-handling trace logs to improve debugging and monitoring.
  • Set the default log level to INFO.
  • Format log events in JSON and with the source information included.

Description

The existing API server logging setup is broken and this PR is to fix the following problems:

  • The logger's worker guard is discarded inside the logger setup function, causing no log will be actually written to the log stream when the server is running (bug in this line). This PR fixes this problem by returning the guard to the server's main and hold it for the entire server execution.
  • Even with the worker guard fix, the log will not contain any request handling log. This PR adds logs to track request handling status, making it easier to debug issues (I was debugging whether fetching query results would reject non-existing queries and found nothing was showing up in the log).
  • The DEBUG log would only contain sqlx's log, which is very noisy as we probably don't need the actual SQL executed for debugging. This PR sets the log level to INFO to make the log concise.
  • Format log events in JSON with the source information included (file name and line number).

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Ensure all workflows pass.
  • Build a package locally and test the following step by step:
    • Compress some JSON logs.
    • Submit a query.
    • Fetch results and ensure the results are successfully received.
    • Fetch results from a non-existing query and ensure it's rejected.
      Logs:
{"timestamp":"2025-11-29T04:46:27.504843Z","level":"INFO","fields":{"message":"Server started at 0.0.0.0:3001"},"filename":"components/api-server/src/bin/api_server.rs","line_number":101}
{"timestamp":"2025-11-29T04:46:40.423889Z","level":"INFO","fields":{"message":"Submitting query: QueryConfig { query_string: \"*\", dataset: None, max_num_results: 0, begin_timestamp: None, end_timestamp: None, ignore_case: false, write_to_file: false }"},"filename":"components/api-server/src/routes.rs","line_number":101}
{"timestamp":"2025-11-29T04:46:40.427186Z","level":"INFO","fields":{"message":"Submitted query with search job ID: 1"},"filename":"components/api-server/src/routes.rs","line_number":104}
{"timestamp":"2025-11-29T04:46:47.498577Z","level":"INFO","fields":{"message":"Fetching results for search job ID: 1"},"filename":"components/api-server/src/routes.rs","line_number":146}
{"timestamp":"2025-11-29T04:46:47.502079Z","level":"INFO","fields":{"message":"Successfully initiated result stream for search job ID 1"},"filename":"components/api-server/src/routes.rs","line_number":149}
{"timestamp":"2025-11-29T04:46:49.770245Z","level":"INFO","fields":{"message":"Fetching results for search job ID: 100"},"filename":"components/api-server/src/routes.rs","line_number":146}
{"timestamp":"2025-11-29T04:46:49.770603Z","level":"ERROR","fields":{"message":"Failed to fetch results for search job ID 100: Sql(RowNotFound)"},"filename":"components/api-server/src/routes.rs","line_number":156}

Summary by CodeRabbit

  • Chores

    • Improved logging setup to use a non-blocking writer guard so logging stays active for the program duration.
    • Switched default runtime log level from DEBUG to INFO.
    • Enabled Debug formatting for query configuration objects.
  • Bug Fixes

    • Added diagnostic logs around query submission and result streaming and explicit handling/logging for malformed SSE lines.

✏️ Tip: You can customize this high-level summary in your review settings.

@LinZhihao-723 LinZhihao-723 requested a review from a team as a code owner November 29, 2025 03:27
@coderabbitai

coderabbitai Bot commented Nov 29, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Logging setup now returns and retains a tracing_appender non-blocking writer guard; QueryConfig gains a Debug derive; additional tracing added around query submission and result streaming (including malformed SSE line handling); api_server RUST_LOG default changed from DEBUG to INFO.

Changes

Cohort / File(s) Summary
Logging infrastructure
components/api-server/src/bin/api_server.rs
set_up_logging signature changed from -> anyhow::Result<()> to -> anyhow::Result<WorkerGuard>; returns and the caller retains the non-blocking WorkerGuard; event output explicitly JSON-formatted with level, file, and line number.
Type enhancement
components/api-server/src/client.rs
Added Debug to the derives for QueryConfig (Clone, Debug, Serialize, Deserialize, ToSchema).
Observability / request flow
components/api-server/src/routes.rs
Added tracing/logging around query submission (logs incoming QueryConfig, logs assigned search job ID on success, logs and returns on submit error) and around result fetching (logs fetch attempts, success on stream initiation, errors); added error logging and InternalServer return for malformed SSE lines with multiple trimmed lines.
Configuration
tools/deployment/package/docker-compose-all.yaml
Changed RUST_LOG for api_server service from "DEBUG" to "INFO".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review components/api-server/src/bin/api_server.rs to ensure the WorkerGuard is kept in scope and JSON event formatting is correct.
  • Verify components/api-server/src/routes.rs logging does not expose sensitive data and SSE malformed-line handling is correct.
  • Confirm components/api-server/src/client.rs derive addition causes no downstream trait conflicts.
  • Check tools/deployment/package/docker-compose-all.yaml RUST_LOG change aligns with operational expectations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main changes: repairing logging setup and enhancing observability by fixing the worker guard lifecycle and adding comprehensive request-handling logs.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a303418 and 5fec971.

📒 Files selected for processing (1)
  • components/api-server/src/bin/api_server.rs (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LinZhihao-723
Repo: y-scope/clp PR: 549
File: components/core/tests/test-ir_encoding_methods.cpp:1180-1186
Timestamp: 2024-10-08T15:52:50.753Z
Learning: In the context of loop constructs, LinZhihao-723 prefers using `while (true)` loops and does not consider alternative loop constructs necessarily more readable.
Learnt from: LinZhihao-723
Repo: y-scope/clp PR: 549
File: components/core/tests/test-ir_encoding_methods.cpp:1180-1186
Timestamp: 2024-10-01T07:59:11.208Z
Learning: In the context of loop constructs, LinZhihao-723 prefers using `while (true)` loops and does not consider alternative loop constructs necessarily more readable.
⏰ 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). (4)
  • GitHub Check: package-image
  • GitHub Check: rust-checks
  • GitHub Check: lint-check (ubuntu-24.04)
  • GitHub Check: lint-check (macos-15)
🔇 Additional comments (3)
components/api-server/src/bin/api_server.rs (3)

4-8: LGTM!

Import changes correctly bring in WorkerGuard to support the new logging lifecycle management.


45-66: Correct fix for the logging guard lifecycle issue.

Returning the WorkerGuard ensures the non-blocking writer's background thread stays alive for the server's lifetime. Previously, the guard was dropped at the end of set_up_logging(), which would have caused logs to be lost.

The JSON event format with source location (with_file and with_line_number) is well-suited for structured logging and observability.


84-84: LGTM!

Binding to _guard correctly keeps the WorkerGuard alive for the program's duration while signalling the variable is intentionally unused. This ensures all logs are properly flushed on shutdown.


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.

hoophalab
hoophalab previously approved these changes Nov 29, 2025
@LinZhihao-723 LinZhihao-723 merged commit e6d7a58 into y-scope:main Nov 29, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants