Skip to content

feat: enhance logging with debug timing for various operations and ad…#631

Merged
jirhiker merged 2 commits intostagingfrom
field-compilation-optimization
Mar 30, 2026
Merged

feat: enhance logging with debug timing for various operations and ad…#631
jirhiker merged 2 commits intostagingfrom
field-compilation-optimization

Conversation

@jirhiker
Copy link
Copy Markdown
Member

Summary

This PR fixes the API lock-up that showed up after generating a PDF from the Field Compilation sheets flow, and trims the temporary debugging instrumentation down to a maintainable baseline.

The main issue ended up being on the follow-up read path, not the export/upload itself. After the initial export completed, concurrent GET /sample/{id} requests could stall or fail due to ORM relationship loading behavior. This PR makes the sample endpoint load the response graph explicitly and safely, while also keeping the request lifecycle visible in logs.

What Changed

Why

The original symptom looked like “PDF generation breaks the API,” but the logs showed the export, asset lookup, and groundwater observation requests were completing. The hang began when the frontend issued follow-up sample requests. Those responses were relying on ORM relationship loading during serialization, which is brittle under concurrent access and was enough to wedge the worker in this flow.

This change makes the sample endpoint deterministic and removes most of the high-volume timing noise unless explicitly enabled for debugging.

Operational Notes

  • Set API_DEBUG_TIMING=true to re-enable detailed timing/stage logs.
  • Set UVICORN_RELOAD=true if you want reload behavior in local Docker dev.

Validation

  • black passed on touched files.
  • python -m py_compile passed on touched files.
  • flake8 still reports pre-existing line-length (E501) issues in several touched modules.
  • Targeted pytest was not run in this pass.

Copilot AI review requested due to automatic review settings March 30, 2026 20:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses an API lock-up observed after PDF generation by making GET /sample/{id} deterministic via explicit eager-loading of the SampleResponse relationship graph, while refining request lifecycle logging and gating detailed timing instrumentation behind API_DEBUG_TIMING.

Changes:

  • Eager-load Sample relationships for both list and detail sample retrieval, and route GET /sample/{id} through a dedicated helper.
  • Add lightweight request lifecycle logs (request started / request completed) and gate detailed timing logs across several services with API_DEBUG_TIMING.
  • Update dev runtime defaults (Docker entrypoint / compose) and add/adjust tests for logging and GCS upload behavior.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
core/app.py Replaces request timing lock logic with request lifecycle logging and request IDs.
api/sample.py Switches sample-by-id to a helper that eagerly loads the response graph.
services/sample_helper.py Introduces comprehensive loader options and a new “by id with relationships” query helper.
api/asset.py Adds debug timing for bucket resolution/upload; moves signed-url generation to a threadpool for single-asset fetch.
services/gcs_helper.py Adds chunked hashing, stage timing logs (gated), and additional stage instrumentation.
services/thing_helper.py Adds optional (gated) timing log for thing lookup.
services/observation_helper.py Adds optional (gated) timing log for observation pagination query.
services/well_details_helper.py Adds (gated) stage timing logs and a new export payload builder.
api/thing.py Adds a new water-well export endpoint returning the minimal export payload.
schemas/well_export.py Adds WellExportResponse schema for the new export endpoint.
db/engine.py Adds connection pool event logging and configurable DB_POOL_TIMEOUT.
tests/test_request_timing.py Updates tests to assert lifecycle logs instead of cold/warm request timing logs.
tests/test_asset.py Adds tests for GCS timing logs, skipping existing blobs, and file rewind behavior; minor assertion/style tweaks.
entrypoint.sh Makes Uvicorn --reload opt-in via UVICORN_RELOAD=true.
docker-compose.yml Switches DB service to a PostGIS image and tweaks port quoting/commented build config.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65e51c6678

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jirhiker jirhiker merged commit 16b7197 into staging Mar 30, 2026
8 checks passed
@jirhiker jirhiker deleted the field-compilation-optimization branch March 30, 2026 20:54
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