Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Upgrade storage integration tests to Storage v2 API #6366

Open
6 tasks
yurishkuro opened this issue Dec 15, 2024 · 4 comments · May be fixed by #6369
Open
6 tasks

Upgrade storage integration tests to Storage v2 API #6366

yurishkuro opened this issue Dec 15, 2024 · 4 comments · May be fixed by #6369
Labels
area/storage good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement v2

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Dec 15, 2024 •
edited
Loading

Currently our storage integration tests (plugin/storage/integration/ and cmd/jaeger/internal/integration/) operate in the existing v1 storage APIs. We want to migrate them to use V2 api from storage_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

type StorageIntegration struct {
SpanWriter spanstore.Writer
SpanReader spanstore.Reader
ArchiveSpanReader spanstore.Reader
ArchiveSpanWriter spanstore.Writer
DependencyWriter dependencystore.Writer
DependencyReader dependencystore.Reader
SamplingStore samplingstore.Store

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 with TraceReader 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 in storage_v2/v1adapter.

Then in the following PRs we can upgrade the remaining interfaces

  • SpanReader
  • SpanWriter
  • ArchiveSpanReader
  • ArchiveSpanWriter
  • DependencyWriter
  • DependencyReader
@yurishkuro yurishkuro added good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement labels Dec 15, 2024
@yurishkuro
Copy link
Member Author

Sure. You cannot learn if you don't try.

@Ayush-Vish Ayush-Vish linked a pull request Dec 16, 2024 that will close this issue
Open
4 tasks
@Ayush-Vish
Copy link

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!

@yurishkuro
Copy link
Member Author

can you tell how to download all the dependencies for jaeger in order to contribute

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.

their is no integration_test.go

The test is linked in the description.

@ekefan
Copy link

ekefan commented Dec 18, 2024 •
edited
Loading

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:

  1. Do we need to rewrite the StorageIntegration test methods and supporting functions to use ptrace.Traces instead of model.Trace?
  2. For the test fixtures (e.g., JSON files) that are currently formatted for the Jaeger model.Trace structure, do we need to convert or rewrite them to comply with the storage_v2 API's data format? Or is there a recommended way to handle this transition dynamically during tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/storage good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement v2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade storage integration tests to Storage v2 API Ayush-Vish/jaeger
3 participants
@yurishkuro @Ayush-Vish @ekefan