Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
e37b454
Relocating checks into try
dougqh Jun 12, 2024
e31388e
Refactoring if statement into multiple statements
dougqh Jun 12, 2024
a6e86c6
Sketching out structure of initialization telemetry
dougqh Jun 17, 2024
65d71d7
Adding "InitializationTelemetry"
dougqh Jul 5, 2024
1ea0d0d
spotless silliness
dougqh Jul 9, 2024
0f65b9f
Fixing resource leak in IntegrationTestUtils
dougqh Jul 9, 2024
122bf2b
Misc clean-up to improve test readability
dougqh Jul 11, 2024
7cc2251
spotless
dougqh Jul 12, 2024
b60ad48
Introducing utilities to help with exception handling
dougqh Jul 12, 2024
22e5542
Adding SecurityManager tests
dougqh Jul 12, 2024
d75864f
Added no network access test
dougqh Jul 12, 2024
b4623d6
tryGetProperty -> tryGetEnv
dougqh Jul 15, 2024
2453d4d
spotless
dougqh Jul 15, 2024
60c7981
Adding escaping of \
dougqh Jul 16, 2024
ca42c53
Adding toString for ease of debugging + spotless
dougqh Jul 18, 2024
405ca3c
Adding nonProxyHosts
dougqh Jul 18, 2024
34e3781
Starting to fully work
dougqh Jul 18, 2024
c0245ae
Pushing InitializationTelemetry into Agent & AgentCLI
dougqh Jul 29, 2024
6b5676a
Added a bit more header doc
dougqh Jul 29, 2024
9cc6da6
Merge remote-tracking branch 'origin/master' into dougqh/guardrails-t…
dougqh Jul 29, 2024
b5ae75a
none -> noneInstance
dougqh Jul 29, 2024
19cfbbd
spotless & restoring all telemetry tests
dougqh Jul 29, 2024
ef25275
Adjusting permissions to allow tests to pass on Java 17
dougqh Aug 2, 2024
a084432
Updating CustomSecurityManager to work on JDK21
dougqh Aug 2, 2024
a16fe75
Merge branch 'master' into dougqh/guardrails-telemetry
dougqh Aug 2, 2024
7694c6c
Enabling debugging in CI to understand failure
dougqh Aug 2, 2024
f4c19de
Merge branch 'dougqh/guardrails-telemetry' of https://github.com/Data…
dougqh Aug 2, 2024
623004a
Adding jdk properties to avoid blowing up JVMs
dougqh Aug 2, 2024
79cf2ea
Adding more properties to get JDK21 minimal access to pass
dougqh Aug 2, 2024
d44e73c
Finishing incomplete javadoc
dougqh Aug 6, 2024
3812431
A bit of clean-up based on review feedback
dougqh Aug 8, 2024
97b429f
Added some comments to explain the need for the proxy
dougqh Aug 8, 2024
60406b4
Refactoring to be easier to test
dougqh Aug 8, 2024
f44d294
Increased visibility to be accessible in test
dougqh Aug 9, 2024
de0b7eb
Pushing a test for help debugging
dougqh Aug 9, 2024
fe2a16a
Adding call to endsValue in endObject
dougqh Aug 12, 2024
319b87b
Function BootstrapInitializationTelemetry test
dougqh Aug 12, 2024
a9ffb10
Merge branch 'master' into dougqh/guardrails-telemetry
dougqh Aug 13, 2024
bf26a18
Fixing erroneous codeNarc error
dougqh Aug 13, 2024
2754fa2
Merge branch 'dougqh/guardrails-telemetry' of https://github.com/Data…
dougqh Aug 13, 2024
b6a80db
Making codeNarc comment a bit more descriptive + spotless
dougqh Aug 13, 2024
d729531
Merge branch 'master' into dougqh/guardrails-telemetry
dougqh Aug 14, 2024
79197c1
Switching to flush as suggested in review
dougqh Aug 14, 2024
deb8831
Merge branch 'dougqh/guardrails-telemetry' of https://github.com/Data…
dougqh Aug 14, 2024
13e5272
Adding final
dougqh Aug 19, 2024
536b940
Revert "Adding final"
dougqh Aug 19, 2024
5784379
Adding final
dougqh Aug 19, 2024
93c5107
Fixing backwards null checking logic
dougqh Aug 19, 2024
73dca78
Cleaning up throws clause
dougqh Aug 19, 2024
e95d976
Merge branch 'dougqh/guardrails-telemetry' of https://github.com/Data…
dougqh Aug 19, 2024
c0b3f77
Changing none to noop
dougqh Aug 19, 2024
d1d886b
Removing the finals, since the class is now final
dougqh Aug 19, 2024
7137af6
Adding JsonBufferTest & expanding escape sequences
dougqh Aug 19, 2024
91a892e
Adding thread safety info
dougqh Aug 19, 2024
aa76a5c
spotless
dougqh Aug 19, 2024
416f728
Adding more JSON tests - nesting, partials, and reset
dougqh Aug 19, 2024
83b0eaa
Null handling test for InitializationTelemetry wrapper
dougqh Aug 19, 2024
08ffeb7
Refactor to introduce Result class
dougqh Aug 21, 2024
982282f
Fixing issue - forwarder wasn't flushing the JSON
dougqh Aug 21, 2024
651b125
Completing the integration tests
dougqh Aug 21, 2024
01dccb0
Merge branch 'master' into dougqh/guardrails-telemetry
dougqh Aug 22, 2024
22f9192
spotless
dougqh Aug 22, 2024
e1e17fc
Merge branch 'master' into dougqh/guardrails-telemetry
dougqh Aug 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,19 @@ public boolean isEnabledByDefault() {
private static boolean debuggerEnabled = false;
private static boolean agentlessLogSubmissionEnabled = false;

public static void start(final Instrumentation inst, final URL agentJarURL, String agentArgs) {
/**
* Starts the agent; returns a boolean indicating if Agent started successfully
*
* <p>The Agent is considered to start successfully if Instrumentation can be activated. All other
* pieces are considered optional.
*/
public static void start(
final Object bootstrapInitTelemetry,
final Instrumentation inst,
final URL agentJarURL,
final String agentArgs) {
InitializationTelemetry initTelemetry = InitializationTelemetry.proxy(bootstrapInitTelemetry);

StaticEventLogger.begin("Agent");
StaticEventLogger.begin("Agent.start");

Expand All @@ -163,8 +175,9 @@ public static void start(final Instrumentation inst, final URL agentJarURL, Stri
remoteConfigEnabled = false;
telemetryEnabled = false;
// apply trace instrumentation, but skip starting other services
startDatadogAgent(inst);
startDatadogAgent(initTelemetry, inst);
StaticEventLogger.end("Agent.start");

return;
}

Expand Down Expand Up @@ -279,7 +292,7 @@ public void run() {
* when it will happen after the class transformers were added.
*/
AgentTaskScheduler.initialize();
startDatadogAgent(inst);
startDatadogAgent(initTelemetry, inst);

final EnumSet<Library> libraries = detectLibraries(log);

Expand Down Expand Up @@ -326,7 +339,7 @@ public void run() {
* logging facility. Likewise on IBM JDKs OkHttp may indirectly load 'IBMSASL' which in turn loads LogManager.
*/
InstallDatadogTracerCallback installDatadogTracerCallback =
new InstallDatadogTracerCallback(inst);
new InstallDatadogTracerCallback(initTelemetry, inst);
if (delayOkHttp) {
log.debug("Custom logger detected. Delaying Datadog Tracer initialization.");
registerLogManagerCallback(installDatadogTracerCallback);
Expand Down Expand Up @@ -397,10 +410,10 @@ public static synchronized Class<?> installAgentCLI() throws Exception {
}

/** Used by AgentCLI to send sample traces from the command-line. */
public static void startDatadogTracer() throws Exception {
public static void startDatadogTracer(InitializationTelemetry initTelemetry) throws Exception {
Class<?> scoClass =
AGENT_CLASSLOADER.loadClass("datadog.communication.ddagent.SharedCommunicationObjects");
installDatadogTracer(scoClass, scoClass.getConstructor().newInstance());
installDatadogTracer(initTelemetry, scoClass, scoClass.getConstructor().newInstance());
startJmx(); // send runtime metrics along with the traces
}

Expand Down Expand Up @@ -475,9 +488,12 @@ public void execute() {
}

protected static class InstallDatadogTracerCallback extends ClassLoadCallBack {
private final InitializationTelemetry initTelemetry;
private final Instrumentation instrumentation;

public InstallDatadogTracerCallback(Instrumentation instrumentation) {
public InstallDatadogTracerCallback(
InitializationTelemetry initTelemetry, Instrumentation instrumentation) {
this.initTelemetry = initTelemetry;
this.instrumentation = instrumentation;
}

Expand All @@ -502,7 +518,7 @@ public void execute() {
throw new UndeclaredThrowableException(e);
}

installDatadogTracer(scoClass, sco);
installDatadogTracer(initTelemetry, scoClass, sco);
maybeStartAppSec(scoClass, sco);
maybeStartIast(scoClass, sco);
maybeStartCiVisibility(instrumentation, scoClass, sco);
Expand Down Expand Up @@ -582,9 +598,9 @@ private static void maybeStartRemoteConfig(Class<?> scoClass, Object sco) {
StaticEventLogger.end("Remote Config");
}

private static synchronized void startDatadogAgent(final Instrumentation inst) {
private static synchronized void startDatadogAgent(
final InitializationTelemetry initTelemetry, final Instrumentation inst) {
if (null != inst) {

StaticEventLogger.begin("BytebuddyAgent");

try {
Expand All @@ -595,13 +611,15 @@ private static synchronized void startDatadogAgent(final Instrumentation inst) {
agentInstallerMethod.invoke(null, inst);
} catch (final Throwable ex) {
log.error("Throwable thrown while installing the Datadog Agent", ex);
initTelemetry.onFatalError(ex);
} finally {
StaticEventLogger.end("BytebuddyAgent");
}

StaticEventLogger.end("BytebuddyAgent");
}
}

private static synchronized void installDatadogTracer(Class<?> scoClass, Object sco) {
private static synchronized void installDatadogTracer(
InitializationTelemetry initTelemetry, Class<?> scoClass, Object sco) {
if (AGENT_CLASSLOADER == null) {
throw new IllegalStateException("Datadog agent should have been started already");
}
Expand All @@ -622,6 +640,8 @@ private static synchronized void installDatadogTracer(Class<?> scoClass, Object
throw ex;
} catch (final Throwable ex) {
log.error("Throwable thrown while installing the Datadog Tracer", ex);

initTelemetry.onFatalError(ex);
}

StaticEventLogger.end("GlobalTracer");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
package datadog.trace.bootstrap;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;

/**
* Thread safe wrapper around BootstrapInitializationTelemetry used inside the Datadog ClassLoader.
*
* <p>Right now, this is needed because of the build separation between the two portions of the
* bootstrap. We should consider adjusting the build to allow Agent et al to reference
* BootstrapInitializationTelemetry, then we could remove this proxy.
*/
public abstract class InitializationTelemetry {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we have a static instance for InitializationTelemetry and use it like any other logger?
It can be none/noop until to be set in the Agent.start() or if bootstrap telemetry is wrong.

It feels way more easier to use it later where we will need it rather than adding one additional parameter to every function and not setting it for AgentCLI will be transparent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really want to have a mutable static, so I decided to pass the InitializationTelemetry instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some tests for BootstrapInitializationTelemetry. I'll add some for JsonBuffer, too.

/** Returns a proxy around a BoostrapInitializationTelemetry object */
public static final InitializationTelemetry proxy(Object bootstrapInitTelemetry) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final InitializationTelemetry proxy(Object bootstrapInitTelemetry) {
public static InitializationTelemetry proxy(Object bootstrapInitTelemetry) {

Copy link
Contributor Author

@dougqh dougqh Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep final almost everywhere, but I decided to remove the final where it is redundant because the class is final.

In this case, the class isn't final. So I'm keeping the final because I don't want someone to override this method on a child class.

if (bootstrapInitTelemetry == null) {
return InitializationTelemetry.noOpInstance();
} else {
return new BootstrapProxy(bootstrapInitTelemetry);
}
}

/** Returns a singleton of the no op InitializationTelemetry */
public static final InitializationTelemetry noOpInstance() {
return NoOp.INSTANCE;
}

/**
* Indicates that an abort condition occurred during the bootstrapping process. Abort conditions
* are assumed to leave the bootstrapping process incomplete. {@link #markIncomplete()}
*/
public abstract void onAbort(String reasonCode);

/**
* Indicates that an exception occurred during the bootstrapping process By default the exception
* is assumed to NOT have fully stopped the initialization of the tracer.
*
* <p>If this exception stops the core bootstrapping of the tracer, then {@link #markIncomplete()}
* should also be called.
*/
public abstract void onError(Throwable t);

/**
* Indicates an exception that occurred during the bootstrapping process that left initialization
* incomplete. Equivalent to calling {@link #onError(Throwable)} and {@link #markIncomplete()}
*/
public abstract void onFatalError(Throwable t);

/**
* Indicates that an exception conditional occurred during the bootstrapping process. By default
* the exceptional condition is assumed to NOT have fully stopped the initialization of the
* tracer.
*
* <p>If this exception stops the core bootstrapping of the tracer, then {@link #markIncomplete()}
* should also be called.
*/
public abstract void onError(String reasonCode);

/**
* Marks bootstrapping of tracer as an incomplete Should only be called when a core (e.g.
* non-optional) component fails to initialize
*/
public abstract void markIncomplete();

/** No telemetry - used for delayed initialization outside bootstrap invocation */
static final class NoOp extends InitializationTelemetry {
static final NoOp INSTANCE = new NoOp();

NoOp() {}

@Override
public void onAbort(String reasonCode) {}

@Override
public void onError(String reasonCode) {}

@Override
public void onError(Throwable t) {}

@Override
public void onFatalError(Throwable t) {}

@Override
public void markIncomplete() {}
}

/** Reflective proxy to BootstrapInitializationTelemetry */
static final class BootstrapProxy extends InitializationTelemetry {
private final Object bootstrapInitTelemetry;
private volatile MethodHandle bmh_onAbortString;
private volatile MethodHandle bmh_onErrorString;
private volatile MethodHandle bmh_onErrorThrowable;
private volatile MethodHandle bmh_onFatalErrorThrowable;
private volatile MethodHandle bmh_markIncomplete;

// DQH - Decided not to eager access MethodHandles, since exceptions are uncommon
// However, MethodHandles are cached on lookup

/** @param bootstrapInitTelemetry - non-null BootstrapInitializationTelemetry */
BootstrapProxy(final Object bootstrapInitTelemetry) {
this.bootstrapInitTelemetry = bootstrapInitTelemetry;
}

@Override
public void onAbort(String reasonCode) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to figure out a better way to test this proxy.
Right now, I'm doing an integration test from the bootstrap, but it doesn't cover all the methods.

MethodHandle bmh_onAbortString = this.bmh_onAbortString;
if (bmh_onAbortString == null) {
bmh_onAbortString = findBoundHandle("onAbort", String.class);
this.bmh_onAbortString = bmh_onAbortString;
}
if (bmh_onAbortString != null) {
try {
bmh_onAbortString.invokeExact(reasonCode);
} catch (Throwable t) {
// ignore
}
}
}

@Override
public void onError(String reasonCode) {
MethodHandle bmh_onErrorString = this.bmh_onErrorString;
if (bmh_onErrorString == null) {
bmh_onErrorString = findBoundHandle("onError", String.class);
this.bmh_onErrorString = bmh_onErrorString;
}
if (bmh_onErrorString != null) {
try {
bmh_onErrorString.invokeExact(reasonCode);
} catch (Throwable t) {
// ignore
}
}
}

@Override
public void onError(Throwable cause) {
MethodHandle bmh_onErrorThrowable = this.bmh_onErrorThrowable;
if (bmh_onErrorThrowable == null) {
bmh_onErrorThrowable = findBoundHandle("onError", Throwable.class);
this.bmh_onErrorThrowable = bmh_onErrorThrowable;
}
if (bmh_onErrorThrowable != null) {
try {
bmh_onErrorThrowable.invokeExact(cause);
} catch (Throwable t) {
// ignore
}
}
}

@Override
public void onFatalError(Throwable cause) {
MethodHandle bmh_onFatalErrorThrowable = this.bmh_onFatalErrorThrowable;
if (bmh_onFatalErrorThrowable == null) {
bmh_onFatalErrorThrowable = findBoundHandle("onFatalError", Throwable.class);
this.bmh_onFatalErrorThrowable = bmh_onFatalErrorThrowable;
}
if (bmh_onFatalErrorThrowable != null) {
try {
bmh_onFatalErrorThrowable.invokeExact(cause);
} catch (Throwable t) {
// ignore
}
}
}

@Override
public void markIncomplete() {
MethodHandle bmh_markIncomplete = this.bmh_markIncomplete;
if (bmh_markIncomplete == null) {
bmh_markIncomplete = findBoundHandle("markIncomplete");
this.bmh_markIncomplete = bmh_markIncomplete;
}
if (bmh_markIncomplete != null) {
try {
bmh_markIncomplete.invokeExact();
} catch (Throwable t) {
// ignore
}
}
}

private final MethodHandle findBoundHandle(String name, Class<?> paramType) {
try {
MethodHandle virtualHandle =
MethodHandles.publicLookup()
.findVirtual(
bootstrapInitTelemetry.getClass(),
name,
MethodType.methodType(void.class, paramType));

return virtualHandle.bindTo(bootstrapInitTelemetry);
} catch (NoSuchMethodException | IllegalAccessException e) {
return null;
}
}

private final MethodHandle findBoundHandle(String name) {
try {
MethodHandle virtualHandle =
MethodHandles.publicLookup()
.findVirtual(
bootstrapInitTelemetry.getClass(), name, MethodType.methodType(void.class));

return virtualHandle.bindTo(bootstrapInitTelemetry);
} catch (NoSuchMethodException | IllegalAccessException e) {
return null;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package datadog.trace.bootstrap

import datadog.trace.test.util.DDSpecification

class InitializationTelemetryTest extends DDSpecification {
def "telemetry wrapper - null case"() {
expect:
InitializationTelemetry.proxy(null) == InitializationTelemetry.noOpInstance()
}

def "telemetry wrapper - wrap bootstrap"() {
// TODO: Figure out how to test the wrapper fully
expect:
InitializationTelemetry.proxy(new Object()) != InitializationTelemetry.noOpInstance()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import datadog.trace.agent.tooling.bytebuddy.SharedTypePools;
import datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers;
import datadog.trace.bootstrap.Agent;
import datadog.trace.bootstrap.InitializationTelemetry;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import java.io.File;
Expand Down Expand Up @@ -53,7 +54,7 @@ public static void printIntegrationNames() {
* @param interval the interval (in seconds) to wait for each trace
*/
public static void sendSampleTraces(final int count, final double interval) throws Exception {
Agent.startDatadogTracer();
Agent.startDatadogTracer(InitializationTelemetry.noOpInstance());

int numTraces = 0;
while (++numTraces <= count || count < 0) {
Expand Down
Loading