feat(telemetry): Add bytes-scanned metrics for query jobs. - #2380
feat(telemetry): Add bytes-scanned metrics for query jobs.#2380rishikeshdevsot wants to merge 4 commits into
Conversation
WalkthroughThe query scheduler now selects archive size metadata, stores compressed and uncompressed scan-byte totals on search jobs, emits four OpenTelemetry metrics at completion, and documents the new metrics. ChangesQuery scheduler scan telemetry
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant QueryScheduler
participant ArchiveSelection
participant SearchJob
participant OpenTelemetry
QueryScheduler->>ArchiveSelection: Select archive sizes
ArchiveSelection-->>QueryScheduler: Return compressed and uncompressed sizes
QueryScheduler->>SearchJob: Store summed scan-byte totals
QueryScheduler->>OpenTelemetry: Record totals at job completion
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
rishikeshdevsot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py (1)
1-1: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSemantic mismatch between metric names ("scanned") and tracking logic ("selected").
The new metrics and fields use the term
scanned, but the logic and descriptions explicitly track the dataselectedfor query jobs (i.e., the total size of all archives initially matching the query). Because search jobs can terminate early (e.g., when reachingmax_num_results), they may not scan all the selected archives, making thescannedlabel misleading.If the intent is to track the initial data selected (as the descriptions suggest), please update the names to
selectedto align with the behaviour. If the intent is to track data actually scanned, the logic must be reworked to incrementally aggregate the sizes of only completed tasks.
components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py#L157-176: Rename the OpenTelemetry metrics to_bytes_selectedand_bytes_selected_total(or change the tracking logic to measure actual scanned data).components/job-orchestration/job_orchestration/scheduler/scheduler_data.py#L90-91: Rename the fields touncompressed_bytes_selectedandcompressed_bytes_selected.components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py#L1043-1046: Update the metric emissions to use the correctly named metrics and fields.components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py#L1474-1476: Assign the values to the correctly named fields.docs/src/user-docs/reference-telemetry.md#L34-35: Update the documentation table rows to reflect the renamed counters.docs/src/user-docs/reference-telemetry.md#L60-61: Update the documentation table rows to reflect the renamed histograms.🤖 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 `@components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py` at line 1, The metrics and scheduler fields currently call data “scanned” while tracking the total data initially selected by the query. Rename the related OpenTelemetry metrics in query scheduler initialization, SchedulerData fields, metric emissions, and assignments to use “selected” consistently, including uncompressed/compressed variants; update both corresponding telemetry documentation tables to match the new names.
🤖 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.
Inline comments:
In
`@components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py`:
- Around line 1474-1476: Update the archive byte totals in the query scheduler
result construction to treat NULL database values as zero before summing.
Specifically, adjust the uncompressed_size and compressed_size lookups in the
sum expressions for uncompressed_bytes_scanned and compressed_bytes_scanned,
preserving the existing aggregation for non-NULL values.
---
Outside diff comments:
In
`@components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py`:
- Line 1: The metrics and scheduler fields currently call data “scanned” while
tracking the total data initially selected by the query. Rename the related
OpenTelemetry metrics in query scheduler initialization, SchedulerData fields,
metric emissions, and assignments to use “selected” consistently, including
uncompressed/compressed variants; update both corresponding telemetry
documentation tables to match the new names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0129d25-fef6-462d-9529-010e2a4381e2
📒 Files selected for processing (3)
components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.pycomponents/job-orchestration/job_orchestration/scheduler/scheduler_data.pydocs/src/user-docs/reference-telemetry.md
| uncompressed_bytes_scanned=sum(a["uncompressed_size"] for a in archives_for_search), | ||
| compressed_bytes_scanned=sum(a["compressed_size"] for a in archives_for_search), | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Safeguard against None values when summing database results.
If uncompressed_size or size can be NULL in the database schema, a["uncompressed_size"] will evaluate to None, causing sum() to raise a TypeError. Consider defaulting to 0 to safely handle potential NULL values.
🛡️ Proposed fix
- uncompressed_bytes_scanned=sum(a["uncompressed_size"] for a in archives_for_search),
- compressed_bytes_scanned=sum(a["compressed_size"] for a in archives_for_search),
+ uncompressed_bytes_scanned=sum((a["uncompressed_size"] or 0) for a in archives_for_search),
+ compressed_bytes_scanned=sum((a["compressed_size"] or 0) for a in archives_for_search),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uncompressed_bytes_scanned=sum(a["uncompressed_size"] for a in archives_for_search), | |
| compressed_bytes_scanned=sum(a["compressed_size"] for a in archives_for_search), | |
| ) | |
| uncompressed_bytes_scanned=sum((a["uncompressed_size"] or 0) for a in archives_for_search), | |
| compressed_bytes_scanned=sum((a["compressed_size"] or 0) for a in archives_for_search), | |
| ) |
🤖 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
`@components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py`
around lines 1474 - 1476, Update the archive byte totals in the query scheduler
result construction to treat NULL database values as zero before summing.
Specifically, adjust the uncompressed_size and compressed_size lookups in the
sum expressions for uncompressed_bytes_scanned and compressed_bytes_scanned,
preserving the existing aggregation for non-NULL values.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py (1)
1078-1081: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMetric emission logic looks correct; consider adding test coverage.
Emission is gated on a successful terminal-state DB update, mirroring the existing
job_duration_histogrampattern, and matches the PR's intent (histogram once per job, counters cumulative). No functional issue found here, but there's no test exercising the new sum-and-emit path (byte totals computed in_handle_new_search_joband recorded here on completion).🤖 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 `@components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py` around lines 1078 - 1081, Add test coverage for the successful terminal-state path that computes byte totals in _handle_new_search_job and records them at completion alongside job_duration_histogram. Verify each byte histogram is recorded once per job and each byte counter receives the cumulative total, while preserving the existing emission gating behavior.
🤖 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.
Outside diff comments:
In
`@components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py`:
- Around line 1078-1081: Add test coverage for the successful terminal-state
path that computes byte totals in _handle_new_search_job and records them at
completion alongside job_duration_histogram. Verify each byte histogram is
recorded once per job and each byte counter receives the cumulative total, while
preserving the existing emission gating behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 955768fe-87bb-4685-83b3-874807ad4bcc
📒 Files selected for processing (1)
components/job-orchestration/job_orchestration/scheduler/query/query_scheduler.py
3 participants
Description
Adds OpenTelemetry metrics to the query scheduler that track how much archive data is selected for each search/aggregation job, in both uncompressed (original log volume) and compressed (on-disk) bytes.
Two new histogram metrics, emitted once per query job:
clp.query.uncompressed_bytes_scanned,clp.query.compressed_bytes_scanned. Two new cumulative metrics:clp.query.uncompressed_bytes_scanned_total,clp.query.compressed_bytes_scanned_totalChecklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Documentation