fix(agent): set oapi callbacks the right way (#700)

When wrapping a user function do not add special instrumentation if ANY
special instrumentation (before/after/cleanup) has already been set.
This approach matches pre-oapi instrumentation.

Author: ZNeumann

agent/php_wrapper.c

@@ -23,45 +23,42 @@ nruserfn_t* nr_php_wrap_user_function_before_after_clean_with_transience(
     return wraprec;
   }
 
-  if (after_callback) {
-    if (is_instrumentation_set(wraprec->special_instrumentation,
-                               after_callback)) {
-      nrl_verbosedebug(
-          NRL_INSTRUMENT,
-          "%s: attempting to set special_instrumentation for %.*s, but "
-          "it is already set",
-          __func__, NRSAFELEN(namelen), NRBLANKSTR(name));
-    } else {
-      wraprec->special_instrumentation = after_callback;
-    }
+  /* If any of the callbacks we are attempting to set are already set to
+   * something else, we want to exit without setting new callbacks */
+  if (is_instrumentation_set_and_not_equal(wraprec->special_instrumentation,
+                                           after_callback)) {
+    nrl_verbosedebug(
+        NRL_INSTRUMENT,
+        "%s: attempting to set special_instrumentation for %.*s, but "
+        "it is already set",
+        __func__, NRSAFELEN(namelen), NRBLANKSTR(name));
+    return wraprec;
   }
 
-  if (before_callback) {
-    if (is_instrumentation_set(wraprec->special_instrumentation_before,
-                               before_callback)) {
-      nrl_verbosedebug(NRL_INSTRUMENT,
-                       "%s: attempting to set special_instrumentation_before "
-                       "for %.*s, but "
-                       "it is already set",
-                       __func__, NRSAFELEN(namelen), NRBLANKSTR(name));
-    } else {
-      wraprec->special_instrumentation_before = before_callback;
-    }
+  if (is_instrumentation_set_and_not_equal(wraprec->special_instrumentation_before,
+                                           before_callback)) {
+    nrl_verbosedebug(NRL_INSTRUMENT,
+                     "%s: attempting to set special_instrumentation_before "
+                     "for %.*s, but "
+                     "it is already set",
+                     __func__, NRSAFELEN(namelen), NRBLANKSTR(name));
+    return wraprec;
   }
 
-  if (clean_callback) {
-    if (is_instrumentation_set(wraprec->special_instrumentation_clean,
-                               clean_callback)) {
-      nrl_verbosedebug(NRL_INSTRUMENT,
-                       "%s: attempting to set special_instrumentation_clean "
-                       "for %.*s, but "
-                       "it is already set",
-                       __func__, NRSAFELEN(namelen), NRBLANKSTR(name));
-    } else {
-      wraprec->special_instrumentation_clean = clean_callback;
-    }
+  if (is_instrumentation_set_and_not_equal(wraprec->special_instrumentation_clean,
+                                           clean_callback)) {
+    nrl_verbosedebug(NRL_INSTRUMENT,
+                     "%s: attempting to set special_instrumentation_clean "
+                     "for %.*s, but "
+                     "it is already set",
+                     __func__, NRSAFELEN(namelen), NRBLANKSTR(name));
+    return wraprec;
   }
 
+  wraprec->special_instrumentation = after_callback;
+  wraprec->special_instrumentation_before = before_callback;
+  wraprec->special_instrumentation_clean = clean_callback;
+
   return wraprec;
 }
 #endif

agent/php_wrapper.h

@@ -327,8 +327,9 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D);
     was_executed = 1;                                                \
   }
 
-static inline bool is_instrumentation_set(nrspecialfn_t instrumentation,
-                                          nrspecialfn_t callback) {
+static inline bool is_instrumentation_set_and_not_equal(
+    nrspecialfn_t instrumentation,
+    nrspecialfn_t callback) {
   if ((NULL != instrumentation) && (callback != instrumentation)) {
     return true;
   }