Move private code into /internal/ #6869

yurishkuro · 2025-03-18T16:51:17Z

This is an umbrella issue for many many packages we have under /pkg/ and /model/. Everything there should be moved under /internal/, to prevent 3rd party repositories taking dependencies (e.g. #6408).

The recommendation to contributors:

pick one of the subpackages
make a suggestion where to move it
- sometimes we may want to change the paths
- sometimes we may want to just moved the files where they are used, e.g. pkg/normalizer does not need to be a separate package
open a PR with just that one change
rinse and repeat
remove pkg/metrics/metrics_test.go which was left over

The text was updated successfully, but these errors were encountered:

gentcod · 2025-03-20T03:12:15Z

@yurishkuro I am working on moving pkg/config

## Which problem is this PR solving? - part of #6869 ## Description of the changes - inline the `/pkg/normalizer `package into the metrics.go file where it's used. ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - move `pkg/tenancy `to `internal/tenancy` ## How was this change tested? - ## 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: danish9039 <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `pkg/recoveryhandler` to `internal/recoveryhandler` ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `pkg/hostname` to` internal/hostname` ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: hippie-danish <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `pkg/healthcheck` to` internal/healthcheck` ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: hippie-danish <[email protected]>

## Which problem is this PR solving? - Part of #6869 ## Description of the changes - It moves private code from `pkg/config` to `internal/config` - Modifies related files where there are related imports of the moved file(s) ## How was this change tested? ```bash make test ``` ## 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` --------- Signed-off-by: Oyefule <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `model/proto` to `internal/proto` ## How was this change tested? - ## 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: danish9039 <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `pkg/ prometheus` to` internal/prometheus` ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: hippie-danish <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `pkg/kafka` to `internal/kafka` ## How was this change tested? - ## 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: danish9039 <[email protected]>

- part of jaegertracing#6869 - moved `pkg/kafka` to `internal/kafka` - - [ ] 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: danish9039 <[email protected]>

- part of jaegertracing#6869 - moved `pkg/kafka` to `internal/kafka` - - [ ] 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: danish9039 <[email protected]> Signed-off-by: sAchin-680 <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - moved `pkg/metrics` to `internal/metrics` ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: hippie-danish <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

## Which problem is this PR solving? - **Resolves** #6869 ## Description of the changes - Moved `pkg/gogocodec` to `internal/gogocodec` - Updated all imports referencing `pkg/gogocodec` to `internal/gogocodec` ## How was this change tested? - Ran all tests to confirm that no other parts of the project were affected: ```bash make test ``` ## 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: sAchin-680 <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - This change is part of our ongoing effort to move public packages that are not intended for external use into the /internal directory. This prevents accidental external dependencies on our implementation details. - Moved all files from` /pkg/httpmetrics` to `/internal/httpmetrics` - Updated all import paths throughout the codebase - No functional changes ## How was this change tested? - ## 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: danish9039 <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes This change is part of our ongoing effort to move public packages that are not intended for external use into the /internal directory. This prevents accidental external dependencies on our implementation details. - Moved all files from `/pkg/distributedlock `to` /internal/distributedlock` - Updated all import paths throughout the codebase - No functional changes ## How was this change tested? - ## 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: danish9039 <[email protected]> Signed-off-by: hippie-danish <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - move `pkg/es` to `internal/storage/elasticsearch` ## How was this change tested? - ## 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: danish9039 <[email protected]>

## Which problem is this PR solving? - Part of #6869 ## Description of the changes - Move `proto-gen/*` under `internal/` ## How was this change tested? - CI --------- Signed-off-by: Yuri Shkuro <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - Remove package pkg/netutils by - removing unused function `FixLocalhost` - moving function GetPort as private util function `getPortForAddr` within package `cmd/query/app` - the only place where this function is used ## How was this change tested? - running existing tests in 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 <[email protected]>

## Which problem is this PR solving? - part of #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 <[email protected]>

## Which problem is this PR solving? - part of #6869 - closes #6914 (replacement) ## Description of the changes - moved `pkg/cassandra` to `internal/storage/cassandra` - users of `pkg/scassandra` are `internal/storage/v1/cassandra`, `internal/storage/distributedlock/cassandra`, `cmd/jaeger/internal/extension/jaegerstorage`. ## 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 <[email protected]>

## Which problem is this PR solving? - part of #6869 - closes #6910 (replacement) ## Description of the changes - Move `pkg/otelsemconv` to `internal/telemetry/otelsemconv` ## 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 <[email protected]>

## Which problem is this PR solving? - part of #6869 ## Description of the changes - Now pkg package is empty. Let's remove it [🎉](https://emojipedia.org/party-popper) ## How was this change tested? - tested with check list ## 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 <[email protected]> Signed-off-by: pipiland <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>

pipiland2612 · 2025-04-03T15:01:13Z

Where to put package model/json isn't straight forwards.
Here is how it's used

The package contains ui models, and primarily used by cmd/* and crossdock. The package is imported and aliases as ui there.
The package internal/converter/json convert jaeger domain models (github.com/jaegertracing/jaeger-idl/model) to ui models.

I see few options here:

O1: simply move this package inside internal: internal/json
O2: move this package inside internal, rename to uimodel: internal/uimodel
- this makes it clear that this package isn't about json utilities
- clients of this package doesn't need to alias package import as ui again and again
O3: O2 plus merging with internal/converter/json
- we would get 1 less package (reducing complexity)
- these two packages are coupled - often used together. this leads to move cohesive, complete package, and simpler client code

@yurishkuro I would love to year your recommendation here? I personally would go with O3 due to combined benefit.

yurishkuro · 2025-04-03T17:46:43Z

O2 sgtm. O3 does not - the projects are different and we're migrating the backend to use OTLP as the data model overall, so conversion between the old model/* and uimodel/ will become unnecessary next year (once we start deleting v1 code).

The internal/converter/json is a bad location though, I would rather move it to internal/uimodel/converter/v1

## Which problem is this PR solving? - Resolves #6869 ## Description of the changes - move `model/json` to `internal/uimodel/converter/v1` - remove unused `fixture.json` and `model` pakage - move `model/json/model.go` to `internal/uimodel/converter/v1` ## How was this change tested? - pass project test ## 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 <[email protected]>

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 Mar 18, 2025

danish9039 mentioned this issue Mar 19, 2025

Move pkg/normalizer to collector #6877

Merged

4 tasks

gentcod mentioned this issue Mar 20, 2025

Move pkg/config to internal/config #6884

Merged

4 tasks

This was referenced Mar 22, 2025

Move pkg/metrics to internal/metrics #6901

Merged

Remove pkg/discovery #6902

Merged

Move pkg/distributedlock to internal/distributedlock #6903

Merged

moved pkg/es to internal/storage/es #6904

Closed

Move pkg/httpmetrics to internal/httpmetrics #6905

Merged

sAchin-680 mentioned this issue Mar 22, 2025

Move pkg/jtracer to internal/jtracer #6906

Open

4 tasks

danish9039 mentioned this issue Mar 22, 2025

Move pkg/jtracer to internal/jtracer #6907

Merged

4 tasks

danish9039 mentioned this issue Mar 27, 2025

Move pkg/es to internal/storage/elasticsearch #6937

Merged

4 tasks

yurishkuro mentioned this issue Mar 29, 2025

Move proto-gen to internal #6941

Merged

This was referenced Apr 1, 2025

Remove package pkg/netutils #6955

Merged

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

Merged

This was referenced Apr 2, 2025

Move pkg/cassandra to internal/storage/cassandra #6960

Merged

Move pkg/otelsemconv to internal/telemetry/otelsemconv #6961

Merged

pipiland2612 mentioned this issue Apr 2, 2025

Delete empty pkg package #6967

Merged

4 tasks

pipiland2612 mentioned this issue Apr 3, 2025

Move model/json to internal/uimodel #6973

Merged

4 tasks

yurishkuro closed this as completed in #6973 Apr 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move private code into /internal/ #6869

Move private code into /internal/ #6869

yurishkuro commented Mar 18, 2025 •

edited

Loading

gentcod commented Mar 20, 2025

pipiland2612 commented Apr 3, 2025 •

edited

Loading

yurishkuro commented Apr 3, 2025

Move private code into /internal/ #6869

Move private code into /internal/ #6869

Comments

yurishkuro commented Mar 18, 2025 • edited Loading

gentcod commented Mar 20, 2025

pipiland2612 commented Apr 3, 2025 • edited Loading

yurishkuro commented Apr 3, 2025

yurishkuro commented Mar 18, 2025 •

edited

Loading

pipiland2612 commented Apr 3, 2025 •

edited

Loading