[v2] Implement FindTraces for memory backend
#7062
Conversation
|
@yurishkuro This PR is for getting a direction. Can we expect some OTLP parameters like scope version, name, span kind, status code and message as a tag from the user? (This is assumed in this PR) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7062 +/- ##
==========================================
+ Coverage 96.20% 96.22% +0.01%
==========================================
Files 358 358
Lines 21596 21689 +93
==========================================
+ Hits 20777 20870 +93
Misses 613 613
Partials 206 206
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:
|
|
Right now we only support search either by otel attributes (at any level) or by some first class fields like span name. Status search is by tag error=true. Other otel fields not supported. |
So what should we do on getting them in attributes? simply remove them? Currently I have extracted the required information from the tags, for example if the key is |
|
I don't think we need to worry about those right now, it should be implemented as a storage-agnostic solution by extending the search API to support first class fields like |
There was a problem hiding this comment.
Pull Request Overview
This PR implements the FindTraces functionality for the memory backend while updating the internal Tenant structure and associated tests.
- Updated Tenant struct: changed ids from a slice to a map and traces from a map to a slice of pointer wrappers.
- Implemented FindTraces in both Tenant and Store with error handling for non-positive search depths.
- Adjusted test cases and fixtures to account for the new structure and additional span fields.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/storage/v2/memory/tenant.go | Updated Tenant struct and eviction logic; implemented findTraces. |
| internal/storage/v2/memory/memory_test.go | Modified tests to access traces via new ids mapping and slice structure. |
| internal/storage/v2/memory/memory.go | Added FindTraces implementation in the Store with error handling. |
| internal/storage/v2/memory/fixtures/*.json | Updated fixture files to reflect new span kind and traceState values. |
Comments suppressed due to low confidence (1)
internal/storage/v2/memory/tenant.go:27
- [nitpick] Consider renaming 'tracesById' to 'tracesByID' for consistency with Go naming conventions.
type tracesById struct {
internal/storage/v2/memory/tenant.go
Outdated
| } | ||
|
|
||
| func validSpan(resourceAttributes, scopeAttributes pcommon.Map, span ptrace.Span, query tracestore.TraceQueryParams) bool { | ||
| _, errAttributeFound := query.Attributes.Get(errorAttribute) |
There was a problem hiding this comment.
What if the input is error=false?
There was a problem hiding this comment.
I am confused for that case. What if user has entered a string for error attribute, it will still be converted to attribute and val.Bool() would return false. Currently in both cases (when user has entered false and a string) it will return both unset and ok status spans. To fix this should we compare strings? For example errAttr.AsString() == "false"?
Signed-off-by: Manik2708 <mehtamanik96@gmail.com>
Which problem is this PR solving?
Description of the changes
FindTracesfor memory backendHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test