-
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
Upgrade storage integration tests to Storage v2 API #6366
Comments
Sure. You cannot learn if you don't try. |
Hi @yurishkuro I hope you're doing well. I'm relatively new to Jaeger and I'm excited about the opportunity to contribute. I’d love to get involved and make some meaningful contributions. I am looking forward to learning from the community! |
There are no dependencies aside from Go tool chain (and we only support Linux-like environment). Please use Discussions for these types of questions, as they are distracting from the main topic of this issue.
The test is linked in the description. |
Hi @yurishkuro, Currently, the StorageIntegration test methods rely on the model.Trace data structure for traces. Since the storage_v2 API is built around OpenTelemetry's ptrace.Traces (pdata module), I have the following questions:
|
@ekefan yes we need to update the business logic of the tests to use ptrace model. We need to find a way to do that incrementally, not in a single humongous PR. Maybe for some time we will need to live in a state where implementations provide both v1 and v2 storage objects while we gradually transition the tests. We don't need to change json fixtures just yet, because we can easily transform them from model to ptrace. |
Currently our storage integration tests (
plugin/storage/integration/
andcmd/jaeger/internal/integration/
) operate in the existing v1 storage APIs. We want to migrate them to use V2 api fromstorage_v2/
.The way these tests operate is each storage backend has its own entry point to the tests and that entry point is responsible for initializing various storage reader/write APIs
jaeger/plugin/storage/integration/integration.go
Lines 43 to 50 in d6456fb
We can begin upgrading to v2 API by incrementally swapping these fields to have the corresponding v2 interfaces, e.g. replacing
SpanReader spanstore.Reader
field withTraceReader tracestore.Reader
and adjusting the test functions using that interface accordingly. The backend entry points then can provide TraceReader from the existing v1 SpanReader by wrapping it instorage_v2/v1adapter
.Then in the following PRs we can upgrade the remaining interfaces
The text was updated successfully, but these errors were encountered: