Validate start_time_max after start_time_min#8309
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #8289 by adding validation to ensure that start_time_max is after start_time_min in the Jaeger MCP server's search_traces tool. Previously, reversed time parameters would silently result in empty results. Now, the tool returns a clear error message when this validation fails.
Changes:
- Added validation check in the
buildQueryfunction to comparemaxStartTimeandminStartTimeafter both are parsed - Added comprehensive test case
TestSearchTracesHandler_Handle_StartTimeMaxBeforeMinthat verifies the validation works correctly with reversed time parameters - Error message is consistent with existing validation patterns in the codebase
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/jaeger/internal/extension/jaegermcp/internal/handlers/search_traces.go | Added validation logic to check that start_time_max is after start_time_min (lines 120-122) |
| cmd/jaeger/internal/extension/jaegermcp/internal/handlers/search_traces_test.go | Added test case for the new validation (lines 419-432) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
commits must be signed |
github-actions
Bot
added
the
waiting-for-author
Signed-off-by: ugurtafrali <[email protected]>
utafrali
force-pushed
the
fix/issue-8289-bug-jaegermcp-search-traces-missing-vali
branch
from
f4514f6 to
05a114e
Compare
github-actions
Bot
removed
the
waiting-for-author
|
on it, signing the commits now |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8309 +/- ##
==========================================
- Coverage 95.65% 95.64% -0.02%
==========================================
Files 318 314 -4
Lines 16861 16507 -354
==========================================
- Hits 16129 15788 -341
+ Misses 578 567 -11
+ Partials 154 152 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CI Summary ReportMetrics Comparison❌ 41 metric change(s) detected View changed metricsFor label-level diff details, open the CI run and expand the "Compare metrics and generate summary" step logs. metrics_snapshot_badger_e2e
metrics_snapshot_cassandras_4.x_v004_e2e_auto
metrics_snapshot_cassandras_4.x_v004_e2e_manual
metrics_snapshot_cassandras_5.x_v004_e2e_auto
metrics_snapshot_cassandras_5.x_v004_e2e_manual
metrics_snapshot_clickhouse
metrics_snapshot_elasticsearch_8.x_e2e
metrics_snapshot_elasticsearch_9.x_e2e
metrics_snapshot_grpc_e2e
metrics_snapshot_kafka_v2
metrics_snapshot_memory
metrics_snapshot_opensearch_2.x
metrics_snapshot_opensearch_3.x
Code Coverage❌ Coverage 96.8% (baseline 97%) ➡️ View CI run | View publish logs |
Fixes #8289
Wasn't validating that start_time_max is after start_time_min. Added the check and a test.