Skip to content

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Oct 14, 2024

Which problem is this PR solving?

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

@yurishkuro yurishkuro added the changelog:breaking-change Change that is breaking public APIs or established behavior label Oct 14, 2024
@yurishkuro yurishkuro requested a review from a team as a code owner October 14, 2024 02:53
@yurishkuro yurishkuro requested a review from albertteoh October 14, 2024 02:53
@dosubot dosubot bot added the docker Pull requests that update Docker code label Oct 14, 2024
@yurishkuro yurishkuro force-pushed the remove-agent-from-distro branch from 2193982 to 23eb88c Compare October 14, 2024 02:59
Copy link

codecov bot commented Oct 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.93%. Comparing base (cfc9298) to head (a2f918a).
Report is 4 commits behind head on main.

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     
Flag Coverage Δ
badger_v1 7.98% <ø> (ø)
badger_v2 1.84% <ø> (ø)
cassandra-4.x-v1 15.75% <ø> (ø)
cassandra-4.x-v2 1.77% <ø> (ø)
cassandra-5.x-v1 15.75% <ø> (ø)
cassandra-5.x-v2 1.77% <ø> (ø)
elasticsearch-6.x-v1 18.94% <ø> (-0.02%) ⬇️
elasticsearch-7.x-v1 19.01% <ø> (ø)
elasticsearch-8.x-v1 19.20% <ø> (ø)
elasticsearch-8.x-v2 1.84% <ø> (ø)
grpc_v1 9.35% <ø> (-0.02%) ⬇️
grpc_v2 7.11% <ø> (ø)
kafka-v1 9.68% <ø> (ø)
kafka-v2 1.84% <ø> (ø)
memory_v2 1.84% <ø> (ø)
opensearch-1.x-v1 19.06% <ø> (+0.01%) ⬆️
opensearch-2.x-v1 19.06% <ø> (ø)
opensearch-2.x-v2 1.84% <ø> (+0.01%) ⬆️
tailsampling-processor 0.49% <ø> (ø)
unittests 95.74% <100.00%> (+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.

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro force-pushed the remove-agent-from-distro branch from 1af89f1 to 2832689 Compare October 14, 2024 16:03
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro requested review from mahadzaryab1 and removed request for albertteoh October 14, 2024 21:15
@yurishkuro yurishkuro mentioned this pull request Oct 14, 2024
11 tasks
Copy link
Collaborator

@mahadzaryab1 mahadzaryab1 left a 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)
Copy link
Collaborator

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?

Copy link
Member Author

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")
	}

@yurishkuro yurishkuro merged commit ac6f78b into jaegertracing:main Oct 15, 2024
49 checks passed
@yurishkuro yurishkuro deleted the remove-agent-from-distro branch October 15, 2024 01:16
chahatsagarmain pushed a commit to chahatsagarmain/jaeger that referenced this pull request Oct 23, 2024
## 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 <github@ysh.us>
yurishkuro added a commit to yurishkuro/jaeger that referenced this pull request Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:breaking-change Change that is breaking public APIs or established behavior docker Pull requests that update Docker code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants