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

Move All Storage Packages Under internal #6637

Open
mahadzaryab1 opened this issue Jan 30, 2025 · 3 comments
Open

Move All Storage Packages Under internal #6637

mahadzaryab1 opened this issue Jan 30, 2025 · 3 comments
Assignees
@mahadzaryab1
Labels
area/storage enhancement

Comments

@mahadzaryab1
Copy link
Collaborator

mahadzaryab1 commented Jan 30, 2025 •
edited
Loading

We want to move all the storage package and organize them under internal to prevent any external dependencies.

The new proposed directory structure is as follows:

  • internal/storage/ - move existing storage/* here
  • internal/storage/v1 - all implementations go here (e.g internal/storage/v1/cassandra)
  • internal/storage/v1/meta - move plugin/storage/* here
  • storage/v2 - move storage_v2 here
@mahadzaryab1 mahadzaryab1 self-assigned this Jan 30, 2025
@dosubot dosubot bot added the area/storage label Jan 30, 2025
@mahadzaryab1 mahadzaryab1 mentioned this issue Jan 31, 2025
Merged
4 tasks
mahadzaryab1 added a commit that referenced this issue Jan 31, 2025
@mahadzaryab1
[refactor] Move cassandra implementation to internal package (#6639)
619a9f7 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation from `plugin/storage/` to
`internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
This was referenced Jan 31, 2025
Open
Merged
Merged
Merged
Merged
Merged
yurishkuro pushed a commit that referenced this issue Feb 1, 2025
@mahadzaryab1
[refactor] Move badger implementation to internal package (#6645)
3588dbe 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation for the badger storage from
`plugin/storage/` to `internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
yurishkuro added a commit that referenced this issue Feb 1, 2025
@mahadzaryab1 @yurishkuro
[refactor] Move memory implementation to internal package (#6646)
d5432a1 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation for memory storage from
`plugin/storage/` to `internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
mahadzaryab1 added a commit that referenced this issue Feb 2, 2025
@mahadzaryab1
[refactor] Move gRPC implementation to internal package (#6649)
eecc334 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation for gRPC storage from
`plugin/storage/` to `internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com>
mahadzaryab1 added a commit that referenced this issue Feb 2, 2025
@mahadzaryab1
[refactor] Move elasticsearch implementation to internal package (#…
d590c11 
…6648)

## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation for elasticsearch storage from
`plugin/storage/` to `internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com>
mahadzaryab1 added a commit that referenced this issue Feb 2, 2025
@mahadzaryab1
[refactor] Move blackhole implementation to internal package (#6647)
92246c5 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation for blackhole storage from
`plugin/storage/` to `internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
Signed-off-by: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com>
@yurishkuro
Copy link
Member

@mahadzaryab1 I think plugin/storage/scylladb could be moved to docker-compose/ instead

This was referenced Feb 2, 2025
Merged
Merged
mahadzaryab1 added a commit that referenced this issue Feb 2, 2025
@mahadzaryab1
[chore] Move scylladb implementation to docker-compose (#6652)
987f795 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the docker-compose setup for scylladb to from
`plugin/storage/scylladb` to `docker-compose/scylladb`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
mahadzaryab1 added a commit that referenced this issue Feb 2, 2025
@mahadzaryab1
[refactor] Move kafka implementation to internal package (#6651)
3dfdf8e 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the implementation for gRPC storage from
`plugin/storage/` to `internal/storage/`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
@mahadzaryab1 mahadzaryab1 mentioned this issue Feb 2, 2025
Merged
4 tasks
mahadzaryab1 added a commit that referenced this issue Feb 2, 2025
@mahadzaryab1
[refactor] Move storage to internal/storage/v1 (#6653)
f3b3981 
## Which problem is this PR solving?
- Towards #6637

## Description of the changes
- This PR moves the storage interfaces defined in `storage` to
`internal/storage/v1/api`

## How was this change tested?
- CI

## 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: Mahad Zaryab <mahadzaryab1@gmail.com>
@yurishkuro yurishkuro mentioned this issue Feb 2, 2025
Merged
yurishkuro added a commit that referenced this issue Feb 2, 2025
@yurishkuro
[refactor] Rename storage/v1/es to storage/v1/elasticsearch (#6655)
d3ceb01 
## Which problem is this PR solving?
- Part of #6637
- `es` was the only package name that did not contain the full name of
the database.

## Description of the changes
- Bring the naming in line with the rest of the backends: `es ->
elasticsearch`
- Keep direct imports using `es` as alias to minimize code changes
- Change es-mapping-generator binary location to match other binaries:
`./cmd/esmapping-generator/{binary}-{platform}`
- Remove unused target `build-esmapping-generator-linux`

## How was this change tested?
- CI

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro mentioned this issue Feb 2, 2025
Merged
@yurishkuro
Copy link
Member

I think plugin/storage/integration/ can go under internal/storage/integration. It doesn't need to be under v1/v2.

@yurishkuro
Copy link
Member

The meta-factory under plugin/storage could go to internal/storage/v1/factory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mahadzaryab1 mahadzaryab1

Labels
area/storage enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yurishkuro @mahadzaryab1