Skip to content

Conversation

Manik2708
Copy link
Contributor

Which problem is this PR solving?

Fixes a part of #6458

Description of the changes

  • Currently dbmodel can't be accessed by the v2, therefore it is moved out of v1/elasticsearch/internal/spanstore/internal

How was this change tested?

  • Unit Tests

Checklist

Copy link

codecov bot commented Mar 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.11%. Comparing base (3951c47) to head (c81532a).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6894      +/-   ##
==========================================
- Coverage   96.12%   96.11%   -0.02%     
==========================================
  Files         339      339              
  Lines       19597    19597              
==========================================
- Hits        18838    18835       -3     
- Misses        574      576       +2     
- Partials      185      186       +1     
Flag Coverage Δ
badger_v1 10.21% <0.00%> (ø)
badger_v2 2.12% <0.00%> (ø)
cassandra-4.x-v1-manual 15.36% <0.00%> (ø)
cassandra-4.x-v2-auto 2.11% <0.00%> (ø)
cassandra-4.x-v2-manual 2.11% <0.00%> (ø)
cassandra-5.x-v1-manual 15.36% <0.00%> (ø)
cassandra-5.x-v2-auto 2.11% <0.00%> (ø)
cassandra-5.x-v2-manual 2.11% <0.00%> (ø)
elasticsearch-6.x-v1 20.12% <100.00%> (ø)
elasticsearch-7.x-v1 20.21% <100.00%> (ø)
elasticsearch-8.x-v1 20.38% <100.00%> (ø)
elasticsearch-8.x-v2 2.12% <0.00%> (ø)
grpc_v1 11.28% <0.00%> (ø)
grpc_v2 8.25% <0.00%> (ø)
kafka-3.x-v1 10.51% <0.00%> (ø)
kafka-3.x-v2 2.12% <0.00%> (ø)
memory_v2 2.12% <0.00%> (ø)
opensearch-1.x-v1 20.26% <100.00%> (ø)
opensearch-2.x-v1 20.26% <100.00%> (ø)
opensearch-2.x-v2 2.12% <0.00%> (ø)
tailsampling-processor 0.57% <0.00%> (ø)
unittests 94.85% <95.23%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Manik2708
Copy link
Contributor Author

@yurishkuro @mahadzaryab1 What should be my next PR, implementing the writer for v2 or a directory structure with reader/writer implemented with panic("method not implemented")?

@Manik2708
Copy link
Contributor Author

Unit tests are passing locally, have to see what's going wrong in CI

Signed-off-by: Manik2708 <mehtamanik96@gmail.com>
Signed-off-by: Manik2708 <mehtamanik96@gmail.com>
@Manik2708
Copy link
Contributor Author

@yurishkuro @mahadzaryab1 Please review this PR as it is a blocker for implementing writer and reader for v2

@@ -17,6 +17,7 @@ import (
"github.com/stretchr/testify/require"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spanstore/internal/dbmodel is unnecessary nesting. Move it directly to spanstore/

Signed-off-by: Manik2708 <mehtamanik96@gmail.com>
@Manik2708 Manik2708 requested a review from yurishkuro March 23, 2025 15:03
@yurishkuro yurishkuro added the changelog:refactoring Internal code refactoring without functional changes label Mar 23, 2025
Signed-off-by: Manik2708 <mehtamanik96@gmail.com>
@yurishkuro yurishkuro enabled auto-merge March 23, 2025 16:23
@yurishkuro yurishkuro added this pull request to the merge queue Mar 23, 2025
Merged via the queue into jaegertracing:main with commit 3d35eda Mar 23, 2025
56 checks passed
amilbcahat pushed a commit to amilbcahat/jaeger that referenced this pull request May 4, 2025
…al` (jaegertracing#6894)

## Which problem is this PR solving?
Fixes a part of jaegertracing#6458 

## Description of the changes
- Currently `dbmodel` can't be accessed by the `v2`, therefore it is
moved out of `v1/elasticsearch/internal/spanstore/internal`

## 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: Manik2708 <mehtamanik96@gmail.com>
amilbcahat pushed a commit to amilbcahat/jaeger that referenced this pull request May 4, 2025
…al` (jaegertracing#6894)

## Which problem is this PR solving?
Fixes a part of jaegertracing#6458

## Description of the changes
- Currently `dbmodel` can't be accessed by the `v2`, therefore it is
moved out of `v1/elasticsearch/internal/spanstore/internal`

## 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: Manik2708 <mehtamanik96@gmail.com>
Signed-off-by: amol-verma-allen <amol.verma@allen.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/storage changelog:refactoring Internal code refactoring without functional changes storage/elasticsearch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants