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

feat: move jaeger tests to a separate package #403

Merged
merged 3 commits into from
Dec 4, 2024

Conversation

kruskall
Copy link
Member

@kruskall kruskall commented Dec 3, 2024

❓ Why is this being changed

jaeger is included in the module graph of downstream clients as an indirect dependency because of usage in otlp package.
(go module pruning doesn't seem to distinguish test files vs regular files 😢 )

APM Server output:

$ go mod why -m github.com/jaegertracing/jaeger
# github.com/jaegertracing/jaeger
github.com/elastic/apm-server/internal/beater/otlp
github.com/elastic/apm-data/input/otlp
github.com/elastic/apm-data/input/otlp.test
github.com/jaegertracing/jaeger/model

Related to elastic/apm-server#14768

🧑‍💻 What is being changed

move jaeger usage to a separate package along with testdata files

✅ How to validate the change

go test ./... to ensure tests are passing

go mod tidy in apm-server to validate the dependency is removed

move jaeger usage to a separate package to let module pruning do
the magic and remove it from the module graph for downstream clients.
@kruskall kruskall requested a review from a team as a code owner December 3, 2024 13:12
@elastic-observability-automation elastic-observability-automation bot added the safe-to-test Changes are safe to run in the CI label Dec 3, 2024
@kruskall kruskall merged commit 1c68dad into elastic:main Dec 4, 2024
5 checks passed
@kruskall kruskall deleted the feat/remove-jaeger branch December 4, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe-to-test Changes are safe to run in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants