Skip to content

Commit af2d214

Browse files
PerfectSlayerPerfectSlayer
authored
Make decorator exception safe 3/3 (#12017)
feat(bootstrap): Make decorator exception safe feat(bootstrap): Add nullability information feat(doc): Update code pattern from documentation fix(bootstrap): Fix unit tests feat(doc): Update instrumentations documentation fix(synpase): Revert instrumentation changes fix(openai): Fix parent call feat(bootstrap): Make decorator exception safe feat(bootstrap): Make decorator exception safe feat(bootstrap): Make decorator exception safe feat(bootstrap): Make decorator exception safe feat(bootstrap): Make decorator exception safe feat(bootstrap): Make decorator exception safe feat(bootstrap): Add null type info to decorator feat(bootstrap): Make decorator exception safe feat(bootstrap): Add null type info to decorator feat(bootstrap): Add null type info to decorator feat(bootstrap): Add null type info to decorator feat(bootstrap): Make decorator exception safe feat(http-client): Simplify instrumentation feat(axway): Simplify instrumentation feat(http-client): Simplify instrumentation feat(http-client): Simplify instrumentation feat(hazelcast): Simplify instrumentation feat(ignite): Simplify instrumentation feat(openai): Simplify instrumentation feat(spark): Simplify instrumentation feat(twilio): Simplify instrumentation feat(websocket): Simplify instrumentation feat(openai): Simplify instrumentation feat(websocket): Simplify instrumentation feat(hazelcast): Simplify instrumentation feat(axway): Simplify instrumentation fix(doc): Fix typo fix(bootstrap): Restore class qualifier for readability feat(bootstrap): Handle null check at public entry points fix(bootstrap): Restoring qualifier for readability fix(bootstrap): Fix null type annotation fix(instrumentation): Fix null type annotation fix(core): Fix null type annotation fix(instrumentation): Fix null type annotation fix(instrumentation): Fix logger initialization time Make sure logger is initialize before the singleton instance fix(instrumentation): Cache span as local var fix(instrumentation): Fix qualifier consistency feat(http-client): Simplify instrumentation feat(ignite): Simplify instrumentation feat(hazelcast): Simplify instrumentation feat(twilio): Simplify instrumentation feat(liberty): Simplify instrumentation Co-authored-by: bruce.bujon <bruce.bujon@datadoghq.com>
1 parent f381459 commit af2d214

69 files changed

Lines changed: 617 additions & 424 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java‎

Lines changed: 66 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver.hostName;
44

5+
import datadog.appsec.api.blocking.BlockingException;
56
import datadog.context.Context;
67
import datadog.context.ContextScope;
78
import datadog.trace.api.Config;
@@ -20,8 +21,15 @@
2021
import java.net.InetSocketAddress;
2122
import java.util.concurrent.ExecutionException;
2223
import java.util.function.Function;
24+
import javax.annotation.Nullable;
25+
import javax.annotation.ParametersAreNonnullByDefault;
26+
import org.slf4j.Logger;
27+
import org.slf4j.LoggerFactory;
2328

29+
@ParametersAreNonnullByDefault
2430
public abstract class BaseDecorator {
31+
private static final Logger log = LoggerFactory.getLogger(BaseDecorator.class);
32+
2533
protected static final int UNSET_PORT = 0;
2634

2735
private static final QualifiedClassNameCache CLASS_NAMES =
@@ -90,7 +98,17 @@ protected boolean traceAnalyticsDefault() {
9098
return false;
9199
}
92100

93-
public void afterStart(final AgentSpan span) {
101+
public final void afterStart(final AgentSpan span) {
102+
try {
103+
doAfterStart(span);
104+
} catch (BlockingException e) {
105+
throw e;
106+
} catch (Throwable t) {
107+
log.debug("Failed to decorate span after start", t);
108+
}
109+
}
110+
111+
protected void doAfterStart(final AgentSpan span) {
94112
if (spanType() != null) {
95113
span.setSpanType(spanType());
96114
}
@@ -106,50 +124,80 @@ public void afterStart(final AgentSpan span) {
106124
span.setMetric(traceAnalyticsEntry);
107125
}
108126

109-
public void beforeFinish(final ContextScope scope) {
110-
beforeFinish(scope.context());
127+
public final void beforeFinish(@Nullable final ContextScope scope) {
128+
if (scope != null) {
129+
beforeFinish(scope.context());
130+
}
131+
}
132+
133+
public final void beforeFinish(@Nullable final AgentSpan span) {
134+
if (span != null) {
135+
beforeFinish((Context) span);
136+
}
111137
}
112138

113-
public void beforeFinish(final AgentSpan span) {}
139+
public final void beforeFinish(final Context context) {
140+
try {
141+
doBeforeFinish(context);
142+
} catch (BlockingException e) {
143+
throw e;
144+
} catch (Throwable t) {
145+
log.debug("Failed to decorate span before finish", t);
146+
}
147+
}
114148

115-
public void beforeFinish(final Context context) {}
149+
protected void doBeforeFinish(final Context context) {}
116150

117-
public void onError(final AgentScope scope, final Throwable throwable) {
151+
public final void onError(@Nullable final AgentScope scope, @Nullable final Throwable throwable) {
118152
if (scope != null) {
119153
onError(scope.span(), throwable);
120154
}
121155
}
122156

123-
public void onError(final AgentSpan span, final Throwable throwable) {
157+
public final void onError(@Nullable final AgentSpan span, @Nullable final Throwable throwable) {
124158
onError(span, throwable, ErrorPriorities.DEFAULT);
125159
}
126160

127-
public void onError(final AgentSpan span, final Throwable throwable, byte errorPriority) {
128-
if (throwable != null && span != null) {
129-
span.addThrowable(
130-
throwable instanceof ExecutionException ? throwable.getCause() : throwable,
131-
errorPriority);
161+
public final void onError(
162+
@Nullable final AgentSpan span, @Nullable final Throwable throwable, byte errorPriority) {
163+
if (span != null && throwable != null) {
164+
try {
165+
doOnError(span, throwable, errorPriority);
166+
} catch (BlockingException e) {
167+
throw e;
168+
} catch (Throwable t) {
169+
log.debug("Failed to decorate span on error", t);
170+
}
132171
}
133172
}
134173

135-
public void onError(final ContextScope scope, final Throwable throwable) {
174+
public final void onError(
175+
@Nullable final ContextScope scope, @Nullable final Throwable throwable) {
136176
if (scope != null) {
137177
onError(AgentSpan.fromContext(scope.context()), throwable);
138178
}
139179
}
140180

141-
public void onPeerConnection(final AgentSpan span, final InetSocketAddress remoteConnection) {
181+
protected void doOnError(final AgentSpan span, final Throwable throwable, byte errorPriority) {
182+
span.addThrowable(
183+
throwable instanceof ExecutionException ? throwable.getCause() : throwable, errorPriority);
184+
}
185+
186+
public final void onPeerConnection(
187+
final AgentSpan span, @Nullable final InetSocketAddress remoteConnection) {
142188
if (remoteConnection != null) {
143189
onPeerConnection(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved());
144190
setPeerPort(span, remoteConnection.getPort());
145191
}
146192
}
147193

148-
public void onPeerConnection(final AgentSpan span, final InetAddress remoteAddress) {
194+
public final void onPeerConnection(
195+
final AgentSpan span, @Nullable final InetAddress remoteAddress) {
149196
onPeerConnection(span, remoteAddress, true);
150197
}
151198

152-
public void onPeerConnection(AgentSpan span, InetAddress remoteAddress, boolean resolved) {
199+
public final void onPeerConnection(
200+
AgentSpan span, @Nullable InetAddress remoteAddress, boolean resolved) {
153201
if (remoteAddress != null) {
154202
String ip = remoteAddress.getHostAddress();
155203
if (resolved && Config.get().isPeerHostNameEnabled()) {
@@ -176,9 +224,6 @@ public void setPeerPort(AgentSpan span, int port) {
176224
/**
177225
* This method is used to generate an acceptable span (operation) name based on a given method
178226
* reference. Anonymous classes are named based on their parent.
179-
*
180-
* @param method
181-
* @return
182227
*/
183228
public CharSequence spanNameForMethod(final Method method) {
184229
return spanNameForMethod(method.getDeclaringClass(), method);
@@ -191,7 +236,7 @@ public CharSequence spanNameForMethod(final Method method) {
191236
* @param method the method to get the name from, nullable
192237
* @return the span name from the class and method
193238
*/
194-
public CharSequence spanNameForMethod(final Class<?> clazz, final Method method) {
239+
public CharSequence spanNameForMethod(final Class<?> clazz, @Nullable final Method method) {
195240
if (null == method) {
196241
return CLASS_NAMES.getClassName(clazz);
197242
}
@@ -205,16 +250,13 @@ public CharSequence spanNameForMethod(final Class<?> clazz, final Method method)
205250
* @param methodName the name of the method to get the name from, nullable
206251
* @return the span name from the class and method
207252
*/
208-
public CharSequence spanNameForMethod(final Class<?> clazz, final String methodName) {
253+
public CharSequence spanNameForMethod(final Class<?> clazz, @Nullable final String methodName) {
209254
return CLASS_NAMES.getQualifiedName(clazz, methodName);
210255
}
211256

212257
/**
213258
* This method is used to generate an acceptable span (operation) name based on a given class
214259
* reference. Anonymous classes are named based on their parent.
215-
*
216-
* @param clazz
217-
* @return
218260
*/
219261
public CharSequence className(final Class<?> clazz) {
220262
String simpleName = clazz.getSimpleName();

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import datadog.trace.api.TagMap;
44
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
55
import datadog.trace.bootstrap.instrumentation.api.Tags;
6+
import javax.annotation.ParametersAreNonnullByDefault;
67

8+
@ParametersAreNonnullByDefault
79
public abstract class ClientDecorator extends BaseDecorator {
810
// Deliberately not volatile, reading a stale null and creating an extra Entry is safe
911
private TagMap.Entry cachedSpanKindEntry = null;
@@ -32,7 +34,7 @@ protected String spanKind() {
3234
}
3335

3436
@Override
35-
public void afterStart(final AgentSpan span) {
37+
protected void doAfterStart(final AgentSpan span) {
3638
final String service = service();
3739
if (service != null) {
3840
span.setServiceName(service, component());
@@ -41,6 +43,6 @@ public void afterStart(final AgentSpan span) {
4143

4244
// Generate metrics for all client spans.
4345
span.setMeasured(true);
44-
super.afterStart(span);
46+
super.doAfterStart(span);
4547
}
4648
}

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/DBTypeProcessingDatabaseClientDecorator.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package datadog.trace.bootstrap.instrumentation.decorator;
22

33
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
4+
import javax.annotation.ParametersAreNonnullByDefault;
45

6+
@ParametersAreNonnullByDefault
57
public abstract class DBTypeProcessingDatabaseClientDecorator<CONNECTION>
68
extends DatabaseClientDecorator<CONNECTION> {
79

810
@Override
9-
public void afterStart(AgentSpan span) {
11+
protected void doAfterStart(AgentSpan span) {
1012
processDatabaseType(span, dbType());
11-
super.afterStart(span);
13+
super.doAfterStart(span);
1214
}
1315
}

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,17 @@ protected boolean shouldSetResourceName() {
9292
}
9393
};
9494

95-
public void onRequest(final AgentSpan span, final REQUEST request) {
95+
public final void onRequest(final AgentSpan span, final REQUEST request) {
96+
try {
97+
doOnRequest(span, request);
98+
} catch (BlockingException e) {
99+
throw e;
100+
} catch (Throwable t) {
101+
log.debug("Failed to decorate span on request", t);
102+
}
103+
}
104+
105+
protected void doOnRequest(final AgentSpan span, final REQUEST request) {
96106
if (request != null) {
97107
AgentTracer.get()
98108
.getDataStreamsMonitoring()
@@ -145,7 +155,17 @@ public void onRequest(final AgentSpan span, final REQUEST request) {
145155
}
146156
}
147157

148-
public void onResponse(final AgentSpan span, final RESPONSE response) {
158+
public final void onResponse(final AgentSpan span, final RESPONSE response) {
159+
try {
160+
doOnResponse(span, response);
161+
} catch (BlockingException e) {
162+
throw e;
163+
} catch (Throwable t) {
164+
log.debug("Failed to decorate span on response", t);
165+
}
166+
}
167+
168+
protected void doOnResponse(final AgentSpan span, final RESPONSE response) {
149169
if (response != null) {
150170
final int status = status(response);
151171
if (status > UNSET_STATUS) {

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static datadog.trace.api.datastreams.DataStreamsContext.forHttpServer;
66
import static datadog.trace.api.gateway.Events.EVENTS;
77
import static datadog.trace.bootstrap.ActiveSubsystems.APPSEC_ACTIVE;
8+
import static datadog.trace.bootstrap.instrumentation.api.AgentSpan.fromContext;
89
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.traceConfig;
910
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
1011

@@ -237,7 +238,21 @@ private void resetServiceNameIfUnderInferredProxy(Context parentContext, AgentSp
237238
}
238239
};
239240

240-
public void onRequest(
241+
public final void onRequest(
242+
final AgentSpan span,
243+
final CONNECTION connection,
244+
final REQUEST request,
245+
final Context parentContext) {
246+
try {
247+
doOnRequest(span, connection, request, parentContext);
248+
} catch (BlockingException e) {
249+
throw e;
250+
} catch (Throwable t) {
251+
log.debug("Failed to decorate span on request", t);
252+
}
253+
}
254+
255+
protected void doOnRequest(
241256
final AgentSpan span,
242257
final CONNECTION connection,
243258
final REQUEST request,
@@ -431,7 +446,7 @@ public void onRequest(
431446
}
432447

433448
protected static AgentSpanContext.Extracted getExtractedSpanContext(Context parentContext) {
434-
AgentSpan extractedSpan = AgentSpan.fromContext(parentContext);
449+
AgentSpan extractedSpan = fromContext(parentContext);
435450
if (extractedSpan != null) {
436451
AgentSpanContext extractedSpanContext = extractedSpan.spanContext();
437452
if (extractedSpanContext instanceof AgentSpanContext.Extracted) {
@@ -448,7 +463,17 @@ protected BlockResponseFunction createBlockResponseFunction(
448463
return null;
449464
}
450465

451-
public void onResponseStatus(final AgentSpan span, final int status) {
466+
public final void onResponseStatus(final AgentSpan span, final int status) {
467+
try {
468+
doOnResponseStatus(span, status);
469+
} catch (BlockingException e) {
470+
throw e;
471+
} catch (Throwable t) {
472+
log.debug("Failed to decorate span on response status", t);
473+
}
474+
}
475+
476+
protected void doOnResponseStatus(final AgentSpan span, final int status) {
452477
if (status > UNSET_STATUS) {
453478
span.setHttpStatusCode(status);
454479
// explicitly set here because some other decorators might already set an error without
@@ -481,10 +506,20 @@ protected boolean isAppSecOnResponseSeparate() {
481506
return false;
482507
}
483508

484-
public void onResponse(final AgentSpan span, final RESPONSE response) {
509+
public final void onResponse(final AgentSpan span, final RESPONSE response) {
510+
try {
511+
doOnResponse(span, response);
512+
} catch (BlockingException e) {
513+
throw e;
514+
} catch (Throwable t) {
515+
log.debug("Failed to decorate span on response", t);
516+
}
517+
}
518+
519+
protected void doOnResponse(final AgentSpan span, final RESPONSE response) {
485520
if (response != null) {
486521
final int status = status(response);
487-
onResponseStatus(span, status);
522+
doOnResponseStatus(span, status);
488523

489524
AgentPropagation.ContextVisitor<RESPONSE> getter = responseGetter();
490525
if (getter != null) {
@@ -533,7 +568,8 @@ private AgentSpanContext callIGCallbackStart(@Nullable final AgentSpanContext ex
533568
}
534569

535570
@Override
536-
public void onError(final AgentSpan span, final Throwable throwable) {
571+
protected void doOnError(
572+
@Nonnull final AgentSpan span, @Nonnull final Throwable throwable, byte errorPriority) {
537573
if (throwable != null) {
538574
span.addThrowable(
539575
throwable instanceof ExecutionException ? throwable.getCause() : throwable,
@@ -635,16 +671,16 @@ private Flow<Void> callIGCallbackURI(
635671
}
636672

637673
@Override
638-
public void beforeFinish(Context context) {
639-
AgentSpan span = AgentSpan.fromContext(context);
674+
protected void doBeforeFinish(@Nonnull Context context) {
675+
AgentSpan span = fromContext(context);
640676
if (span != null) {
641677
onRequestEndForInstrumentationGateway(span);
642678
}
643679

644680
// Close Serverless Gateway Inferred Span if any
645681
finishInferredProxySpan(context);
646682

647-
super.beforeFinish(context);
683+
super.doBeforeFinish(context);
648684
}
649685

650686
protected void finishInferredProxySpan(Context context) {

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/MessagingClientDecorator.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import datadog.trace.api.Config;
44
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
5+
import javax.annotation.ParametersAreNonnullByDefault;
56

7+
@ParametersAreNonnullByDefault
68
public abstract class MessagingClientDecorator extends ClientDecorator {
79

810
protected final boolean endToEndDurationsEnabled;
@@ -20,10 +22,10 @@ protected boolean endToEndDurationsDefault() {
2022
}
2123

2224
@Override
23-
public void afterStart(final AgentSpan span) {
25+
protected void doAfterStart(final AgentSpan span) {
2426
if (endToEndDurationsEnabled) {
2527
span.beginEndToEnd();
2628
}
27-
super.afterStart(span);
29+
super.doAfterStart(span);
2830
}
2931
}

0 commit comments

Comments
 (0)