Skip to content

feat(devops): add out-of-the-box Prometheus metrics and Grafana observability stack#9346

Closed
akoweicollinxx wants to merge 4 commits into
makeplane:previewfrom
akoweicollinxx:preview
Closed

feat(devops): add out-of-the-box Prometheus metrics and Grafana observability stack#9346
akoweicollinxx wants to merge 4 commits into
makeplane:previewfrom
akoweicollinxx:preview

Conversation

@akoweicollinxx

@akoweicollinxx akoweicollinxx commented Jul 2, 2026

Copy link
Copy Markdown

Description
This PR adds a self-contained observability setup for local development and self-hosted deployments.

What changed
Added django-prometheus to the API so it exposes Prometheus-compatible metrics.
Integrated Prometheus middleware to collect request and database timing data.
Added Prometheus and Grafana services to docker-compose-local.yml.
Included Grafana provisioning so datasources and dashboards are configured automatically.
Added a prebuilt dashboard to visualize API request counts and latency.

Validation
Built the local stack with Docker Compose.
Verified the /metrics endpoint is available.
Confirmed Prometheus is scraping targets successfully.
Opened Grafana and validated the dashboard renders live data.

Summary by CodeRabbit

  • Bug Fixes
    • Improved pagination handling for requests using either a page number or cursor value.
    • Added clearer validation messages for invalid pagination inputs.
    • Updated pagination responses so the next cursor is omitted when there are no more results.
    • Added test coverage for page-based navigation, end-of-list behavior, and stepping through successive pages.

Copilot AI review requested due to automatic review settings July 2, 2026 21:09
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ Copilot
❌ akoweicollinxx
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Modifies BasePaginator.paginate to explicitly parse a page query parameter (validated as integer >= 1) or cursor parameter, raising ParseError on invalid input, and changes next_cursor to return None when no further results exist. Adds unit tests covering this behavior.

Changes

Pagination page parameter support

Layer / File(s) Summary
Paginator cursor/page parsing and next_cursor logic
apps/api/plane/utils/paginator.py
paginate now explicitly parses cursor or page query parameters, validates page as an integer >= 1, raises ParseError with specific messages for invalid cursor/page, defaults to an initial cursor when neither is present, and returns next_cursor as None when cursor_result.next.has_results is false.
Unit tests for page-based pagination
apps/api/plane/tests/unit/utils/test_paginator.py
New test module with OffsetEchoPaginator and TwoPagePaginator stubs verifies explicit page-to-offset mapping, last-page behavior (next_page_results=False, next_cursor=None), and iteration termination via next_cursor.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

Not applicable; changes involve internal pagination logic with fewer than 3 distinct interacting components.

Poem
A rabbit hopped through pages new,
Cursor or page, it always knew,
When results ran dry and next was none,
It stopped the hop, the search was done,
Tests confirm the trail is true. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes Prometheus/Grafana observability, but the PR actually changes paginator behavior and tests, so it's misleading. Rename it to reflect the paginator changes, e.g. "Fix page parameter handling in BasePaginator".
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description is detailed and includes summary, changes, and validation, though it omits some template sections like Type of Change and References.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Warning

⚠️ This pull request shows signs of AI-generated slop (description_diff_mismatch). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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, as reflected by the included diff, updates the API pagination utility to support a page query parameter (in addition to cursor-based pagination) and adjusts cursor metadata serialization, with accompanying unit tests. However, this does not match the PR title/description about adding Prometheus/Grafana observability (see stored comment).

Changes:

  • Added page query param support in BasePaginator.paginate() with validation and conversion into an internal cursor.
  • Changed next_cursor serialization to return null/None when there are no further results.
  • Added unit tests covering page behavior and termination when following next_cursor.

Reviewed changes

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

File Description
apps/api/plane/utils/paginator.py Adds page parameter handling and modifies cursor metadata serialization in paginated responses.
apps/api/plane/tests/unit/utils/test_paginator.py Adds unit tests for page support and next-cursor termination behavior.

Comment on lines 736 to 740
"total_count": (cursor_result.hits),
"next_cursor": str(cursor_result.next),
"next_cursor": str(cursor_result.next) if cursor_result.next.has_results else None,
"prev_cursor": str(cursor_result.prev),
"next_page_results": cursor_result.next.has_results,
"prev_page_results": cursor_result.prev.has_results,
Comment thread apps/api/plane/utils/paginator.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
apps/api/plane/tests/unit/utils/test_paginator.py (1)

29-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the new error paths.

The new paginate logic raises ParseError for non-integer page, page < 1, and invalid cursor strings, but none of these are tested here. Consider adding cases asserting ParseError (e.g., pytest.raises) for page="abc", page="0", and a malformed cursor value.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/plane/tests/unit/utils/test_paginator.py` around lines 29 - 49, The
paginator tests cover valid page behavior but miss the new ParseError branches
in paginate. Add negative test cases in
test_paginate_honors_explicit_page_parameter or nearby tests to assert
ParseError is raised for non-integer page values, page values less than 1, and
malformed cursor strings; use BasePaginator.paginate with the existing
OffsetEchoPaginator and pytest.raises to verify these error paths.
apps/api/plane/utils/paginator.py (2)

737-738: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

prev_cursor is not nulled symmetrically with next_cursor.

This change nulls next_cursor when cursor_result.next.has_results is false, but prev_cursor (line 738) is always returned as a string even when prev_page_results is false (e.g., on the first page). This is an inconsistent API contract for consumers that now rely on None to detect the end of pagination — the same pattern should arguably apply to prev_cursor at the start.

♻️ Suggested symmetry fix
-                "prev_cursor": str(cursor_result.prev),
+                "prev_cursor": str(cursor_result.prev) if cursor_result.prev.has_results else None,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/plane/utils/paginator.py` around lines 737 - 738, The pagination
cursor response in paginator.py is inconsistent because `next_cursor` is nulled
via `cursor_result.next.has_results` but `prev_cursor` from the pagination
result is always stringified. Update the cursor-building logic in the paginator
response to mirror the `next_cursor` pattern by returning `None` for
`prev_cursor` when the previous page has no results (for example on the first
page), using the existing cursor result fields in the paginator method that
assembles `next_cursor` and `prev_cursor`.

677-696: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Cursor takes silent precedence over page param.

When both cursor and page query params are supplied, page is silently ignored with no validation or feedback. Consider documenting this precedence in a comment, or raising a ParseError if both are supplied to avoid ambiguous client expectations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/plane/utils/paginator.py` around lines 677 - 696, The
cursor-handling logic in paginator parsing silently ignores the page parameter
when both query params are present. Update the branch in the paginator method
that reads input_cursor_value and page_value to either explicitly reject
requests containing both with a ParseError or add a clear comment/documented
precedence if cursor must win. Keep the behavior consistent in the cursor
parsing path and ensure the precedence decision is visible near
cursor_cls.from_string and the page_number handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/api/plane/tests/unit/utils/test_paginator.py`:
- Around line 29-49: The paginator tests cover valid page behavior but miss the
new ParseError branches in paginate. Add negative test cases in
test_paginate_honors_explicit_page_parameter or nearby tests to assert
ParseError is raised for non-integer page values, page values less than 1, and
malformed cursor strings; use BasePaginator.paginate with the existing
OffsetEchoPaginator and pytest.raises to verify these error paths.

In `@apps/api/plane/utils/paginator.py`:
- Around line 737-738: The pagination cursor response in paginator.py is
inconsistent because `next_cursor` is nulled via
`cursor_result.next.has_results` but `prev_cursor` from the pagination result is
always stringified. Update the cursor-building logic in the paginator response
to mirror the `next_cursor` pattern by returning `None` for `prev_cursor` when
the previous page has no results (for example on the first page), using the
existing cursor result fields in the paginator method that assembles
`next_cursor` and `prev_cursor`.
- Around line 677-696: The cursor-handling logic in paginator parsing silently
ignores the page parameter when both query params are present. Update the branch
in the paginator method that reads input_cursor_value and page_value to either
explicitly reject requests containing both with a ParseError or add a clear
comment/documented precedence if cursor must win. Keep the behavior consistent
in the cursor parsing path and ensure the precedence decision is visible near
cursor_cls.from_string and the page_number handling.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5b6ccafe-513d-4be6-a941-d51775abdcbc

📥 Commits

Reviewing files that changed from the base of the PR and between 7fbf14a and 76308ca.

📒 Files selected for processing (2)
  • apps/api/plane/tests/unit/utils/test_paginator.py
  • apps/api/plane/utils/paginator.py

@akoweicollinxx akoweicollinxx reopened this Jul 2, 2026
@akoweicollinxx akoweicollinxx closed this by deleting the head repository Jul 2, 2026
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.

4 participants