[storage][v2] Implement GetTrace in v2 factory adapter
#6329
Conversation
Signed-off-by: Mahad Zaryab <[email protected]>
storage_v2/factoryadapter/reader.go
Outdated
| func (*TraceReader) GetTrace(_ context.Context, _ pcommon.TraceID) (ptrace.Traces, error) { | ||
| panic("not implemented") | ||
| func (tr *TraceReader) GetTrace(ctx context.Context, traceID pcommon.TraceID) (ptrace.Traces, error) { | ||
| tid, _ := model.TraceIDFromBytes(traceID[:]) |
There was a problem hiding this comment.
@yurishkuro are we okay to ignore this error? or do we want to handle it? An error is only returned if the length of the traceID isn't 8 or 16 and pcommon.TraceID is defined as type TraceID [16]byte
There was a problem hiding this comment.
If it's easy to test I wouldn't ignore it.
There was a problem hiding this comment.
@yurishkuro Its not possible to because the length of traceID is always 16 so the error will never be triggered. Let me know if I'm missing something though.
There was a problem hiding this comment.
if len=16 is guaranteed, I would just create TraceIDFromOTEL function that doesn't return an error.
There was a problem hiding this comment.
@yurishkuro thanks for the suggestion - done
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6329 +/- ##
==========================================
+ Coverage 96.12% 96.15% +0.02%
==========================================
Files 357 357
Lines 20537 20547 +10
==========================================
+ Hits 19742 19757 +15
+ Misses 607 603 -4
+ Partials 188 187 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Mahad Zaryab <[email protected]>
model/ids.go
Outdated
| @@ -156,6 +156,11 @@ func (t *TraceID) ToOTELTraceID() pcommon.TraceID { | |||
| return traceID | |||
| } | |||
|
|
|||
| func TraceIDFromOTEL(traceID pcommon.TraceID) TraceID { | |||
| tid, _ := TraceIDFromBytes(traceID[:]) | |||
There was a problem hiding this comment.
not exactly what I meant. I would move the 16-byte logic from TraceIDFromBytes() into this new function, and invoke this new function from TraceIDFromBytes()
There was a problem hiding this comment.
what about something like this? or do we want this to be more generic to just accept any 16 byte array so that it can be used from TraceIDFromBytes?
func TraceIDFromOTEL(traceID pcommon.TraceID) TraceID {
return TraceID{
High: binary.BigEndian.Uint64(traceID[:traceIDShortBytesLen]),
Low: binary.BigEndian.Uint64(traceID[traceIDShortBytesLen:]),
}
}Signed-off-by: Mahad Zaryab <[email protected]>
Which problem is this PR solving?
Description of the changes
GetTracein the v2 factory adapterHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test