fix: grpc reconnection #1521
fix: grpc reconnection #1521
Conversation
kstasik
requested review from
Aneurysm9,
dashpole,
evantorrie,
jmacd,
lizthegrey,
MrAlias and
XSAM
as code owners
|
|
Codecov Report
@@ Coverage Diff @@
## main #1521 +/- ##
=====================================
Coverage 78.0% 78.0%
=====================================
Files 127 127
Lines 6600 6600
=====================================
+ Hits 5153 5154 +1
+ Misses 1203 1202 -1
Partials 244 244
|
|
Is there an issue tracking this? If not, can we start there? It would be nice to better understand the problem this is attempting to solve. |
|
I hope it helps a little bit: |
|
It would be great if we had a test that would prevent regressions, since this bug has been a major issue of this nature. I am not very familiar with the code, but this certainly looks correct. |
| @@ -13,6 +13,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | |||
| - Rename project default branch from `master` to `main`. | |||
| - Reverse order in which `Resource` attributes are merged, per change in spec. (#1501) | |||
|
|
|||
| ## Fixed | |||
|
|
|||
| - Fixed otlpgrpc reconnection issue. | |||
There was a problem hiding this comment.
| - Fixed otlpgrpc reconnection issue. | |
| - Fixed otlpgrpc reconnection issue. (#1521) |
|
thank you - i didn't notice integration tests. I'm not sure about my solution but I think this test doesn't cover this line: Which i think causes goroutine to hang. I will try to write new test or extend this. |
|
I think changing disconnectedCh channel to buffered may be better option but I will change this PR once i have time to write integration tests. |
Actually, I saw it flake several times in CI yesterday, and locally I see it fail about 5% of the time (https://gist.githubusercontent.com/punya/1db2a5fc7aede3baa06685997e145f3b/raw/bc52d8e2ba68772539a36c87f8ff9d1492e2ecf6/runs.txt). |
|
btw. in this case connected() function was very misleading for me (grpc exporter). It doesn't mean exporter is connected and thanks to this reconnection goroutine it almost always returns true. First reconnection always successfully creates new "connection" which is really not established. I changed fix to buffered channel, please review and let me know what do you think? |
* fix: grpc reconnection fixed * chore: changelog update * fix: grpc reconnection issue - red test * fix: grpc reconnection open-telemetry#1524 * fix: grpc reconnection issue cleanup
picking up the change from open-telemetry#1521
I have successfully reproduced this issue on 0.15.0 but i think it also applies to recent version. After disconnection it tries to reconnect only once. With this fix it should try reconnect periodically until successful connection.
Please let me know what do you think. Maybe there is some other better way to fix it.