[WIP][v2][storage] Create v2 query service to operate on otlp data model #6343
Conversation
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6343 +/- ##
==========================================
- Coverage 96.30% 95.90% -0.40%
==========================================
Files 366 367 +1
Lines 20946 21027 +81
==========================================
- Hits 20171 20165 -6
- Misses 593 678 +85
- Partials 182 184 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
mahadzaryab1
changed the title
…e on otlp format (#6396) ## Which problem is this PR solving? - Towards #6344 ## Description of the changes - Implemented a function `StandardAdjusters` that returns a list of adjusters to be applied on ptrace.Traces - This will be used by the v2 query service in #6343 ## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
| func (qs QueryServiceV2) Adjust(tracesIter iter.Seq[[]ptrace.Traces]) { | ||
| tracesIter(func(traces []ptrace.Traces) bool { | ||
| for _, trace := range traces { | ||
| qs.options.Adjuster.Adjust(trace) | ||
| } | ||
| return true | ||
| }) | ||
| } |
There was a problem hiding this comment.
@yurishkuro Did I understand correctly in that this is what we wanted here? Or did you mean that we should change the underlying adjusters themselves to work on Seq?
There was a problem hiding this comment.
yes, you understood correctly. However, because adjusting needs to re-arrange the data I think this func should return Seq[ptrace.Traces] where each item is fully adjusted trace.
Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
mahadzaryab1
added
the
changelog:new-feature
|
@yurishkuro Should this be classified as a new feature or a minor feature? |
| } | ||
|
|
||
| // StorageCapabilities is a feature flag for query service | ||
| type StorageCapabilitiesV2 struct { |
There was a problem hiding this comment.
I don't think this needs to be a new type, since it's in the same package.
| } | ||
|
|
||
| if qsvc.options.Adjuster == nil { | ||
| qsvc.options.Adjuster = adjuster.Sequence(adjuster.StandardAdjusters(defaultMaxClockSkewAdjust)...) |
There was a problem hiding this comment.
| qsvc.options.Adjuster = adjuster.Sequence(adjuster.StandardAdjusters(defaultMaxClockSkewAdjust)...) | |
| qsvc.options.Adjuster = adjuster.Sequence( | |
| adjuster.StandardAdjusters(defaultMaxClockSkewAdjust)...) |
| return yield(traces, nil) | ||
| }) | ||
| if qs.options.ArchiveTraceReader != nil { | ||
| missingTraceIDs := []tracestore.GetTraceParams{} |
There was a problem hiding this comment.
| missingTraceIDs := []tracestore.GetTraceParams{} | |
| var missingTraceIDs []tracestore.GetTraceParams |
no need to init to empty slice
| func (qs QueryServiceV2) Adjust(tracesIter iter.Seq[[]ptrace.Traces]) { | ||
| tracesIter(func(traces []ptrace.Traces) bool { | ||
| for _, trace := range traces { | ||
| qs.options.Adjuster.Adjust(trace) |
There was a problem hiding this comment.
you cannot assume here that trace is a full trace, you need to clump consecutive chunks if they are for the same trace ID. We can implement a helper function for that that will take Seq[[]ptrace.Traces] and return Seq[ptrace.Traces] where each item is a full trace. Similar to what I was suggesting in https://github.com/jaegertracing/jaeger/pull/6388/files#r1894703201
| func (qs QueryServiceV2) Adjust(tracesIter iter.Seq[[]ptrace.Traces]) { | ||
| tracesIter(func(traces []ptrace.Traces) bool { | ||
| for _, trace := range traces { | ||
| qs.options.Adjuster.Adjust(trace) | ||
| } | ||
| return true | ||
| }) | ||
| } |
There was a problem hiding this comment.
yes, you understood correctly. However, because adjusting needs to re-arrange the data I think this func should return Seq[ptrace.Traces] where each item is fully adjusted trace.
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test