DataDog · jpbempel · May 31, 2024 · May 30, 2024
@@ -10,4 +10,6 @@ enum Source {
   }
 
   void accept(Source source, Collection<? extends ProbeDefinition> definitions);
+
+  void handleException(String configId, Exception ex);
 }
@@ -1,5 +1,9 @@
 package com.datadog.debugger.agent;
 
+import static com.datadog.debugger.agent.DebuggerProductChangesListener.LOG_PROBE_PREFIX;
+import static com.datadog.debugger.agent.DebuggerProductChangesListener.METRIC_PROBE_PREFIX;
+import static com.datadog.debugger.agent.DebuggerProductChangesListener.SPAN_DECORATION_PROBE_PREFIX;
+import static com.datadog.debugger.agent.DebuggerProductChangesListener.SPAN_PROBE_PREFIX;
 import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
 
 import com.datadog.debugger.instrumentation.InstrumentationResult;
@@ -9,6 +13,7 @@
 import com.datadog.debugger.util.ExceptionHelper;
 import datadog.trace.api.Config;
 import datadog.trace.bootstrap.debugger.DebuggerContext;
+import datadog.trace.bootstrap.debugger.ProbeId;
 import datadog.trace.bootstrap.debugger.ProbeImplementation;
 import datadog.trace.bootstrap.debugger.ProbeRateLimiter;
 import datadog.trace.util.TagsHelper;
@@ -72,6 +77,7 @@ public ConfigurationUpdater(
   // /!\ Can be called by different threads and concurrently /!\
   // Should throw a runtime exception if there is a problem. The message of
   // the exception will be reported in the next request to the conf service
+  @Override
   public void accept(Source source, Collection<? extends ProbeDefinition> definitions) {
     try {
       LOGGER.debug("Received new definitions from {}", source);
@@ -84,6 +90,31 @@ public void accept(Source source, Collection<? extends ProbeDefinition> definiti
     }
   }
 
+  @Override
+  public void handleException(String configId, Exception ex) {
+    if (configId == null) {
+      return;
+    }
+    ProbeId probeId;
+    if (configId.startsWith(LOG_PROBE_PREFIX)) {
+      probeId = extractPrefix(LOG_PROBE_PREFIX, configId);
+    } else if (configId.startsWith(METRIC_PROBE_PREFIX)) {
+      probeId = extractPrefix(METRIC_PROBE_PREFIX, configId);
+    } else if (configId.startsWith(SPAN_PROBE_PREFIX)) {
+      probeId = extractPrefix(SPAN_PROBE_PREFIX, configId);
+    } else if (configId.startsWith(SPAN_DECORATION_PROBE_PREFIX)) {
+      probeId = extractPrefix(SPAN_DECORATION_PROBE_PREFIX, configId);
+    } else {
+      probeId = new ProbeId(configId, 0);
+    }
+    LOGGER.warn("Error handling probe configuration: {}", configId, ex);
+    sink.getProbeStatusSink().addError(probeId, ex);
+  }
+
+  private ProbeId extractPrefix(String prefix, String configId) {
+    return new ProbeId(configId.substring(prefix.length()), 0);
+  }
+
   private void applyNewConfiguration(Configuration newConfiguration) {
     configurationLock.lock();
     try {

@@ -32,6 +32,10 @@ public class DebuggerProductChangesListener implements ProductListener {
   public static final int MAX_ALLOWED_LOG_PROBES = 100;
   public static final int MAX_ALLOWED_SPAN_PROBES = 100;
   public static final int MAX_ALLOWED_SPAN_DECORATION_PROBES = 100;
+  public static final String LOG_PROBE_PREFIX = "logProbe_";
+  public static final String METRIC_PROBE_PREFIX = "metricProbe_";
+  public static final String SPAN_PROBE_PREFIX = "spanProbe_";
+  public static final String SPAN_DECORATION_PROBE_PREFIX = "spanDecorationProbe_";
   private static final Logger LOGGER =
       LoggerFactory.getLogger(DebuggerProductChangesListener.class);
 
@@ -97,32 +101,37 @@ public void accept(
       datadog.remoteconfig.ConfigurationChangesListener.PollingRateHinter pollingRateHinter)
       throws IOException {
     String configId = configKey.getConfigId();
-    if (configId.startsWith("metricProbe_")) {
-      MetricProbe metricProbe = Adapter.deserializeMetricProbe(content);
-      configChunks.put(configId, definitions -> definitions.add(metricProbe));
-    } else if (configId.startsWith("logProbe_")) {
-      LogProbe logProbe = Adapter.deserializeLogProbe(content);
-      configChunks.put(configId, definitions -> definitions.add(logProbe));
-    } else if (configId.startsWith("spanProbe_")) {
-      SpanProbe spanProbe = Adapter.deserializeSpanProbe(content);
-      configChunks.put(configId, definitions -> definitions.add(spanProbe));
-    } else if (configId.startsWith("spanDecorationProbe_")) {
-      SpanDecorationProbe spanDecorationProbe = Adapter.deserializeSpanDecorationProbe(content);
-      configChunks.put(configId, definitions -> definitions.add(spanDecorationProbe));
-    } else if (IS_UUID.test(configId)) {
-      Configuration newConfig = Adapter.deserializeConfiguration(content);
-      if (newConfig.getService().equals(serviceName)) {
-        configChunks.put(
-            configId,
-            (builder) -> {
-              builder.addAll(newConfig.getDefinitions());
-            });
+    try {
+      if (configId.startsWith(METRIC_PROBE_PREFIX)) {
+        MetricProbe metricProbe = Adapter.deserializeMetricProbe(content);
+        configChunks.put(configId, definitions -> definitions.add(metricProbe));
+      } else if (configId.startsWith(LOG_PROBE_PREFIX)) {
+        LogProbe logProbe = Adapter.deserializeLogProbe(content);
+        configChunks.put(configId, definitions -> definitions.add(logProbe));
+      } else if (configId.startsWith(SPAN_PROBE_PREFIX)) {
+        SpanProbe spanProbe = Adapter.deserializeSpanProbe(content);
+        configChunks.put(configId, definitions -> definitions.add(spanProbe));
+      } else if (configId.startsWith(SPAN_DECORATION_PROBE_PREFIX)) {
+        SpanDecorationProbe spanDecorationProbe = Adapter.deserializeSpanDecorationProbe(content);
+        configChunks.put(configId, definitions -> definitions.add(spanDecorationProbe));
+      } else if (IS_UUID.test(configId)) {
+        Configuration newConfig = Adapter.deserializeConfiguration(content);
+        if (newConfig.getService().equals(serviceName)) {
+          configChunks.put(
+              configId,
+              (builder) -> {
+                builder.addAll(newConfig.getDefinitions());
+              });
+        } else {
+          throw new IOException(
+              "got config.serviceName = " + newConfig.getService() + ", ignoring configuration");
+        }
       } else {
-        throw new IOException(
-            "got config.serviceName = " + newConfig.getService() + ", ignoring configuration");
+        LOGGER.debug("Unsupported configuration id: {}, ignoring configuration", configId);
       }
-    } else {
-      LOGGER.debug("Unsupported configuration id: {}, ignoring configuration", configId);
+    } catch (Exception ex) {
+      configurationAcceptor.handleException(configId, ex);
+      throw new IOException(ex);
     }
   }
 

@@ -1,6 +1,7 @@
 package com.datadog.debugger.agent;
 
 import static com.datadog.debugger.agent.ConfigurationAcceptor.Source.REMOTE_CONFIG;
+import static com.datadog.debugger.agent.DebuggerProductChangesListener.LOG_PROBE_PREFIX;
 import static java.util.Collections.singletonList;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -614,6 +615,14 @@ public void acceptNewDecorationSpan() {
     assertTrue(appliedDefinitions.containsKey(SPAN_DECORATION_ID.getEncodedId()));
   }
 
+  @Test
+  public void handleException() {
+    ConfigurationUpdater configurationUpdater = createConfigUpdater(debuggerSinkWithMockStatusSink);
+    Exception ex = new Exception("oops");
+    configurationUpdater.handleException(LOG_PROBE_PREFIX + PROBE_ID.getId(), ex);
+    verify(probeStatusSink).addError(eq(ProbeId.from(PROBE_ID.getId() + ":0")), eq(ex));
+  }
+
   private DebuggerTransformer createTransformer(
       Config tracerConfig,
       Configuration configuration,

@@ -7,7 +7,9 @@
 import static com.datadog.debugger.util.LogProbeTestHelper.parseTemplate;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.lenient;
 
@@ -44,15 +46,25 @@ public class DebuggerProductChangesListenerTest {
 
   class SimpleAcceptor implements ConfigurationAcceptor {
     private Collection<? extends ProbeDefinition> definitions;
+    private Exception lastException;
 
     @Override
     public void accept(Source source, Collection<? extends ProbeDefinition> definitions) {
       this.definitions = definitions;
     }
 
+    @Override
+    public void handleException(String configId, Exception ex) {
+      lastException = ex;
+    }
+
     public Collection<? extends ProbeDefinition> getDefinitions() {
       return definitions;
     }
+
+    public Exception getLastException() {
+      return lastException;
+    }
   }
 
   @BeforeEach
@@ -256,6 +268,24 @@ public void maxSpanDecorationProbes() {
     assertEquals(MAX_ALLOWED_SPAN_DECORATION_PROBES, acceptor.getDefinitions().size());
   }
 
+  @Test
+  public void parsingException() throws IOException {
+    SimpleAcceptor acceptor = new SimpleAcceptor();
+    DebuggerProductChangesListener listener =
+        new DebuggerProductChangesListener(tracerConfig, acceptor);
+    String probeUUID = UUID.randomUUID().toString();
+    IOException ioException =
+        assertThrows(
+            IOException.class,
+            () ->
+                listener.accept(
+                    createConfigKey("logProbe_" + probeUUID),
+                    "{bad json}".getBytes(StandardCharsets.UTF_8),
+                    pollingHinter));
+    assertNotNull(acceptor.lastException);
+    assertEquals(ioException.getCause(), acceptor.lastException);
+  }
+
   private void assertDefinitions(
       Collection<? extends ProbeDefinition> actualDefinitions,
       ProbeDefinition... expectedDefinitions) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,4 +10,6 @@ enum Source { @@
       }
       void accept(Source source, Collection<? extends ProbeDefinition> definitions);
+      void handleException(String configId, Exception ex);
     }