Fix Zipkin receiver keep-alive flag not being applied to HTTP server#7389
Fix Zipkin receiver keep-alive flag not being applied to HTTP server#7389Parship999 wants to merge 4 commits intojaegertracing:mainfrom Parship999:fix/Zipkin-receiver-keep-alive-flag
Conversation
Signed-off-by: Parship Chowdhury <i.am.parship@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7389 +/- ##
==========================================
- Coverage 96.44% 96.44% -0.01%
==========================================
Files 375 375
Lines 22871 22922 +51
==========================================
+ Hits 22058 22106 +48
- Misses 615 617 +2
- Partials 198 199 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Parship Chowdhury <i.am.parship@gmail.com>
Signed-off-by: Parship Chowdhury <i.am.parship@gmail.com>
Signed-off-by: Parship Chowdhury <i.am.parship@gmail.com>
|
@yurishkuro could you please take a look and let me know if the approach seems correct? |
| return errors.New("receiver is nil") | ||
| } | ||
|
|
||
| receiverValue := reflect.ValueOf(w.Traces) |
There was a problem hiding this comment.
it seems like you're trying to unwrap the zipkin receiver internals and set a flag. Why not just create a PR in OTEL contrib to expose the keepalive (assuming it's not exposed now)?
There was a problem hiding this comment.
I have created the pr as you suggested, now waiting for review:
open-telemetry/opentelemetry-collector-contrib#42531
|
@yurishkuro based on your feedback i have created this PR in the core repo: |
|
@yurishkuro |
|
Do we need any changes here? |
Yes, I think we need some changes here also. I am currently out of station, will complete it by next week. |
|
we don't require any changes here. so closing this pr. |
2 participants
Which problem is this PR solving?
Description of the changes
Issue is the
--collector.zipkin.keep-aliveflag was not being applied to the HTTP server created by the OpenTelemetry Zipkin receiver. While the flag existed in the collector configuration (options.Zipkin.KeepAlive), it was never used to control the actual HTTP server's keep alive behavior.Root Cause:
confighttp.ServerConfig.ToServer(), but theKeepAlivesetting from Jaeger's configuration was stored separately and never applied to the server.Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test