-
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
Crossdock sampling redirection to collector #6115
Conversation
Signed-off-by: chahatsagarmain <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6115 +/- ##
========================================
Coverage 96.46% 96.47%
========================================
Files 352 353 +1
Lines 19985 20113 +128
========================================
+ Hits 19279 19404 +125
- Misses 522 524 +2
- Partials 184 185 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -44,7 +44,7 @@ type clientHandler struct { | |||
} | |||
|
|||
func main() { | |||
agentHostPort = getEnv(envAgentHostPort, "jaeger-agent:5778") |
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.
I am unclear how this even works before this change. There is no jaeger-agent
anymore in crossdock/jaeger-docker-compose.yml
. Is this code even being executed? Can we add some logging so that we can see it in the runs?
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.
I have added logs , but nothing new pops up when running the build and test script for this .
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.
ERROR: failed to solve: failed to compute cache key: failed to calculate checksum of ref gy29m51xhvo6f6bjl06qu1w8m::sc4007cgbnhy890gx6i98pld0: "/agent-linux-amd64": not found
This seems to be error from the DockerFile when running build and test script .
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.
what do you mean - are you running something manually?
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.
Its from crossdock ci test
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.
I checked the docker compose logs , got the log for initialization of agent service(crossdock) but no log for fetching sampling rate by the agent (GetSamplingRate method) .
Signed-off-by: chahatsagarmain <[email protected]>
Signed-off-by: chahatsagarmain <[email protected]>
Signed-off-by: chahatsagarmain <[email protected]>
Signed-off-by: chahatsagarmain <[email protected]>
@mahadzaryab1 how should i further remove "agent" related code ? |
@chahatsagarmain I would recommend doing this in a separate PR. The only dependency remaining on the jaeger-agent is crossdock. Once this PR is in and has removed the dependency on the agent, we can just remove the agent from jaeger altogether (see #4739 for more details). |
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.
I realized what the problem is - the function func (h *TraceHandler) AdaptiveSamplingTest(t crossdock.T)
is never called by anything. It was added in #230 but apparently never actually hooked up into the runtime.
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test