Skip to content

Commit

Permalink
Delay generation of costly config elements (#4302)
Browse files Browse the repository at this point in the history
* Update SQS 2.0 instrumentation to avoid touching main config during native-image build
* Move directAllocationProfilingEnabled flag to InstrumenterConfig as it controls directbytebuffer instrumentation
* Begin clean-up of instrumenter activation
* Move instrumenter target system flags to InstrumenterConfig
* Print out instrumenter config first
* Make these config elements lazy to avoid pulling in types such as ThreadPoolExecutor before the agent is installed
  • Loading branch information
mcculls authored Nov 24, 2022
1 parent c9093b0 commit 50a78b0
Show file tree
Hide file tree
Showing 23 changed files with 256 additions and 255 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ private static boolean isFeatureEnabled(AgentFeature feature) {
}
}

/** @see datadog.trace.api.ProductActivationConfig#fromString(String) */
/** @see datadog.trace.api.ProductActivation#fromString(String) */
private static boolean isAppSecFullyDisabled() {
// must be kept in sync with logic from Config!
final String featureEnabledSysprop = AgentFeature.APPSEC.systemProp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@
import datadog.trace.agent.tooling.bytebuddy.DDOutlinePoolStrategy;
import datadog.trace.agent.tooling.bytebuddy.SharedTypePools;
import datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers;
import datadog.trace.api.Config;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.api.IntegrationsCollector;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.bootstrap.FieldBackedContextAccessor;
import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
import datadog.trace.util.AgentTaskScheduler;
Expand Down Expand Up @@ -49,18 +48,14 @@ public class AgentInstaller {

public static void installBytebuddyAgent(final Instrumentation inst) {
/*
* ByteBuddy agent is used by tracing, profiling, appsec and civisibility and since they can
* be enabled independently we need to install the agent when either of them
* is active.
* ByteBuddy agent is used by several systems which can be enabled independently;
* we need to install the agent whenever any of them is active.
*/
if (Config.get().isTraceEnabled()
|| Config.get().isProfilingEnabled()
|| Config.get().getAppSecEnabledConfig() != ProductActivationConfig.FULLY_DISABLED
|| Config.get().isIastEnabled()
|| Config.get().isCiVisibilityEnabled()) {
installBytebuddyAgent(inst, false, new AgentBuilder.Listener[0]);
Set<Instrumenter.TargetSystem> enabledSystems = getEnabledSystems();
if (!enabledSystems.isEmpty()) {
installBytebuddyAgent(inst, false, enabledSystems);
if (DEBUG) {
log.debug("Class instrumentation installed");
log.debug("Instrumentation installed for {}", enabledSystems);
}
int poolCleaningInterval = InstrumenterConfig.get().getResolverResetInterval();
if (poolCleaningInterval > 0) {
Expand All @@ -71,19 +66,19 @@ public static void installBytebuddyAgent(final Instrumentation inst) {
TimeUnit.SECONDS);
}
} else if (DEBUG) {
log.debug("There are not any enabled subsystems, not installing instrumentations.");
log.debug("No target systems enabled, skipping instrumentation.");
}
}

/**
* Install the core bytebuddy agent along with all implementations of {@link Instrumenter}.
*
* @param inst Java Instrumentation used to install bytebuddy
* @return the agent's class transformer
*/
public static ResettableClassFileTransformer installBytebuddyAgent(
final Instrumentation inst,
final boolean skipAdditionalLibraryMatcher,
final Set<Instrumenter.TargetSystem> enabledSystems,
final AgentBuilder.Listener... listeners) {
Utils.setInstrumentation(inst);

Expand Down Expand Up @@ -154,7 +149,6 @@ public static ResettableClassFileTransformer installBytebuddyAgent(
AgentTransformerBuilder transformerBuilder = new AgentTransformerBuilder(agentBuilder);

int installedCount = 0;
Set<Instrumenter.TargetSystem> enabledSystems = getEnabledSystems();
for (Instrumenter instrumenter : instrumenters) {
if (!instrumenter.isApplicable(enabledSystems)) {
if (DEBUG) {
Expand All @@ -177,7 +171,7 @@ public static ResettableClassFileTransformer installBytebuddyAgent(
log.debug("Installed {} instrumenter(s)", installedCount);
}

if (Config.get().isTelemetryEnabled()) {
if (InstrumenterConfig.get().isTelemetryEnabled()) {
InstrumenterState.setObserver(
new InstrumenterState.Observer() {
@Override
Expand All @@ -195,17 +189,17 @@ public void applied(Iterable<String> instrumentationNames) {
}
}

private static Set<Instrumenter.TargetSystem> getEnabledSystems() {
public static Set<Instrumenter.TargetSystem> getEnabledSystems() {
EnumSet<Instrumenter.TargetSystem> enabledSystems =
EnumSet.noneOf(Instrumenter.TargetSystem.class);
Config cfg = Config.get();
InstrumenterConfig cfg = InstrumenterConfig.get();
if (cfg.isTraceEnabled()) {
enabledSystems.add(Instrumenter.TargetSystem.TRACING);
}
if (cfg.isProfilingEnabled()) {
enabledSystems.add(Instrumenter.TargetSystem.PROFILING);
}
if (cfg.getAppSecEnabledConfig() != ProductActivationConfig.FULLY_DISABLED) {
if (cfg.getAppSecActivation() != ProductActivation.FULLY_DISABLED) {
enabledSystems.add(Instrumenter.TargetSystem.APPSEC);
}
if (cfg.isIastEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import datadog.trace.agent.tooling.WeakMaps;
import datadog.trace.api.Config;
import datadog.trace.api.Platform;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.Instrumentation;
Expand Down Expand Up @@ -38,7 +38,7 @@ public class AgentInstaller {
public static void installBytebuddyAgent(Instrumentation inst) {
if (Config.get().isTraceEnabled()
|| Config.get().isProfilingEnabled()
|| Config.get().getAppSecEnabledConfig() != ProductActivationConfig.FULLY_DISABLED
|| Config.get().getAppSecActivation() != ProductActivation.FULLY_DISABLED
|| Config.get().isCiVisibilityEnabled()) {
Utils.setInstrumentation(inst);
installClassTransformer(inst);
Expand Down Expand Up @@ -175,7 +175,7 @@ private static Set<Instrumenter.TargetSystem> getEnabledSystems() {
if (cfg.isProfilingEnabled()) {
enabledSystems.add(Instrumenter.TargetSystem.PROFILING);
}
if (cfg.getAppSecEnabledConfig() != ProductActivationConfig.FULLY_DISABLED) {
if (cfg.getAppSecActivation() != ProductActivation.FULLY_DISABLED) {
enabledSystems.add(Instrumenter.TargetSystem.APPSEC);
}
if (cfg.isCiVisibilityEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,15 @@
*/
public interface Instrumenter {
/**
* Since several subsystems are sharing the same instrumentation infrastructure in order to enable
* only the applicable {@link Instrumenter instrumenters} on startup each {@linkplain
* Instrumenter} type must declare its target system. Four systems are currently supported
* Since several systems share the same instrumentation infrastructure in order to enable only the
* applicable {@link Instrumenter instrumenters} on startup each {@linkplain Instrumenter} type
* must declare its target system. Five systems are currently supported:
*
* <ul>
* <li>{@link TargetSystem#TRACING tracing}
* <li>{@link TargetSystem#PROFILING profiling}
* <li>{@link TargetSystem#APPSEC appsec}
* <li>{@link TargetSystem#IAST iast}
* <li>{@link TargetSystem#CIVISIBILITY ci-visibility}
* </ul>
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import datadog.communication.monitor.Monitoring;
import datadog.remoteconfig.ConfigurationPoller;
import datadog.trace.api.Config;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.api.gateway.SubscriptionService;
import datadog.trace.api.time.SystemTimeSource;
import datadog.trace.bootstrap.ActiveSubsystems;
Expand Down Expand Up @@ -50,8 +50,8 @@ public static void start(SubscriptionService gw, SharedCommunicationObjects sco)

private static void doStart(SubscriptionService gw, SharedCommunicationObjects sco) {
final Config config = Config.get();
ProductActivationConfig appSecEnabledConfig = config.getAppSecEnabledConfig();
if (appSecEnabledConfig == ProductActivationConfig.FULLY_DISABLED) {
ProductActivation appSecEnabledConfig = config.getAppSecActivation();
if (appSecEnabledConfig == ProductActivation.FULLY_DISABLED) {
log.debug("AppSec: disabled");
return;
}
Expand Down Expand Up @@ -81,7 +81,7 @@ private static void doStart(SubscriptionService gw, SharedCommunicationObjects s
loadModules(eventDispatcher);
gatewayBridge.init();

setActive(appSecEnabledConfig == ProductActivationConfig.FULLY_ENABLED);
setActive(appSecEnabledConfig == ProductActivation.FULLY_ENABLED);

APP_SEC_CONFIG_SERVICE.maybeSubscribeConfigPolling();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.google.auto.service.AutoService;
import com.squareup.moshi.*;
import datadog.trace.api.Config;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.api.gateway.Flow;
import io.sqreen.powerwaf.Additive;
import io.sqreen.powerwaf.Powerwaf;
Expand Down Expand Up @@ -164,8 +164,8 @@ public void config(AppSecModuleConfigurer appSecConfigService)
Optional<Object> initialConfig =
appSecConfigService.addSubConfigListener("waf", this::applyConfig);

ProductActivationConfig appSecEnabledConfig = Config.get().getAppSecEnabledConfig();
if (appSecEnabledConfig == ProductActivationConfig.FULLY_ENABLED) {
ProductActivation appSecEnabledConfig = Config.get().getAppSecActivation();
if (appSecEnabledConfig == ProductActivation.FULLY_ENABLED) {
if (!initialConfig.isPresent()) {
throw new AppSecModuleActivationException("No initial config for WAF");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ public SqsJmsMessageInstrumentation() {
super("aws-sdk");
}

@Override
protected boolean defaultEnabled() {
return super.defaultEnabled() && Config.get().isSqsPropagationEnabled();
}

@Override
public String instrumentedType() {
return "com.amazon.sqs.javamessaging.message.SQSMessage";
Expand All @@ -44,13 +39,15 @@ public static class CopyTracePropertyAdvice {
public static void onExit(
@Advice.Argument(2) Message sqsMessage, @Advice.FieldValue("properties") Map properties)
throws JMSException {
Map<String, String> systemAttributes = sqsMessage.attributesAsStrings();
if (null != systemAttributes) {
String awsTraceHeader = systemAttributes.get("AWSTraceHeader");
if (null != awsTraceHeader && !awsTraceHeader.isEmpty()) {
properties.put(
"x__dash__amzn__dash__trace__dash__id", // X-Amzn-Trace-Id, encoded for JMS
new SQSMessage.JMSMessagePropertyValue(awsTraceHeader, STRING));
if (Config.get().isSqsPropagationEnabled()) {
Map<String, String> systemAttributes = sqsMessage.attributesAsStrings();
if (null != systemAttributes) {
String awsTraceHeader = systemAttributes.get("AWSTraceHeader");
if (null != awsTraceHeader && !awsTraceHeader.isEmpty()) {
properties.put(
"x__dash__amzn__dash__trace__dash__id", // X-Amzn-Trace-Id, encoded for JMS
new SQSMessage.JMSMessagePropertyValue(awsTraceHeader, STRING));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ public SqsReceiveRequestInstrumentation() {
super("aws-sdk");
}

@Override
protected boolean defaultEnabled() {
return super.defaultEnabled() && Config.get().isSqsPropagationEnabled();
}

@Override
public String instrumentedType() {
return "software.amazon.awssdk.services.sqs.model.ReceiveMessageRequest$BuilderImpl";
Expand All @@ -47,15 +42,17 @@ public static void onEntry(
@Advice.FieldValue(value = "attributeNames", readOnly = false)
List<String> attributeNames) {
// ReceiveMessageRequest.BuilderImpl maintains an immutable list which we may need to replace
for (String name : attributeNames) {
if ("AWSTraceHeader".equals(name) || "All".equals(name)) {
return;
if (Config.get().isSqsPropagationEnabled()) {
for (String name : attributeNames) {
if ("AWSTraceHeader".equals(name) || "All".equals(name)) {
return;
}
}
int oldLength = attributeNames.size();
String[] nameArray = attributeNames.toArray(new String[oldLength + 1]);
nameArray[oldLength] = "AWSTraceHeader";
attributeNames = asList(nameArray);
}
int oldLength = attributeNames.size();
String[] nameArray = attributeNames.toArray(new String[oldLength + 1]);
nameArray[oldLength] = "AWSTraceHeader";
attributeNames = asList(nameArray);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.Config;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.api.Platform;

@AutoService(Instrumenter.class)
Expand All @@ -26,7 +26,7 @@ public boolean isEnabled() {

@Override
protected boolean defaultEnabled() {
return Config.get().isDirectAllocationProfilingEnabled();
return InstrumenterConfig.get().isDirectAllocationProfilingEnabled();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.Config;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.api.Platform;

@AutoService(Instrumenter.class)
Expand All @@ -24,7 +24,7 @@ public boolean isEnabled() {

@Override
protected boolean defaultEnabled() {
return Config.get().isDirectAllocationProfilingEnabled();
return InstrumenterConfig.get().isDirectAllocationProfilingEnabled();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.Config;
import datadog.trace.api.InstrumenterConfig;
import datadog.trace.api.Platform;

@AutoService(Instrumenter.class)
Expand All @@ -25,7 +25,7 @@ public boolean isEnabled() {

@Override
protected boolean defaultEnabled() {
return Config.get().isDirectAllocationProfilingEnabled();
return InstrumenterConfig.get().isDirectAllocationProfilingEnabled();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import datadog.trace.agent.tooling.ExcludeFilterProvider;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.Config;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
import datadog.trace.instrumentation.jetty9.HttpChannelHandleVisitor;
import java.security.ProtectionDomain;
Expand Down Expand Up @@ -111,7 +111,7 @@ public ClassVisitor wrap(
MethodList<?> methods,
int writerFlags,
int readerFlags) {
if (Config.get().getAppSecEnabledConfig() == ProductActivationConfig.FULLY_DISABLED) {
if (Config.get().getAppSecActivation() == ProductActivation.FULLY_DISABLED) {
return classVisitor;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import datadog.trace.api.Config;
import datadog.trace.api.CorrelationIdentifier;
import datadog.trace.api.GlobalTracer;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.bootstrap.InstrumentationContext;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
Expand Down Expand Up @@ -119,7 +119,7 @@ public ClassVisitor wrap(
MethodList<?> methods,
int writerFlags,
int readerFlags) {
if (Config.get().getAppSecEnabledConfig() == ProductActivationConfig.FULLY_DISABLED) {
if (Config.get().getAppSecActivation() == ProductActivation.FULLY_DISABLED) {
return classVisitor;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import datadog.trace.api.Config;
import datadog.trace.api.CorrelationIdentifier;
import datadog.trace.api.GlobalTracer;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.bootstrap.InstrumentationContext;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
Expand Down Expand Up @@ -118,7 +118,7 @@ public ClassVisitor wrap(
MethodList<?> methods,
int writerFlags,
int readerFlags) {
if (Config.get().getAppSecEnabledConfig() == ProductActivationConfig.FULLY_DISABLED) {
if (Config.get().getAppSecActivation() == ProductActivation.FULLY_DISABLED) {
return classVisitor;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import datadog.trace.api.Config;
import datadog.trace.api.CorrelationIdentifier;
import datadog.trace.api.GlobalTracer;
import datadog.trace.api.ProductActivationConfig;
import datadog.trace.api.ProductActivation;
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
Expand Down Expand Up @@ -133,7 +133,7 @@ public ClassVisitor wrap(
MethodList<?> methods,
int writerFlags,
int readerFlags) {
if (Config.get().getAppSecEnabledConfig() == ProductActivationConfig.FULLY_DISABLED) {
if (Config.get().getAppSecActivation() == ProductActivation.FULLY_DISABLED) {
return classVisitor;
}

Expand Down
Loading

0 comments on commit 50a78b0

Please sign in to comment.