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.

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

Remove usages of GetStorageFactory #6564

Open
mahadzaryab1 opened this issue Jan 17, 2025 · 4 comments · May be fixed by #6624
Open

Remove usages of GetStorageFactory #6564

mahadzaryab1 opened this issue Jan 17, 2025 · 4 comments · May be fixed by #6624

Comments

@mahadzaryab1
Copy link
Collaborator

mahadzaryab1 commented Jan 17, 2025

We have two remaining usages of jaegerstorage.GetStorageFactory, which provide the v1 factory:

We want to remove these and find a way to get the same functionality using the v2 factory.

@ADI-ROXX
Copy link
Contributor

@mahadzaryab1 Can you provide some resources to get started on v2. Also, what are the changes from v1 to v2

@osamaahmed17
Copy link

@mahadzaryab1, I am interested in this issue. Can you assign it to me?

@ary82
Copy link
Contributor

ary82 commented Jan 22, 2025

Hey @mahadzaryab1, I'm fairly new to the repository. Is this issue open for contributions?

I read a bit through the v2 proposal and the parent issue. From what I understand, we want to change jaegerstorage.GetStorageFactory to jaegerstorage.GetTraceStoreFactory, which provides the v2 Factory through the storage v1adapter, and implement storage.Purger and storage.SamplingStoreFactory in v1adapter.Factory.
We should be able to do this by defining the required methods and casting the v1 Factory to the necessary interfaces and calling their methods. For example, for Purger, in v1adapter/factory:

func (f *Factory) Purge(ctx context.Context) error {
	p, ok := f.ss.(storage_v1.Purger)
	if !ok {
	// storage backend doesn't implement Purger
	}
	err := p.Purge(ctx)
	return err
}

Am I on the right path?

@theNextEren
Copy link

hey @mahadzaryab1 but storage_v1 dosent have a samplingstore, and we can encapsulate both the depstore.Factory and tracestore.Factory into a struct to use the CreateDependencyReader ,CreateTraceReader and CreateTraceWriter in the jaegarstorage/extension.go WDYS

@ary82 ary82 linked a pull request Jan 27, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants