-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[storage][v2] Add reader adapter that just exposes the underlying v1 reader #6221
Conversation
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6221 +/- ##
=======================================
Coverage 96.43% 96.43%
=======================================
Files 354 355 +1
Lines 20134 20157 +23
=======================================
+ Hits 19416 19439 +23
Misses 530 530
Partials 188 188
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
} | ||
|
||
func (*TraceReader) GetTrace(_ context.Context, _ pcommon.TraceID) (ptrace.Traces, error) { | ||
panic("not implemented") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at some point we would need to implement these methods, because as soon as we start upgrading query subcomponents to use v2 storage api directly (e.g. the apiv3 handler, the easiest to upgrade since it already returns OTLP formats) they will be blocked on this implementation for v1-only backends.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yurishkuro what would these implementations look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you would call the underlying v1 reader to retrieve the trace and then convert the result into OTLP data model
Which problem is this PR solving?
Description of the changes
spanstore.Reader
interface in the factory adapter through theTraceReader
, which is currently just a wrapper around the v1spanstore.Reader
. TheTraceReader
provides a functionGetV1Reader
that exposes the underlying v1 reader which will be used in [v2][storage] Implement read path for v2 storage interface #6170.How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test