Skip to content

Commit f2121a0

Browse files
mccullsdevflow.devflow-routing-intake
mcculls
and
devflow.devflow-routing-intake
authored
Simplify decorator API: void return types and factory method naming (#11929)
Simplify decorator methods by removing unused return type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Rename on* factory methods in decorators to start* convention Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Address PR review comments Merge remote-tracking branch 'origin/master' into mcculls/simplify-decorator-signatures Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 5532f64 commit f2121a0

97 files changed

Lines changed: 294 additions & 422 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/jmh/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorBenchmark.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.openjdk.jmh.annotations.Setup;
3333
import org.openjdk.jmh.annotations.State;
3434
import org.openjdk.jmh.annotations.Warmup;
35+
import org.openjdk.jmh.infra.Blackhole;
3536

3637
@State(Scope.Benchmark)
3738
@Warmup(iterations = 4, time = 30, timeUnit = SECONDS)
@@ -64,8 +65,9 @@ public void setUp() {
6465
}
6566

6667
@Benchmark
67-
public AgentSpan onRequest() {
68-
return decorator.onRequest(span, null, request, root());
68+
public void onRequest(Blackhole bh) {
69+
decorator.onRequest(span, null, request, root());
70+
bh.consume(span);
6971
}
7072

7173
public static class Request {

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

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ protected boolean traceAnalyticsDefault() {
9090
return false;
9191
}
9292

93-
public AgentSpan afterStart(final AgentSpan span) {
93+
public void afterStart(final AgentSpan span) {
9494
if (spanType() != null) {
9595
span.setSpanType(spanType());
9696
}
@@ -104,64 +104,52 @@ public AgentSpan afterStart(final AgentSpan span) {
104104

105105
// null handled by setMetric
106106
span.setMetric(traceAnalyticsEntry);
107-
108-
return span;
109107
}
110108

111-
public ContextScope beforeFinish(final ContextScope scope) {
109+
public void beforeFinish(final ContextScope scope) {
112110
beforeFinish(scope.context());
113-
return scope;
114111
}
115112

116-
public AgentSpan beforeFinish(final AgentSpan span) {
117-
return span;
118-
}
113+
public void beforeFinish(final AgentSpan span) {}
119114

120-
public Context beforeFinish(final Context context) {
121-
return context;
122-
}
115+
public void beforeFinish(final Context context) {}
123116

124-
public AgentScope onError(final AgentScope scope, final Throwable throwable) {
117+
public void onError(final AgentScope scope, final Throwable throwable) {
125118
if (scope != null) {
126119
onError(scope.span(), throwable);
127120
}
128-
return scope;
129121
}
130122

131-
public AgentSpan onError(final AgentSpan span, final Throwable throwable) {
132-
return onError(span, throwable, ErrorPriorities.DEFAULT);
123+
public void onError(final AgentSpan span, final Throwable throwable) {
124+
onError(span, throwable, ErrorPriorities.DEFAULT);
133125
}
134126

135-
public AgentSpan onError(final AgentSpan span, final Throwable throwable, byte errorPriority) {
127+
public void onError(final AgentSpan span, final Throwable throwable, byte errorPriority) {
136128
if (throwable != null && span != null) {
137129
span.addThrowable(
138130
throwable instanceof ExecutionException ? throwable.getCause() : throwable,
139131
errorPriority);
140132
}
141-
return span;
142133
}
143134

144-
public ContextScope onError(final ContextScope scope, final Throwable throwable) {
135+
public void onError(final ContextScope scope, final Throwable throwable) {
145136
if (scope != null) {
146137
onError(AgentSpan.fromContext(scope.context()), throwable);
147138
}
148-
return scope;
149139
}
150140

151-
public AgentSpan onPeerConnection(
152-
final AgentSpan span, final InetSocketAddress remoteConnection) {
141+
public void onPeerConnection(final AgentSpan span, final InetSocketAddress remoteConnection) {
153142
if (remoteConnection != null) {
154143
onPeerConnection(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved());
155144
setPeerPort(span, remoteConnection.getPort());
156145
}
157-
return span;
158146
}
159147

160-
public AgentSpan onPeerConnection(final AgentSpan span, final InetAddress remoteAddress) {
161-
return onPeerConnection(span, remoteAddress, true);
148+
public void onPeerConnection(final AgentSpan span, final InetAddress remoteAddress) {
149+
onPeerConnection(span, remoteAddress, true);
162150
}
163151

164-
public AgentSpan onPeerConnection(AgentSpan span, InetAddress remoteAddress, boolean resolved) {
152+
public void onPeerConnection(AgentSpan span, InetAddress remoteAddress, boolean resolved) {
165153
if (remoteAddress != null) {
166154
String ip = remoteAddress.getHostAddress();
167155
if (resolved && Config.get().isPeerHostNameEnabled()) {
@@ -173,20 +161,16 @@ public AgentSpan onPeerConnection(AgentSpan span, InetAddress remoteAddress, boo
173161
span.setTag(Tags.PEER_HOST_IPV6, ip);
174162
}
175163
}
176-
return span;
177164
}
178165

179-
public AgentSpan setPeerPort(AgentSpan span, String port) {
166+
public void setPeerPort(AgentSpan span, String port) {
180167
span.setTag(Tags.PEER_PORT, port);
181-
182-
return span;
183168
}
184169

185-
public AgentSpan setPeerPort(AgentSpan span, int port) {
170+
public void setPeerPort(AgentSpan span, int port) {
186171
if (port > UNSET_PORT) {
187172
span.setTag(Tags.PEER_PORT, port);
188173
}
189-
return span;
190174
}
191175

192176
/**

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ protected String spanKind() {
3232
}
3333

3434
@Override
35-
public AgentSpan afterStart(final AgentSpan span) {
35+
public void afterStart(final AgentSpan span) {
3636
final String service = service();
3737
if (service != null) {
3838
span.setServiceName(service, component());
@@ -41,6 +41,6 @@ public AgentSpan afterStart(final AgentSpan span) {
4141

4242
// Generate metrics for all client spans.
4343
span.setMeasured(true);
44-
return super.afterStart(span);
44+
super.afterStart(span);
4545
}
4646
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ public abstract class DBTypeProcessingDatabaseClientDecorator<CONNECTION>
66
extends DatabaseClientDecorator<CONNECTION> {
77

88
@Override
9-
public AgentSpan afterStart(AgentSpan span) {
9+
public void afterStart(AgentSpan span) {
1010
processDatabaseType(span, dbType());
11-
return super.afterStart(span);
11+
super.afterStart(span);
1212
}
1313
}

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,8 @@ public String getDbType() {
6666
*
6767
* @param span
6868
* @param connection
69-
* @return
7069
*/
71-
public AgentSpan onConnection(final AgentSpan span, final CONNECTION connection) {
70+
public void onConnection(final AgentSpan span, final CONNECTION connection) {
7271
if (connection != null) {
7372
span.setTag(Tags.DB_USER, dbUser(connection));
7473
onInstance(span, dbInstance(connection));
@@ -81,18 +80,16 @@ public AgentSpan onConnection(final AgentSpan span, final CONNECTION connection)
8180
}
8281
}
8382
}
84-
return span;
8583
}
8684

87-
protected AgentSpan onInstance(final AgentSpan span, final String dbInstance) {
85+
protected void onInstance(final AgentSpan span, final String dbInstance) {
8886
if (dbInstance != null) {
8987
span.setTag(Tags.DB_INSTANCE, dbInstance);
9088
String serviceName = dbClientService(dbInstance);
9189
if (null != serviceName) {
9290
span.setServiceName(serviceName, component());
9391
}
9492
}
95-
return span;
9693
}
9794

9895
public String dbService(final String dbType, final String instanceName) {
@@ -114,9 +111,8 @@ public String dbClientService(final String instanceName) {
114111
return service;
115112
}
116113

117-
public AgentSpan onStatement(final AgentSpan span, final CharSequence statement) {
114+
public void onStatement(final AgentSpan span, final CharSequence statement) {
118115
span.setResourceName(statement);
119-
return span;
120116
}
121117

122118
/**

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

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

95-
public AgentSpan onRequest(final AgentSpan span, final REQUEST request) {
95+
public void onRequest(final AgentSpan span, final REQUEST request) {
9696
if (request != null) {
9797
AgentTracer.get()
9898
.getDataStreamsMonitoring()
@@ -143,10 +143,9 @@ public AgentSpan onRequest(final AgentSpan span, final REQUEST request) {
143143
ssrfIastCheck(request);
144144
}
145145
}
146-
return span;
147146
}
148147

149-
public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
148+
public void onResponse(final AgentSpan span, final RESPONSE response) {
150149
if (response != null) {
151150
final int status = status(response);
152151
if (status > UNSET_STATUS) {
@@ -166,7 +165,6 @@ public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
166165
}
167166
}
168167
}
169-
return span;
170168
}
171169

172170
public String operationName() {

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ private void resetServiceNameIfUnderInferredProxy(Context parentContext, AgentSp
237237
}
238238
};
239239

240-
public AgentSpan onRequest(
240+
public void onRequest(
241241
final AgentSpan span,
242242
final CONNECTION connection,
243243
final REQUEST request,
@@ -428,7 +428,6 @@ public AgentSpan onRequest(
428428
DataStreamsTransactionExtractor.Type.HTTP_IN_HEADERS,
429429
request,
430430
DSM_TRANSACTION_SOURCE_READER);
431-
return span;
432431
}
433432

434433
protected static AgentSpanContext.Extracted getExtractedSpanContext(Context parentContext) {
@@ -449,7 +448,7 @@ protected BlockResponseFunction createBlockResponseFunction(
449448
return null;
450449
}
451450

452-
public AgentSpan onResponseStatus(final AgentSpan span, final int status) {
451+
public void onResponseStatus(final AgentSpan span, final int status) {
453452
if (status > UNSET_STATUS) {
454453
span.setHttpStatusCode(status);
455454
// explicitly set here because some other decorators might already set an error without
@@ -465,7 +464,6 @@ public AgentSpan onResponseStatus(final AgentSpan span, final int status) {
465464
if (SHOULD_SET_404_RESOURCE_NAME && status == 404) {
466465
span.setResourceName(NOT_FOUND_RESOURCE_NAME, ResourceNamePriorities.HTTP_404);
467466
}
468-
return span;
469467
}
470468

471469
/**
@@ -483,7 +481,7 @@ protected boolean isAppSecOnResponseSeparate() {
483481
return false;
484482
}
485483

486-
public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
484+
public void onResponse(final AgentSpan span, final RESPONSE response) {
487485
if (response != null) {
488486
final int status = status(response);
489487
onResponseStatus(span, status);
@@ -501,7 +499,6 @@ public AgentSpan onResponse(final AgentSpan span, final RESPONSE response) {
501499
callIGCallbackResponseAndHeaders(span, response, status);
502500
}
503501
}
504-
return span;
505502
}
506503

507504
private AgentSpanContext callIGCallbackStart(@Nullable final AgentSpanContext extracted) {
@@ -536,13 +533,12 @@ private AgentSpanContext callIGCallbackStart(@Nullable final AgentSpanContext ex
536533
}
537534

538535
@Override
539-
public AgentSpan onError(final AgentSpan span, final Throwable throwable) {
536+
public void onError(final AgentSpan span, final Throwable throwable) {
540537
if (throwable != null) {
541538
span.addThrowable(
542539
throwable instanceof ExecutionException ? throwable.getCause() : throwable,
543540
ErrorPriorities.HTTP_SERVER_DECORATOR);
544541
}
545-
return span;
546542
}
547543

548544
private Flow<Void> callIGCallbackRequestHeaders(AgentSpan span, REQUEST_CARRIER carrier) {
@@ -639,7 +635,7 @@ private Flow<Void> callIGCallbackURI(
639635
}
640636

641637
@Override
642-
public Context beforeFinish(Context context) {
638+
public void beforeFinish(Context context) {
643639
AgentSpan span = AgentSpan.fromContext(context);
644640
if (span != null) {
645641
onRequestEndForInstrumentationGateway(span);
@@ -648,7 +644,7 @@ public Context beforeFinish(Context context) {
648644
// Close Serverless Gateway Inferred Span if any
649645
finishInferredProxySpan(context);
650646

651-
return super.beforeFinish(context);
647+
super.beforeFinish(context);
652648
}
653649

654650
protected void finishInferredProxySpan(Context context) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ protected boolean endToEndDurationsDefault() {
2020
}
2121

2222
@Override
23-
public AgentSpan afterStart(final AgentSpan span) {
23+
public void afterStart(final AgentSpan span) {
2424
if (endToEndDurationsEnabled) {
2525
span.beginEndToEnd();
2626
}
27-
return super.afterStart(span);
27+
super.afterStart(span);
2828
}
2929
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,12 @@ public abstract class OrmClientDecorator extends DatabaseClientDecorator {
66

77
public abstract CharSequence entityName(final Object entity);
88

9-
public AgentSpan onOperation(final AgentSpan span, final Object entity) {
9+
public void onOperation(final AgentSpan span, final Object entity) {
1010
if (entity != null) {
1111
final CharSequence name = entityName(entity);
1212
if (name != null) {
1313
span.setResourceName(name);
1414
} // else we keep any existing resource.
1515
}
16-
return span;
1716
}
1817
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ public abstract class ServerDecorator extends BaseDecorator {
1212
TagMap.Entry.create(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE);
1313

1414
@Override
15-
public AgentSpan afterStart(final AgentSpan span) {
15+
public void afterStart(final AgentSpan span) {
1616
span.setTag(SPAN_KIND_ENTRY);
1717
span.setTag(LANG_ENTRY);
1818

19-
return super.afterStart(span);
19+
super.afterStart(span);
2020
}
2121
}

0 commit comments

Comments
 (0)