-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Remove jaeger-agent from distributions #6081
Remove jaeger-agent from distributions #6081
Conversation
2193982
to
23eb88c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6081 +/- ##
==========================================
+ Coverage 96.92% 96.93% +0.01%
==========================================
Files 351 351
Lines 16675 16675
==========================================
+ Hits 16162 16164 +2
+ Misses 329 328 -1
+ Partials 184 183 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
1af89f1
to
2832689
Compare
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great! just one question
var ( | ||
queryAddr = fmt.Sprintf("http://%s:%d", host, ports.QueryHTTP) | ||
samplingAddr = fmt.Sprintf("http://%s:%d", host, ports.CollectorHTTP) | ||
healthAddr = fmt.Sprintf("http://%s:%d/status", host, ports.CollectorV2HealthChecks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a question for my own understanding: why is this the port here being called CollectorV2HealthChecks
when it is being used in v1 as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not being used with v1:
func healthCheckV2(t *testing.T) {
if os.Getenv("HEALTHCHECK_V2") == "false" {
t.Skip("Skipping health check for V1 Binary")
}
## Which problem is this PR solving? - Part of jaegertracing#4739 ## Description of the changes - Remove jaeger-agent from binary and Docker distributions - Move agent's port mappings into `cmd/agent/app/ports.go` to clean up the main ports ## Dependencies - Need to refactor crossdock to not rely on agent ## How was this change tested? - CI ## 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`: `yarn lint` and `yarn test` --------- Signed-off-by: Yuri Shkuro <[email protected]>
This reverts commit ac6f78b.
Which problem is this PR solving?
Description of the changes
cmd/agent/app/ports.go
to clean up the main portsDependencies
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test