[clickhouse] Update FindTraceIDs to populate start and end timestamps#7770
[clickhouse] Update FindTraceIDs to populate start and end timestamps#7770yurishkuro merged 5 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7770 +/- ##
=======================================
Coverage 95.52% 95.53%
=======================================
Files 308 308
Lines 15475 15482 +7
=======================================
+ Hits 14783 14790 +7
Misses 545 545
Partials 147 147
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:
|
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
| t.start, | ||
| t.end | ||
| FROM spans s | ||
| LEFT JOIN trace_id_timestamps t ON s.trace_id = t.trace_id |
There was a problem hiding this comment.
@yurishkuro Thoughts on a LEFT JOIN vs. an INNER JOIN? A LEFT JOIN could return some null timestamps in between updates whereas an INNER JOIN could result in some trace_ids not being returned in between updates.
| t.end | ||
| FROM spans s | ||
| LEFT JOIN trace_id_timestamps t ON s.trace_id = t.trace_id | ||
| WHERE 1=1 AND s.service_name = ? AND s.name = ? AND s.duration >= ? AND s.duration <= ? LIMIT ?`, |
There was a problem hiding this comment.
How is it that you don't have time bounds on the search? Time range (lookback) is the mandatory query condition
There was a problem hiding this comment.
Going to do this in the next PR
…jaegertracing#7770) ## Which problem is this PR solving? - Towards jaegertracing#7134 ## Description of the changes - This PR updates `FindTraceIDs` function for ClickHouse storage to populate the start and end timestamps for each trace_id. It does this by add a join clause to the search query to join on the trace_id_timestamps table which is populated by a materialized view. ## How was this change tested? - Unit tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com> Signed-off-by: ThatDeparted2061 <harshraocodesup@gmail.com>
Which problem is this PR solving?
Description of the changes
FindTraceIDsfunction for ClickHouse storage to populate the start and end timestamps for each trace_id. It does this by add a join clause to the search query to join on the trace_id_timestamps table which is populated by a materialized view.How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test