Null check return advices (#1601)

* @nullable return value on api-plugin * @nullable return value on dubbo-plugin * @nullable return value on grpc-plugin * @nullable return value on jdbc-plugin * @nullable return value on httpclient-plugin * @nullable return value on {kafka,jms}-plugin * @nullable return value on process-plugin + fix NPE * @nullable return value on servlet-plugin + fix NPE * @nullable rv & thrown live-template exit advice * update changelog

Author: SylvainJuge

CHANGELOG.asciidoc

@@ -43,6 +43,7 @@ endif::[]
 * Serialize all stack trace frames when setting `stack_trace_limit=-1` instead of none - {pull}1571[#1571]
 * Fix `UnsupportedOperationException` when calling `ServletContext.getClassLoader()` - {pull}1576[#1576]
 * Fix improper request body capturing - {pull}1579[#1579]
+* avoid `NullPointerException` due to null return values instrumentation advices - {pull}1601[#1601]
 
 [float]
 ===== Refactors

CONTRIBUTING.md

@@ -81,7 +81,7 @@ These live templates can be pasted in Preferences > Editor > Live Templates > ot
 
 **`enter`**
 ```xml
-<template name="enter" value="@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)&#10;public static void onEnter() {&#10; $END$&#10;}" description="Adds @OnMethodEnter advice" toReformat="false" toShortenFQNames="true">
+<template name="enter" value="@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)&#10;public static void onEnter() {&#10; $END$&#10;}" description="Adds @OnMethodEnter advice" toReformat="true" toShortenFQNames="true">
  <context>
  <option name="JAVA_DECLARATION" value="true" />
  </context>
@@ -91,17 +91,17 @@ These live templates can be pasted in Preferences > Editor > Live Templates > ot
 **`exit`**
 
 ```xml
-<template name="exit" value="@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)&#10;public static void onExit(@Advice.Thrown Throwable thrown) {&#10; $END$&#10;}" description="Adds @OnMethodExit advice" toReformat="false" toShortenFQNames="true">
- <context>
- <option name="JAVA_DECLARATION" value="true" />
- </context>
+<template name="exit" value="@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)&#10;public static void onExit(@Advice.Thrown @Nullable Throwable thrown, @Advice.Return @Nullable Object returnValue) {&#10; $END$&#10;}" description="Adds @OnMethodExit advice" toReformat="true" toShortenFQNames="true">
+  <context>
+  <option name="JAVA_DECLARATION" value="true" />
+  </context>
 </template>
 ```
 
 
 **`logger`**
 ```xml
-<template name="logger" value="private static final Logger logger = LoggerFactory.getLogger($CLASS_NAME$.class);" description="" toReformat="false" toShortenFQNames="true">
+<template name="logger" value="private static final Logger logger = LoggerFactory.getLogger($CLASS_NAME$.class);" description="" toReformat="true" toShortenFQNames="true">
  <variable name="CLASS_NAME" expression="className()" defaultValue="" alwaysStopAt="true" />
  <context>
  <option name="JAVA_DECLARATION" value="true" />
@@ -121,7 +121,7 @@ These live templates can be pasted in Preferences > Editor > Live Templates > ot
 
 **`at`**
 ```xml
-<template name="at" value="assertThat($EXPR$)$END$;" description="assertJ assert expression" toReformat="false" toShortenFQNames="true" useStaticImport="true">
+<template name="at" value="assertThat($EXPR$)$END$;" description="assertJ assert expression" toReformat="true" toShortenFQNames="true" useStaticImport="true">
  <variable name="EXPR" expression="" defaultValue="" alwaysStopAt="true" />
  <context>
  <option name="JAVA_STATEMENT" value="true" />

apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/AbstractSpanInstrumentation.java

@@ -183,7 +183,7 @@ public CaptureExceptionInstrumentation() {
  @Advice.OnMethodExit(suppress = Throwable.class, inline = false)
  public static String captureException(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object context,
  @Advice.Argument(0) Throwable t,
- @Advice.Return String returnValue) {
+ @Advice.Return @Nullable String returnValue) {
  if (context instanceof AbstractSpan<?>) {
  return ((AbstractSpan<?>) context).captureExceptionAndGetErrorId(t);
  } else {
@@ -217,10 +217,11 @@ public GetIdInstrumentation() {
  super(named("getId").and(takesArguments(0)));
  }
 
+ @Nullable
  @AssignTo.Return
  @Advice.OnMethodExit(suppress = Throwable.class, inline = false)
  public static String getId(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object context,
- @Advice.Return String returnValue) {
+ @Advice.Return @Nullable String returnValue) {
  if (context instanceof AbstractSpan<?>) {
  return ((AbstractSpan<?>) context).getTraceContext().getId().toString();
  } else {
@@ -234,10 +235,11 @@ public GetTraceIdInstrumentation() {
  super(named("getTraceId").and(takesArguments(0)));
  }
 
+ @Nullable
  @AssignTo.Return
  @Advice.OnMethodExit(suppress = Throwable.class, inline = false)
  public static String getTraceId(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object context,
- @Advice.Return String returnValue) {
+ @Advice.Return @Nullable String returnValue) {
  if (context instanceof AbstractSpan<?>) {
  return ((AbstractSpan<?>) context).getTraceContext().getTraceId().toString();
  } else {
@@ -253,7 +255,8 @@ public AddStringLabelInstrumentation() {
 
  @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
  public static void addLabel(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object context,
- @Advice.Argument(0) String key, @Nullable @Advice.Argument(1) String value) {
+ @Advice.Argument(0) String key,
+ @Advice.Argument(1) @Nullable String value) {
  if (value != null && context instanceof AbstractSpan) {
  ((AbstractSpan<?>) context).addLabel(key, value);
  }
@@ -267,7 +270,8 @@ public AddNumberLabelInstrumentation() {
 
  @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
  public static void addLabel(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object context,
- @Advice.Argument(0) String key, @Nullable @Advice.Argument(1) Number value) {
+ @Advice.Argument(0) String key,
+ @Advice.Argument(1) @Nullable Number value) {
  if (value != null && context instanceof AbstractSpan) {
  ((AbstractSpan<?>) context).addLabel(key, value);
  }
@@ -281,7 +285,8 @@ public AddBooleanLabelInstrumentation() {
 
  @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
  public static void addLabel(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object context,
- @Advice.Argument(0) String key, @Nullable @Advice.Argument(1) Boolean value) {
+ @Advice.Argument(0) String key,
+ @Advice.Argument(1) @Nullable Boolean value) {
  if (value != null && context instanceof AbstractSpan) {
  ((AbstractSpan<?>) context).addLabel(key, value);
  }

apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/LegacySpanInstrumentation.java

@@ -151,10 +151,11 @@ public GetIdInstrumentation() {
  super(named("getId").and(takesArguments(0)));
  }
 
+ @Nullable
  @AssignTo.Return
  @Advice.OnMethodExit(inline = false)
  public static String getId(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object span,
- @Advice.Return String returnValue) {
+ @Advice.Return @Nullable String returnValue) {
  if (span instanceof Span) {
  return ((Span) span).getTraceContext().getId().toString();
  } else {
@@ -168,10 +169,11 @@ public GetTraceIdInstrumentation() {
  super(named("getTraceId").and(takesArguments(0)));
  }
 
+ @Nullable
  @AssignTo.Return
  @Advice.OnMethodExit(inline = false)
  public static String getTraceId(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object span,
- @Advice.Return String returnValue) {
+ @Advice.Return @Nullable String returnValue) {
  if (span instanceof Span) {
  return ((Span) span).getTraceContext().getTraceId().toString();
  } else {

apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentation.java

@@ -78,10 +78,11 @@ public EnsureParentIdInstrumentation() {
  super(named("ensureParentId"));
  }
 
+ @Nullable
  @AssignTo.Return
  @Advice.OnMethodExit(suppress = Throwable.class, inline = false)
  public static String ensureParentId(@Advice.FieldValue(value = "span", typing = Assigner.Typing.DYNAMIC) Object transaction,
- @Advice.Return String returnValue) {
+ @Advice.Return @Nullable String returnValue) {
  if (transaction instanceof Transaction) {
  final TraceContext traceContext = ((Transaction) transaction).getTraceContext();
  Id parentId = traceContext.getParentId();

apm-agent-plugins/apm-dubbo-plugin/src/main/java/co/elastic/apm/agent/dubbo/advice/AlibabaMonitorFilterAdvice.java

@@ -87,17 +87,21 @@ public static void onEnterFilterInvoke(@Advice.Argument(1) Invocation invocation
 
  @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
  public static void onExitFilterInvoke(@Advice.Argument(1) Invocation invocation,
- @Advice.Return Result result,
- @Nullable @Advice.Local("span") Span span,
- @Nullable @Advice.Thrown Throwable t,
- @Nullable @Advice.Local("transaction") Transaction transaction) {
+ @Advice.Return @Nullable Result result,
+ @Advice.Local("span") @Nullable Span span,
+ @Advice.Thrown @Nullable Throwable t,
+ @Advice.Local("transaction") @Nullable Transaction transaction) {
  AbstractSpan<?> actualSpan = span != null ? span : transaction;
  if (actualSpan == null) {
  return;
  }
- actualSpan.captureException(t)
- .captureException(result.getException())
- .deactivate();
+ actualSpan.captureException(t);
+
+ if (result != null) { // will be null in case of thrown exception
+ actualSpan.captureException(result.getException());
+ }
+ actualSpan.deactivate();
+
  if (!(RpcContext.getContext().getFuture() instanceof FutureAdapter)) {
  actualSpan.end();
  }

apm-agent-plugins/apm-dubbo-plugin/src/main/java/co/elastic/apm/agent/dubbo/advice/ApacheMonitorFilterAdvice.java

@@ -92,10 +92,10 @@ public static void onEnterFilterInvoke(@Advice.Argument(1) Invocation invocation
 
  @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
  public static void onExitFilterInvoke(@Advice.Argument(1) Invocation invocation,
- @Advice.Return Result result,
- @Nullable @Advice.Local("span") final Span span,
- @Advice.Thrown Throwable t,
- @Nullable @Advice.Local("transaction") Transaction transaction) {
+ @Advice.Return @Nullable Result result,
+ @Advice.Local("span") @Nullable final Span span,
+ @Advice.Thrown @Nullable Throwable t,
+ @Advice.Local("transaction") @Nullable Transaction transaction) {
 
  RpcContext context = RpcContext.getContext();
  AbstractSpan<?> actualSpan = span != null ? span : transaction;

apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/ServerCallHandlerInstrumentation.java

@@ -65,9 +65,9 @@ public static Object onEnter(@Advice.Origin Class<?> clazz,
 
  @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
  public static void onExit(@Advice.Thrown @Nullable Throwable thrown,
-  @Advice.Argument(0) ServerCall<?, ?> serverCall,
-  @Advice.Return ServerCall.Listener<?> listener,
-  @Advice.Enter @Nullable Object enterTransaction) {
+ @Advice.Argument(0) ServerCall<?, ?> serverCall,
+ @Advice.Return @Nullable ServerCall.Listener<?> listener,
+ @Advice.Enter @Nullable Object enterTransaction) {
 
  if (!(enterTransaction instanceof Transaction)) {
  return;
@@ -79,7 +79,11 @@ public static void onExit(@Advice.Thrown @Nullable Throwable thrown,
  return;
  }
 
- helper.registerTransaction(serverCall, listener, transaction);
+ if (listener != null) {
+ helper.registerTransaction(serverCall, listener, transaction);
+ }
+
+
  }
 
 }

apm-agent-plugins/apm-grpc/apm-grpc-test-1.6.1/src/test/java/co/elastic/apm/agent/grpc/v1_6_1/testapp/generated/HelloGrpc.java

@@ -2,7 +2,7 @@
  * #%L
  * Elastic APM Java agent
  * %%
- * Copyright (C) 2018 - 2020 Elastic and contributors
+ * Copyright (C) 2018 - 2021 Elastic and contributors
  * %%
  * Licensed to Elasticsearch B.V. under one or more contributor
  * license agreements. See the NOTICE file distributed with

apm-agent-plugins/apm-grpc/apm-grpc-test-1.6.1/src/test/java/co/elastic/apm/agent/grpc/v1_6_1/testapp/generated/HelloReply.java

@@ -2,7 +2,7 @@
  * #%L
  * Elastic APM Java agent
  * %%
- * Copyright (C) 2018 - 2020 Elastic and contributors
+ * Copyright (C) 2018 - 2021 Elastic and contributors
  * %%
  * Licensed to Elasticsearch B.V. under one or more contributor
  * license agreements. See the NOTICE file distributed with