-
Notifications
You must be signed in to change notification settings - Fork 2.8k
loadbalancingexporter: add Start for exporters #1687
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
loadbalancingexporter: add Start for exporters #1687
Conversation
I don't know whether its ok to save host for later usage. @bogdandrutu Please verify that it's legal |
Also I think this kind of bug is really painful. Can we prevent such things in future? Introduce unified end-to-end test for every new receiver and exporter or something. |
Codecov Report
@@ Coverage Diff @@
## master #1687 +/- ##
==========================================
- Coverage 89.39% 89.37% -0.02%
==========================================
Files 370 370
Lines 18115 18118 +3
==========================================
Hits 16193 16193
- Misses 1432 1434 +2
- Partials 490 491 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@Vemmy124 we don't have any concern for the moment about not keeping a reference to the host. So it is ok. |
@jpkrohling Resolved the issue in the latest commit |
…nternal structs (#1687) Signed-off-by: Bogdan Drutu <[email protected]>
Signed-off-by: Bogdan Drutu <[email protected]>
Description:
Fixed a bug with not working loadbalancingexporter with retry queue. Also added sorting for endpoints on backend change (assume it is occasionally missing).
Link to tracking Issue: Fixes #1621
Testing:
Added test for endpoints sort