Add RemoteBaggageRestrictionManager #229
Conversation
| String jsonString; | ||
| try { | ||
| jsonString = | ||
| makeGetRequest("http://" + hostPort + "/baggageRestrictions?service=" + URLEncoder.encode(serviceName, "UTF-8")); |
There was a problem hiding this comment.
I'm not using URIbuilder here since it doesn't seem to be thread safe. (Albeit the usage pattern for this class is once a minute so we shouldn't be getting into situations where thread safety is required)
| Metrics metrics, | ||
| boolean denyBaggageOnInitializationFailure, | ||
| int refreshIntervalMs, | ||
| int initialDelayMs |
There was a problem hiding this comment.
initialDelayMs is only exposed for testing purposes so I can determine when the first call to remote agent is made. Under normal operations, this class will start up and asynchronously fetch restrictions. If the user wants to know if restrictions are ready, they can check via isReady()
There was a problem hiding this comment.
this makes more sense as code comment
Codecov Report
@@ Coverage Diff @@
## master #229 +/- ##
============================================
+ Coverage 81.39% 81.89% +0.49%
- Complexity 504 525 +21
============================================
Files 82 87 +5
Lines 1935 2005 +70
Branches 232 236 +4
============================================
+ Hits 1575 1642 +67
- Misses 261 263 +2
- Partials 99 100 +1
Continue to review full report at Codecov.
|
| Metrics metrics, | ||
| boolean denyBaggageOnInitializationFailure, | ||
| int refreshIntervalMs, | ||
| int initialDelayMs |
There was a problem hiding this comment.
this makes more sense as code comment
| @@ -22,52 +22,27 @@ | |||
|
|
|||
| package com.uber.jaeger.samplers; | |||
|
|
|||
| import static com.uber.jaeger.utils.Utils.makeGetRequest; | |||
There was a problem hiding this comment.
seems com.uber.jaeger.utils.HTTP would've been a better class name
| conn.disconnect(); | ||
| } | ||
| return result.toString(); | ||
| this.hostPort = hostPort != null ? hostPort : DEFAULT_HOST_PORT; | ||
| } | ||
|
|
||
| SamplingStrategyResponse parseJson(String json) { |
There was a problem hiding this comment.
why not move this to util class as well, e.g.
class com.uber.jaeger.utils.HTTP
public <T> T getJSON(String url, Class<T> cls) throws ...;
No description provided.