Fixing RemoteBaggageRestrictionManager NullPointerException on empty response

Author: ravirajj

.gitignore

@@ -49,6 +49,7 @@ local.properties
 
 # IntelliJ
 /out/
+/jaeger-core/out/
 
 ### Intellij Patch ###
 # Comment Reason: https://github.com/joeblau/gitignore.io/issues/186#issuecomment-215987721

jaeger-core/src/main/java/io/jaegertracing/internal/baggage/RemoteBaggageRestrictionManager.java

@@ -79,6 +79,9 @@ void updateBaggageRestrictions() {
     List<BaggageRestrictionResponse> response;
     try {
       response = proxy.getBaggageRestrictions(serviceName);
+      if (response == null || response.isEmpty()) {
+        throw new BaggageRestrictionManagerException("empty restrictions response");
+      }
     } catch (BaggageRestrictionManagerException e) {
       metrics.baggageRestrictionsUpdateFailure.inc(1);
       return;

jaeger-core/src/test/java/io/jaegertracing/internal/baggage/RemoteBaggageRestrictionManagerTest.java

@@ -51,11 +51,6 @@ public class RemoteBaggageRestrictionManagerTest {
   public void setUp() throws Exception {
     metricsFactory = new InMemoryMetricsFactory();
     metrics = new Metrics(metricsFactory);
-    undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
-        .withProxy(baggageRestrictionProxy)
-        .withMetrics(metrics)
-        .withDenyBaggageInitializationFailure(false)
-        .build();
   }
 
   @After
@@ -66,16 +61,53 @@ public void tearDown() {
   @Test
   public void testUpdateBaggageRestrictions() throws Exception {
     when(baggageRestrictionProxy.getBaggageRestrictions(SERVICE_NAME))
-        .thenReturn(new ArrayList<BaggageRestrictionResponse>(Arrays.asList(RESTRICTION)));
-    undertest.updateBaggageRestrictions();
+        .thenReturn(new ArrayList<>(Arrays.asList(RESTRICTION)));
+    undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
+        .withProxy(baggageRestrictionProxy)
+        .withMetrics(metrics)
+        .withDenyBaggageInitializationFailure(false)
+        .build();
 
+    undertest.updateBaggageRestrictions();
     assertEquals(Restriction.of(true, MAX_VALUE_LENGTH), undertest.getRestriction(SERVICE_NAME, BAGGAGE_KEY));
     assertFalse(undertest.getRestriction(SERVICE_NAME, "bad-key").isKeyAllowed());
     assertTrue(
         metricsFactory.getCounter("jaeger_tracer_baggage_restrictions_updates.result=ok",
             Collections.emptyMap()) > 0L);
   }
 
+  @Test
+  public void testEmptyBaggageRestrictions() throws Exception {
+    when(baggageRestrictionProxy.getBaggageRestrictions(SERVICE_NAME))
+        .thenReturn(new ArrayList<>(Arrays.asList(RESTRICTION)))
+        .thenReturn(null);
+    undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
+        .withProxy(baggageRestrictionProxy)
+        .withMetrics(metrics)
+        .withDenyBaggageInitializationFailure(false)
+        .withRefreshIntervalMs(60000)
+        .withInitialDelayMs(60000)
+        .build();
+
+    // Initialized successfully
+    undertest.updateBaggageRestrictions();
+    assertEquals(Restriction.of(true, MAX_VALUE_LENGTH), undertest.getRestriction(SERVICE_NAME, BAGGAGE_KEY));
+    assertFalse(undertest.getRestriction(SERVICE_NAME, "bad-key").isKeyAllowed());
+    assertEquals(1L, metricsFactory.getCounter("jaeger_tracer_baggage_restrictions_updates.result=ok",
+        Collections.emptyMap()));
+    assertEquals(0L, metricsFactory.getCounter("jaeger_tracer_baggage_restrictions_updates.result=err",
+        Collections.emptyMap()));
+
+    // Empty restrictions response use previously initialized restriction
+    undertest.updateBaggageRestrictions();
+    assertEquals(Restriction.of(true, MAX_VALUE_LENGTH), undertest.getRestriction(SERVICE_NAME, BAGGAGE_KEY));
+    assertFalse(undertest.getRestriction(SERVICE_NAME, "bad-key").isKeyAllowed());
+    assertEquals(1L, metricsFactory.getCounter("jaeger_tracer_baggage_restrictions_updates.result=ok",
+        Collections.emptyMap()));
+    assertEquals(1L, metricsFactory.getCounter("jaeger_tracer_baggage_restrictions_updates.result=err",
+        Collections.emptyMap()));
+  }
+
   @Test
   public void testAllowBaggageOnInitializationFailure() throws Exception {
     when(baggageRestrictionProxy.getBaggageRestrictions(SERVICE_NAME))
@@ -100,7 +132,7 @@ public void testAllowBaggageOnInitializationFailure() throws Exception {
   @Test
   public void testDenyBaggageOnInitializationFailure() throws Exception {
     when(baggageRestrictionProxy.getBaggageRestrictions(SERVICE_NAME))
-        .thenReturn(new ArrayList<BaggageRestrictionResponse>(Arrays.asList(RESTRICTION)));
+        .thenReturn(new ArrayList<>(Arrays.asList(RESTRICTION)));
     undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
         .withProxy(baggageRestrictionProxy)
         .withMetrics(metrics)