w3c tracing integration test#522
Conversation
halspang
left a comment
halspang
left a comment
There was a problem hiding this comment.
Looks fine overall, just a few quick comments.
| private static boolean isZipkinUp() { | ||
| try (Socket ignored = new Socket("localhost", ZIPKIN_PORT)) { | ||
| return true; | ||
| } catch (IOException ignored) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it worth calling any specific API here to actually check the health? Or do we not really need to worry about a scenario where the socket is open but the underlying server isn't listening?
There was a problem hiding this comment.
If that happens, then we need to have a smarter logic here. Zipkin is installed way before this check, so we don't run into that race condition (yet).
Codecov Report
@@ Coverage Diff @@
## master #522 +/- ##
============================================
- Coverage 80.59% 80.46% -0.14%
+ Complexity 975 974 -1
============================================
Files 88 88
Lines 3015 3015
Branches 330 330
============================================
- Hits 2430 2426 -4
- Misses 420 423 +3
- Partials 165 166 +1
Continue to review full report at Codecov.
|
* w3c tracing integration test * Addresses PR comments. Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
3 participants
Description
Added integration test for W3C tracing
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #394
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: