[cassandra][v2] Refactor FromDbModel and ToDbModel to accept and return dbmodel.Span#7844
Open
Manik2708 wants to merge 23 commits intojaegertracing:mainfrom
Open
[cassandra][v2] Refactor FromDbModel and ToDbModel to accept and return dbmodel.Span#7844Manik2708 wants to merge 23 commits intojaegertracing:mainfrom
FromDbModel and ToDbModel to accept and return dbmodel.Span#7844Manik2708 wants to merge 23 commits intojaegertracing:mainfrom
Conversation
Contributor
Author
|
Mistakenly closed the last one, please review @yurishkuro |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7844 +/- ##
==========================================
+ Coverage 95.48% 95.67% +0.18%
==========================================
Files 316 316
Lines 16732 16640 -92
==========================================
- Hits 15977 15920 -57
+ Misses 590 569 -21
+ Partials 165 151 -14
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:
|
Metrics Comparison SummaryTotal changes across all snapshots: 32 Detailed changes per snapshotsummary_metrics_snapshot_badger📊 Metrics Diff SummaryTotal Changes: 32
❌ Removed Metrics
View diff sample-jaeger_storage_badger_compaction_current_num_lsm{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_compaction_current_num_lsm{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_get_num_memtable{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_get_num_memtable{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_get_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_get_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_get_with_result_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_get_with_result_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_iterator_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_iterator_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_put_num_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_put_num_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_read_bytes_lsm{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_read_bytes_lsm{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_read_bytes_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_read_bytes_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_read_num_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_read_num_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_size_bytes_lsm{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_size_bytes_lsm{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_size_bytes_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_size_bytes_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_bytes_l0{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_bytes_l0{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_bytes_user{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_bytes_user{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_bytes_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_bytes_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_num_vlog{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_num_vlog{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}View diff sample-jaeger_storage_badger_write_pending_num_memtable{name="another_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"}
-jaeger_storage_badger_write_pending_num_memtable{name="some_store",otel_scope_name="jaeger-v2",otel_scope_schema_url="",otel_scope_version="",role="tracestore"} |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Cassandra v2 trace store converters to work directly with dbmodel.Span instead of Jaeger protobuf models. The changes eliminate intermediate conversions and simplify the data flow by having FromDbModel and ToDbModel functions accept and return dbmodel.Span types.
Changes:
- Refactored converter functions to use
dbmodel.Spandirectly instead ofmodel.Batch - Updated type constants in dbmodel from private to public (e.g.,
stringType→StringType) - Removed intermediate batch grouping logic and process checksumming functionality
- Updated test fixtures to reflect the new data structure
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/storage/v2/cassandra/tracestore/to_dbmodel.go | Refactored conversion from ptrace.Traces to dbmodel.Span with direct field mappings |
| internal/storage/v2/cassandra/tracestore/to_dbmodel_test.go | Updated tests to use dbmodel types and added edge case coverage |
| internal/storage/v2/cassandra/tracestore/from_dbmodel.go | Simplified conversion from dbmodel.Span to ptrace.Traces, removed batch regrouping logic |
| internal/storage/v2/cassandra/tracestore/from_dbmodel_test.go | Updated tests to work with dbmodel.Span instead of model.Batch |
| internal/storage/v1/cassandra/spanstore/dbmodel/model.go | Exported type constants for public API usage |
| internal/storage/v1/cassandra/spanstore/dbmodel/converter.go | Updated references to use exported constant names |
| internal/storage/v2/cassandra/tracestore/fixtures/cas_01.json | Updated fixture to reflect dbmodel.Span structure |
| internal/storage/v2/cassandra/tracestore/fixtures/otel_traces_01.json | Added missing flags and status fields to test data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yurishkuro
reviewed
Jan 11, 2026
yurishkuro
reviewed
Jan 11, 2026
Merged
4 tasks
yurishkuro
pushed a commit
that referenced
this pull request
Jan 19, 2026
…rshaller for `TraceID` (#7867) ## Which problem is this PR solving? - Needed for: #7844 ## Description of the changes - Currently while writing snapshot tests for conversion, `TraceID` is abruptly formatted because of array representation of it. Unmarshaller/Marshaller will convert them to hex string. ## 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: Manik Mehta <mehtamanik96@gmail.com>
Manik2708
added a commit
to Manik2708/jaeger
that referenced
this pull request
Jan 20, 2026
…rshaller for `TraceID` (jaegertracing#7867) ## Which problem is this PR solving? - Needed for: jaegertracing#7844 ## Description of the changes - Currently while writing snapshot tests for conversion, `TraceID` is abruptly formatted because of array representation of it. Unmarshaller/Marshaller will convert them to hex string. ## 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: Manik Mehta <mehtamanik96@gmail.com>
Manik2708
force-pushed
the
cass-convert
branch
from
e930d0c to
f649ea8
Compare
Contributor
Author
|
@yurishkuro a reminder to review this pr |
yurishkuro
reviewed
Feb 15, 2026
yurishkuro
reviewed
Feb 15, 2026
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
yurishkuro
approved these changes
Feb 15, 2026
yurishkuro
added
the
changelog:experimental
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+19
to
+26
| ChildOf = "child-of" | ||
| FollowsFrom = "follows-from" | ||
|
|
||
| StringType = "string" | ||
| BoolType = "bool" | ||
| Int64Type = "int64" | ||
| Float64Type = "float64" | ||
| BinaryType = "binary" |
There was a problem hiding this comment.
These identifiers were made exported (ChildOf, FollowsFrom, StringType, etc.) but they don’t have doc comments. With revive enabled repo-wide, exported constants typically need comments (see e.g. index_filter.go). Please add brief comments for these exported constants (or keep them unexported if they’re not part of the intended API).
Suggested change
| ChildOf = "child-of" | |
| FollowsFrom = "follows-from" | |
| StringType = "string" | |
| BoolType = "bool" | |
| Int64Type = "int64" | |
| Float64Type = "float64" | |
| BinaryType = "binary" | |
| // ChildOf is the span reference type for a direct child relationship. | |
| ChildOf = "child-of" | |
| // FollowsFrom is the span reference type for a causal, but not direct child, relationship. | |
| FollowsFrom = "follows-from" | |
| // StringType indicates that a KeyValue holds a string value. | |
| StringType = "string" | |
| // BoolType indicates that a KeyValue holds a boolean value. | |
| BoolType = "bool" | |
| // Int64Type indicates that a KeyValue holds a 64-bit integer value. | |
| Int64Type = "int64" | |
| // Float64Type indicates that a KeyValue holds a 64-bit floating-point value. | |
| Float64Type = "float64" | |
| // BinaryType indicates that a KeyValue holds a binary (byte slice) value. | |
| BinaryType = "binary" |
Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| linkTraceID := dbmodel.TraceID(link.TraceID()) | ||
| linkSpanID := spanIDToDbSpanId(link.SpanID()) | ||
| linkRefType := dbRefTypeFromLink(link) | ||
| if parentSpanID != 0 && bytes.Equal(linkTraceID[:], traceID[:]) && linkSpanID == parentSpanID { |
There was a problem hiding this comment.
linksToDbSpanRefs compares TraceIDs via bytes.Equal(linkTraceID[:], traceID[:]), but dbmodel.TraceID is an array type and can be compared directly with ==. Using direct comparison avoids slice conversions and lets you drop the bytes import.
Suggested change
| if parentSpanID != 0 && bytes.Equal(linkTraceID[:], traceID[:]) && linkSpanID == parentSpanID { | |
| if parentSpanID != 0 && linkTraceID == traceID && linkSpanID == parentSpanID { |
Contributor
Author
There was a problem hiding this comment.
dbmodel.TraceID is [16]byte and it is best compared by bytes.Equal
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manik Mehta <mehtamanik96@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@yurishkuro all copilot comments are addressed! please review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
Description of the changes
ptrace.Traceto database models of cassandraHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test