Skip to content

Commit f4bd0e5

Browse files
ldecheverz-splitldecheverz-split
committed
Fixing PR Comments
1 parent 91cce5d commit f4bd0e5

26 files changed

+112
-96
lines changed

‎client/src/main/java/io/split/cache/SegmentCache.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ public interface SegmentCache {
5353
List<SegmentImp> getAll();
5454

5555
/**
56-
* return every key
56+
* return key count
5757
* @return
5858
*/
59-
Set<String> getAllKeys();
59+
long getKeyCount();
6060
}

‎client/src/main/java/io/split/cache/SegmentCacheInMemoryImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public List<SegmentImp> getAll() {
6868
}
6969

7070
@Override
71-
public Set<String> getAllKeys() {
72-
return _segments.values().stream().flatMap(si -> si.getKeys().stream()).collect(Collectors.toSet());
71+
public long getKeyCount() {
72+
return _segments.values().stream().mapToLong(SegmentImp::getKeysSize).sum();
7373
}
7474
}

‎client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ public SegmentChange fetch(String segmentName, long since, FetchOptions options)
101101
throw new IllegalStateException("Could not retrieve segment changes for " + segmentName + "; http return code " + statusCode);
102102
}
103103

104-
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SEGMENTS, System.currentTimeMillis()-start);
105104
_telemetryRuntimeProducer.recordSuccessfulSync(LastSynchronizationRecordsEnum.SEGMENTS, System.currentTimeMillis());
106105

107106
String json = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
@@ -113,6 +112,7 @@ public SegmentChange fetch(String segmentName, long since, FetchOptions options)
113112
} catch (Throwable t) {
114113
throw new IllegalStateException("Problem fetching segmentChanges: " + t.getMessage(), t);
115114
} finally {
115+
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SEGMENTS, System.currentTimeMillis()-start);
116116
Utils.forceClose(response);
117117
}
118118

‎client/src/main/java/io/split/client/HttpSplitChangeFetcher.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ public SplitChange fetch(long since, FetchOptions options) {
9898
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.SPLIT_SYNC, statusCode);
9999
throw new IllegalStateException("Could not retrieve splitChanges; http return code " + statusCode);
100100
}
101-
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SPLITS, System.currentTimeMillis()-start);
102101

103102

104103
String json = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
@@ -110,6 +109,7 @@ public SplitChange fetch(long since, FetchOptions options) {
110109
} catch (Throwable t) {
111110
throw new IllegalStateException("Problem fetching splitChanges: " + t.getMessage(), t);
112111
} finally {
112+
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SPLITS, System.currentTimeMillis()-start);
113113
Utils.forceClose(response);
114114
}
115115
}

‎client/src/main/java/io/split/client/SplitClientConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public String streamingServiceURL() {
271271
return _streamingServiceURL;
272272
}
273273

274-
public String get_telemetryURL() {
274+
public String telemetryURL() {
275275
return _telemetryURL;
276276
}
277277

‎client/src/main/java/io/split/client/SplitClientImpl.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@
3737
public final class SplitClientImpl implements SplitClient {
3838
public static final SplitResult SPLIT_RESULT_CONTROL = new SplitResult(Treatments.CONTROL, null);
3939

40-
private static final String GET_TREATMENT = "getTreatment";
41-
private static final String GET_TREATMENT_WITH_CONFIG = "getTreatmentWithConfig";
42-
4340
private static final Logger _log = LoggerFactory.getLogger(SplitClientImpl.class);
4441

4542
private final SplitFactory _container;
@@ -79,27 +76,27 @@ public String getTreatment(String key, String split) {
7976

8077
@Override
8178
public String getTreatment(String key, String split, Map<String, Object> attributes) {
82-
return getTreatmentWithConfigInternal(GET_TREATMENT, key, null, split, attributes, MethodEnum.TREATMENT).treatment();
79+
return getTreatmentWithConfigInternal(key, null, split, attributes, MethodEnum.TREATMENT).treatment();
8380
}
8481

8582
@Override
8683
public String getTreatment(Key key, String split, Map<String, Object> attributes) {
87-
return getTreatmentWithConfigInternal(GET_TREATMENT, key.matchingKey(), key.bucketingKey(), split, attributes, MethodEnum.TREATMENT).treatment();
84+
return getTreatmentWithConfigInternal(key.matchingKey(), key.bucketingKey(), split, attributes, MethodEnum.TREATMENT).treatment();
8885
}
8986

9087
@Override
9188
public SplitResult getTreatmentWithConfig(String key, String split) {
92-
return getTreatmentWithConfigInternal(GET_TREATMENT_WITH_CONFIG, key, null, split, Collections.<String, Object>emptyMap(), MethodEnum.TREATMENT_WITH_CONFIG);
89+
return getTreatmentWithConfigInternal(key, null, split, Collections.<String, Object>emptyMap(), MethodEnum.TREATMENT_WITH_CONFIG);
9390
}
9491

9592
@Override
9693
public SplitResult getTreatmentWithConfig(String key, String split, Map<String, Object> attributes) {
97-
return getTreatmentWithConfigInternal(GET_TREATMENT_WITH_CONFIG, key, null, split, attributes, MethodEnum.TREATMENT_WITH_CONFIG);
94+
return getTreatmentWithConfigInternal(key, null, split, attributes, MethodEnum.TREATMENT_WITH_CONFIG);
9895
}
9996

10097
@Override
10198
public SplitResult getTreatmentWithConfig(Key key, String split, Map<String, Object> attributes) {
102-
return getTreatmentWithConfigInternal(GET_TREATMENT_WITH_CONFIG, key.matchingKey(), key.bucketingKey(), split, attributes, MethodEnum.TREATMENT_WITH_CONFIG);
99+
return getTreatmentWithConfigInternal(key.matchingKey(), key.bucketingKey(), split, attributes, MethodEnum.TREATMENT_WITH_CONFIG);
103100
}
104101

105102
@Override
@@ -184,27 +181,27 @@ private boolean track(Event event) {
184181
return _eventClient.track(event, propertiesResult.getEventSize());
185182
}
186183

187-
private SplitResult getTreatmentWithConfigInternal(String method, String matchingKey, String bucketingKey, String split, Map<String, Object> attributes, MethodEnum methodEnum) {
184+
private SplitResult getTreatmentWithConfigInternal(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes, MethodEnum methodEnum) {
188185
long initTime = System.currentTimeMillis();
189186
try {
190187
if(!_gates.isSDKReady()){
191-
_log.warn(method + ": the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method");
188+
_log.warn(methodEnum.getMethod() + ": the SDK is not ready, results may be incorrect. Make sure to wait for SDK readiness before using this method");
192189
_telemetryConfigProducer.recordNonReadyUsage();
193190
}
194191
if (_container.isDestroyed()) {
195192
_log.error("Client has already been destroyed - no calls possible");
196193
return SPLIT_RESULT_CONTROL;
197194
}
198195

199-
if (!KeyValidator.isValid(matchingKey, "matchingKey", _config.maxStringLength(), method)) {
196+
if (!KeyValidator.isValid(matchingKey, "matchingKey", _config.maxStringLength(), methodEnum.getMethod())) {
200197
return SPLIT_RESULT_CONTROL;
201198
}
202199

203-
if (!KeyValidator.bucketingKeyIsValid(bucketingKey, _config.maxStringLength(), method)) {
200+
if (!KeyValidator.bucketingKeyIsValid(bucketingKey, _config.maxStringLength(), methodEnum.getMethod())) {
204201
return SPLIT_RESULT_CONTROL;
205202
}
206203

207-
Optional<String> splitNameResult = SplitNameValidator.isValid(split, method);
204+
Optional<String> splitNameResult = SplitNameValidator.isValid(split, methodEnum.getMethod());
208205
if (!splitNameResult.isPresent()) {
209206
return SPLIT_RESULT_CONTROL;
210207
}
@@ -226,7 +223,7 @@ private SplitResult getTreatmentWithConfigInternal(String method, String matchin
226223
split,
227224
start,
228225
result.treatment,
229-
String.format("sdk.%s", method),
226+
String.format("sdk.%s", methodEnum.getMethod()),
230227
_config.labelsEnabled() ? result.label : null,
231228
result.changeNumber,
232229
attributes

‎client/src/main/java/io/split/client/SplitFactoryImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn
123123
// Cache Initialisations
124124
_segmentCache = new SegmentCacheInMemoryImpl();
125125
_splitCache = new InMemoryCacheImp();
126-
_telemetrySynchronizer = new TelemetrySubmitter(_httpclient, URI.create(config.get_telemetryURL()), _telemetryStorage, _splitCache, _segmentCache, _telemetryStorage, _startTime);
126+
_telemetrySynchronizer = new TelemetrySubmitter(_httpclient, URI.create(config.telemetryURL()), _telemetryStorage, _splitCache, _segmentCache, _telemetryStorage, _startTime);
127127

128128

129129
// Segments
@@ -213,7 +213,7 @@ public synchronized void destroy() {
213213
try {
214214
long splitCount = _splitCache.getAll().stream().count();
215215
long segmentCount = _segmentCache.getAll().stream().count();
216-
long segmentKeyCount = _segmentCache.getAllKeys().stream().count();
216+
long segmentKeyCount = _segmentCache.getKeyCount();
217217
_impressionsManager.close();
218218
_log.info("Successful shutdown of impressions manager");
219219
_eventClient.close();

‎client/src/main/java/io/split/client/impressions/AsynchronousImpressionListener.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package io.split.client.impressions;
22

33
import com.google.common.util.concurrent.ThreadFactoryBuilder;
4-
import io.split.telemetry.domain.enums.LastSynchronizationRecordsEnum;
5-
import io.split.telemetry.storage.TelemetryRuntimeProducer;
64
import org.slf4j.Logger;
75
import org.slf4j.LoggerFactory;
86

@@ -12,8 +10,6 @@
1210
import java.util.concurrent.ThreadPoolExecutor;
1311
import java.util.concurrent.TimeUnit;
1412

15-
import static com.google.common.base.Preconditions.checkNotNull;
16-
1713
/**
1814
* A wrapper around an ImpressionListener provided by the customer. The purpose
1915
* of the wrapper is to protect the SplitClient from any slow down happening due
@@ -48,14 +44,7 @@ public AsynchronousImpressionListener(ImpressionListener delegate, ExecutorServi
4844
@Override
4945
public void log(final Impression impression) {
5046
try {
51-
long initTime = System.currentTimeMillis();
52-
_executor.execute(new Runnable() {
53-
@Override
54-
public void run() {
55-
_delegate.log(impression);
56-
}
57-
});
58-
long endTime = System.currentTimeMillis();
47+
_executor.execute(() -> _delegate.log(impression));
5948
}
6049
catch (Exception e) {
6150
_log.warn("Unable to send impression to impression listener", e);

‎client/src/main/java/io/split/client/impressions/HttpImpressionsSender.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,12 @@ public void postImpressionsBulk(List<TestImpressions> impressions) {
7878
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.IMPRESSION_SYNC, status);
7979
_logger.warn("Response status was: " + status);
8080
}
81-
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.IMPRESSIONS, System.currentTimeMillis() - initTime);
8281
_telemetryRuntimeProducer.recordSuccessfulSync(LastSynchronizationRecordsEnum.IMPRESSIONS, System.currentTimeMillis());
8382

8483
} catch (Throwable t) {
8584
_logger.warn("Exception when posting impressions" + impressions, t);
8685
} finally {
86+
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.IMPRESSIONS, System.currentTimeMillis() - initTime);
8787
Utils.forceClose(response);
8888
}
8989

‎client/src/main/java/io/split/client/impressions/ImpressionsManagerImpl.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,15 @@ public void track(Impression impression) {
104104
_counter.inc(impression.split(), impression.time(), 1);
105105
}
106106

107-
if (Mode.DEBUG.equals(_mode) || shouldQueueImpression(impression)) {
108-
if (_storage.put(KeyImpression.fromImpression(impression))) {
109-
_telemetryRuntimeProducer.recordImpressionStats(ImpressionsDataTypeEnum.IMPRESSIONS_QUEUED, 1);
110-
} else {
111-
_telemetryRuntimeProducer.recordImpressionStats(ImpressionsDataTypeEnum.IMPRESSIONS_DROPPED, 1);
112-
}
113-
} else {
107+
if (Mode.OPTIMIZED.equals(_mode) && !shouldQueueImpression(impression)) {
114108
_telemetryRuntimeProducer.recordImpressionStats(ImpressionsDataTypeEnum.IMPRESSIONS_DEDUPED, 1);
109+
return;
110+
}
111+
if (!_storage.put(KeyImpression.fromImpression(impression))) {
112+
_telemetryRuntimeProducer.recordImpressionStats(ImpressionsDataTypeEnum.IMPRESSIONS_DROPPED, 1);
113+
return;
115114
}
115+
_telemetryRuntimeProducer.recordImpressionStats(ImpressionsDataTypeEnum.IMPRESSIONS_QUEUED, 1);
116116
}
117117

118118
@Override
@@ -130,7 +130,7 @@ public void close() {
130130
}
131131

132132
@VisibleForTesting
133-
/* package private */ void sendImpressions() {
133+
/* package private */ void sendImpressions() {
134134
if (_storage.isFull()) {
135135
_log.warn("Split SDK impressions queue is full. Impressions may have been dropped. Consider increasing capacity.");
136136
}

0 commit comments

Comments
 (0)