Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster #6956

pipiland2612 · 2025-04-01T19:25:13Z

Which problem is this PR solving?

part of Move private code into /internal/ #6869

Description of the changes

moved pkg/adjuster to cmd/query/app/querysvc/internal/adjuster

How was this change tested?

changes have passed all tests of the project

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
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: pipiland <nguyen.t.dang.minh@gmail.com>

codecov · 2025-04-01T20:06:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.20%. Comparing base (2fb42ef) to head (76f5ec5).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6956   +/-   ##
=======================================
  Coverage   96.19%   96.20%           
=======================================
  Files         344      343    -1     
  Lines       20253    20266   +13     
=======================================
+ Hits        19483    19496   +13     
  Misses        581      581           
  Partials      189      189

Flag	Coverage Δ
badger_v1	`10.53% <ø> (+0.32%)`	⬆️
badger_v2	`2.18% <ø> (+0.06%)`	⬆️
cassandra-4.x-v1-manual	`15.84% <ø> (+0.48%)`	⬆️
cassandra-4.x-v2-auto	`2.17% <ø> (+0.06%)`	⬆️
cassandra-4.x-v2-manual	`2.17% <ø> (+0.06%)`	⬆️
cassandra-5.x-v1-manual	`15.84% <ø> (+0.48%)`	⬆️
cassandra-5.x-v2-auto	`2.17% <ø> (+0.06%)`	⬆️
cassandra-5.x-v2-manual	`2.17% <ø> (+0.06%)`	⬆️
elasticsearch-6.x-v1	`20.76% <ø> (+0.63%)`	⬆️
elasticsearch-7.x-v1	`20.84% <ø> (+0.63%)`	⬆️
elasticsearch-8.x-v1	`21.02% <ø> (+0.64%)`	⬆️
elasticsearch-8.x-v2	`2.18% <ø> (+0.06%)`	⬆️
grpc_v1	`11.61% <ø> (+0.33%)`	⬆️
grpc_v2	`8.47% <ø> (+0.23%)`	⬆️
kafka-3.x-v1	`10.82% <ø> (+0.30%)`	⬆️
kafka-3.x-v2	`2.18% <ø> (+0.06%)`	⬆️
memory_v2	`2.18% <ø> (+0.06%)`	⬆️
opensearch-1.x-v1	`20.89% <ø> (+0.63%)`	⬆️
opensearch-2.x-v1	`20.89% <ø> (+0.63%)`	⬆️
opensearch-2.x-v2	`2.18% <ø> (+0.06%)`	⬆️
tailsampling-processor	`0.59% <ø> (+0.01%)`	⬆️
unittests	`94.98% <ø> (+<0.01%)`	⬆️

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.

yurishkuro · 2025-04-01T20:23:39Z

please see feedback on #6916

waiting on getting Yuri's feedback addressed

pipiland2612 · 2025-04-01T20:44:56Z

@yurishkuro thanks for the comment.

Indeed, the primary client of the package is cmd/query/app/querysvc/query_service.go.

However adjuster.ZipkinSpanIDUniquifier is used in internal/storage/v1/memory/memory.go.

Could you advise if this is actually needed in internal/storage/v1/memory/memory.go at all? I don't see it in use for other Trace Storage like elasticsearch.

If it's not needed, then it's straightforward to move to cmd/query/app/querysvc/adjuster. Otherwise, I struggle to see which would be the best approaches.

yurishkuro · 2025-04-02T04:42:16Z

However adjuster.ZipkinSpanIDUniquifier is used in internal/storage/v1/memory/memory.go.

Feel free to remove it from memory storage.

yurishkuro · 2025-04-02T04:43:34Z

move to cmd/query/app/querysvc/adjuster

Move to cmd/query/app/querysvc/internal/adjuster instead, to prevent usage from other packages

…rage Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

yurishkuro · 2025-04-02T14:02:47Z

cmd/query/app/querysvc/adjusters.go

@@ -7,20 +7,20 @@ package querysvc
 import (
 	"time"

-	"github.com/jaegertracing/jaeger/model/adjuster"
+	adjuster2 "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/internal/adjuster"


renaming is unnecessary change

Agree. I adjusted this in 76f5ec5.

Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

yurishkuro

Thanks

…tracing#6956) ## Which problem is this PR solving? - part of jaegertracing#6869 ## Description of the changes - moved `pkg/adjuster` to `cmd/query/app/querysvc/internal/adjuster` ## How was this change tested? - changes have passed all tests of the project ## 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: pipiland <nguyen.t.dang.minh@gmail.com>

…tracing#6956) ## Which problem is this PR solving? - part of jaegertracing#6869 ## Description of the changes - moved `pkg/adjuster` to `cmd/query/app/querysvc/internal/adjuster` ## How was this change tested? - changes have passed all tests of the project ## 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: pipiland <nguyen.t.dang.minh@gmail.com> Signed-off-by: amol-verma-allen <amol.verma@allen.in>

Move package pkg/adjuster to internal/adjuster

05db9be

Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

pipiland2612 marked this pull request as ready for review April 1, 2025 19:37

pipiland2612 requested a review from a team as a code owner April 1, 2025 19:37

pipiland2612 requested a review from albertteoh April 1, 2025 19:37

mahadzaryab1 previously approved these changes Apr 1, 2025

View reviewed changes

mahadzaryab1 added the changelog:refactoring Internal code refactoring without functional changes label Apr 1, 2025

mahadzaryab1 self-requested a review April 1, 2025 20:36

pipiland2612 added 2 commits April 2, 2025 13:54

Removed unused usage of adjuster.ZipkinSpanIDUniquifier in memory sto…

05cea9a

…rage Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster

628f3be

Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

pipiland2612 force-pushed the 6869-move-pkg-adjuster branch from 5c8f189 to 628f3be Compare April 2, 2025 11:15

yurishkuro requested changes Apr 2, 2025

View reviewed changes

pipiland2612 changed the title ~~Move pkg/adjuster to internal/adjuster~~ Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster Apr 2, 2025

Remove unnecessary pkg alias

76f5ec5

Signed-off-by: pipiland <nguyen.t.dang.minh@gmail.com>

yurishkuro approved these changes Apr 2, 2025

View reviewed changes

yurishkuro added this pull request to the merge queue Apr 2, 2025

yurishkuro mentioned this pull request Apr 2, 2025

moved model/adjuster to internal/adjuster #6916

Closed

4 tasks

Merged via the queue into jaegertracing:main with commit 9224477 Apr 2, 2025
56 checks passed

pipiland2612 deleted the 6869-move-pkg-adjuster branch April 2, 2025 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster #6956

Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster #6956

Uh oh!

pipiland2612 commented Apr 1, 2025 •

edited

Loading

Uh oh!

codecov bot commented Apr 1, 2025 •

edited

Loading

Uh oh!

yurishkuro commented Apr 1, 2025

Uh oh!

pipiland2612 commented Apr 1, 2025 •

edited

Loading

Uh oh!

yurishkuro commented Apr 2, 2025 •

edited

Loading

Uh oh!

yurishkuro commented Apr 2, 2025

Uh oh!

yurishkuro Apr 2, 2025

Uh oh!

pipiland2612 Apr 2, 2025

Uh oh!

yurishkuro left a comment

Uh oh!

Uh oh!

Uh oh!

Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster #6956

Move pkg/adjuster to cmd/query/app/querysvc/internal/adjuster #6956

Uh oh!

Conversation

pipiland2612 commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

Uh oh!

codecov bot commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yurishkuro commented Apr 1, 2025

Uh oh!

pipiland2612 commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurishkuro commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurishkuro commented Apr 2, 2025

Uh oh!

yurishkuro Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

pipiland2612 Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

yurishkuro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pipiland2612 commented Apr 1, 2025 •

edited

Loading

codecov bot commented Apr 1, 2025 •

edited

Loading

pipiland2612 commented Apr 1, 2025 •

edited

Loading

yurishkuro commented Apr 2, 2025 •

edited

Loading