Skip to content

Commit

Permalink
Report exception when deserializing config (#7092)
Browse files Browse the repository at this point in the history 
Send probe diagnostic messages when an error occurred during config
deserialization (compatibility issue, unsupported feature, malformed
json, parsing bug)
  • Loading branch information
@jpbempel
jpbempel committed May 31, 2024
1 parent 7488d13 commit 11a1f41
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 24 deletions.
2 changes: 2 additions & 0 deletions ...-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationAcceptor.java
View file
Original file line number Diff 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);
}
31 changes: 31 additions & 0 deletions ...a-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down
57 changes: 33 additions & 24 deletions ...ent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerProductChangesListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
}

Expand Down
9 changes: 9 additions & 0 deletions ...ent/agent-debugger/src/test/java/com/datadog/debugger/agent/ConfigurationUpdaterTest.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions ...debugger/src/test/java/com/datadog/debugger/agent/DebuggerProductChangesListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 11a1f41

Please sign in to comment.