Enhance logs for non-standard sender factory usage #657
Conversation
Add logs to SenderResolver to handle non-standard usages of SenderFactory. Commit addresses jaegertracing#532. Signed-off-by: Bhargava Varadharajan <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #657 +/- ##
===========================================
- Coverage 89.4% 89.4% -0.01%
- Complexity 563 567 +4
===========================================
Files 69 69
Lines 2076 2085 +9
Branches 263 266 +3
===========================================
+ Hits 1856 1864 +8
Misses 138 138
- Partials 82 83 +1
Continue to review full report at Codecov.
|
| boolean isRequestedFactoryAvailable = false; | ||
| if (!senderFactoryIterator.hasNext()) { | ||
| log.warn("No sender factories available"); | ||
| } |
There was a problem hiding this comment.
You could probably short-circuit here and return NoopSender() directly. Doesn't make much difference in the runtime, especially once the VM is warmed up, but makes the code easier to read.
| @@ -81,6 +81,22 @@ public void testMultipleImplementationsAmbiguous() throws Exception { | |||
| assertTrue(sender instanceof NoopSender); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testMultipleFactoriesButFactoryNotSpecified() throws Exception { | |||
There was a problem hiding this comment.
Isn't this the same as testMultipleImplementationsAmbiguous ?
There was a problem hiding this comment.
True. My bad. Removed redundant test.
Signed-off-by: Bhargava Varadharajan <[email protected]>
| @@ -30,6 +30,7 @@ | |||
| import java.nio.file.Path; | |||
| import java.util.Scanner; | |||
| import org.junit.After; | |||
| import org.junit.Ignore; | |||
There was a problem hiding this comment.
Is this import being used?
There was a problem hiding this comment.
Removed this and one other unused import( import io.jaegertracing.spi.SenderFactory;) in the same class.
Signed-off-by: Bhargava Varadharajan <[email protected]>
Add logs to SenderResolver to handle non-standard usages
of SenderFactory. Commit addresses #532.
Signed-off-by: Bhargava Varadharajan [email protected]
Which problem is this PR solving?
Short description of the changes