Allow user to supply a callback function (#8)

* Failing tests for providing a callable as a callback

* Use call_user_function_ex to execute the callback function.

Collect and pass the same arguments to the callback function (+1 if
there is a current $this object).

* Add tests for specifying callback as a string

* Handle all callables

* Update documentation on being able to supply any callback.

* Segfault on wrong return for executing a callable

* Handle closure and callables the same way

* Fix test name

Author: chingor13

ext/README.md

@@ -96,7 +96,7 @@ opencensus_trace_method('Foobar', '__construct');
 $foobar = new Foobar();
 ```
 
-The `$handler` parameter can be either an array or a closure callback.
+The `$handler` parameter can be either an array or a callable.
 
 If an array is provided, it should be an associative array with the following optional keys:
 
@@ -105,8 +105,8 @@ If an array is provided, it should be an associative array with the following op
 * `endTime` - float - the end time of the span. **Defaults to** the time that the method invocation completed.
 * `labels` - array - an associative array of string => string tags for this span.
 
-If a closure is provided, it will be passed the instance of the class (scope) and a copy of each parameter
-provided to the watched method. The closure should return an array with the above options. If the closure does
+If a callback is provided, it will be passed the instance of the class (scope) and a copy of each parameter
+provided to the watched method. The callback should return an array with the above options. If the callback does
 not return an array, an `E_USER_WARNING` error is thrown.
 
 ```php
@@ -126,6 +126,17 @@ opencensus_trace_method('Foobar', '__construct', function ($scope, $constructArg
         ]
     ];
 });
+
+// Example: supply a callback
+function my_callback($scope, $constructArg1, $constructArg2)
+{
+    return [
+        'labels' => [
+            'arg1' => $constructArg1
+        ]
+    ];
+}
+opencensus_trace_method('Foobar', '__construct', 'my_callback');
 ```
 
 To trace a function, use the `opencensus_trace_function`:
@@ -135,7 +146,7 @@ To trace a function, use the `opencensus_trace_function`:
  * Trace each invocation of the specified function
  *
  * @param  string $functionName
- * @param  array|Closure $handler
+ * @param  array|callback $handler
  * @return bool
  */
 function opencensus_trace_function($functionName, $handler = []);
@@ -145,8 +156,8 @@ opencensus_trace_function('var_dump');
 var_dump(123);
 ```
 
-Just like tracing a method, you can provide a `$handler` option which can be an array or a closure. The behavior
-is the same as the method tracing, except that the closure will not be passed the scope parameter as there is
+Just like tracing a method, you can provide a `$handler` option which can be an array or a callback. The behavior
+is the same as the method tracing, except that the callback will not be passed the scope parameter as there is
 not object scope available.
 
 ```php

ext/opencensus_trace.c

@@ -174,14 +174,12 @@ static double opencensus_now()
 }
 
 /**
- * Call the provided Closure with the provided parameters to the traced
- * function. The Closure must return an array or an E_WARNING is raised.
+ * Call the provided callback with the provided parameters to the traced
+ * function. The callback must return an array or an E_WARNING is raised.
  */
-static int opencensus_trace_zend_fcall_closure(zend_execute_data *execute_data, opencensus_trace_span_t *span, zval *closure, zval *closure_result TSRMLS_DC)
+static int opencensus_trace_call_user_function_callback(zend_execute_data *execute_data, opencensus_trace_span_t *span, zval *callback, zval *callback_result TSRMLS_DC)
 {
     int i, num_args = EX_NUM_ARGS(), has_scope = 0;
-    zend_fcall_info fci;
-    zend_fcall_info_cache fcc;
     zval *args = emalloc((num_args + 1) * sizeof(zval));
 
     if (getThis() == NULL) {
@@ -195,28 +193,7 @@ static int opencensus_trace_zend_fcall_closure(zend_execute_data *execute_data,
         ZVAL_ZVAL(&args[i + has_scope], EX_VAR_NUM(i), 0, 1);
     }
 
-    if (zend_fcall_info_init(
-            closure,
-            0,
-            &fci,
-            &fcc,
-            NULL,
-            NULL
-            TSRMLS_CC
-        ) != SUCCESS) {
-        efree(args);
-        return FAILURE;
-    };
-
-    ZVAL_NULL(closure_result);
-
-    fci.retval = closure_result;
-    fci.params = &args[0];
-    fci.param_count = num_args + has_scope;
-
-    fcc.initialized = 1;
-
-    if (zend_call_function(&fci, &fcc TSRMLS_CC) != SUCCESS) {
+    if (call_user_function_ex(EG(function_table), NULL, callback, callback_result, num_args + has_scope, args, 0, NULL) != SUCCESS) {
         efree(args);
         return FAILURE;
     }
@@ -226,7 +203,7 @@ static int opencensus_trace_zend_fcall_closure(zend_execute_data *execute_data,
         return FAILURE;
     }
 
-    if (Z_TYPE_P(closure_result) != IS_ARRAY) {
+    if (Z_TYPE_P(callback_result) != IS_ARRAY) {
         /* only raise the warning if the closure succeeded */
         php_error_docref(NULL, E_WARNING, "Trace callback should return array");
         return FAILURE;
@@ -237,21 +214,23 @@ static int opencensus_trace_zend_fcall_closure(zend_execute_data *execute_data,
 
 /**
  * Handle the callback for the traced method depending on the type
- * - if the zval is an array, then assume it's the trace span initialization
+ * - if the zval is an associative array, then assume it's the trace span initialization
  *   options
+ * - if the zval is an array that looks like a callable, then assume it's a callable
  * - if the zval is a Closure, then execute the closure and take the results as
  *   the trace span initialization options
  */
 static void opencensus_trace_execute_callback(opencensus_trace_span_t *span, zend_execute_data *execute_data, zval *span_options TSRMLS_DC)
 {
-    if (Z_TYPE_P(span_options) == IS_ARRAY) {
-        opencensus_trace_span_apply_span_options(span, span_options);
-    } else if ( (Z_TYPE_P(span_options) == IS_OBJECT) &&
-                (Z_OBJCE_P(span_options) == zend_ce_closure)) {
-        zval closure_result;
-        if (opencensus_trace_zend_fcall_closure(execute_data, span, span_options, &closure_result TSRMLS_CC) == SUCCESS) {
-            opencensus_trace_span_apply_span_options(span, &closure_result);
+    zend_string *callback_name;
+    if (zend_is_callable(span_options, 0, &callback_name)) {
+        zval callback_result;
+        if (opencensus_trace_call_user_function_callback(execute_data, span, span_options, &callback_result TSRMLS_CC) == SUCCESS) {
+            opencensus_trace_span_apply_span_options(span, &callback_result);
         }
+        zend_string_release(callback_name);
+    } else if (Z_TYPE_P(span_options) == IS_ARRAY) {
+        opencensus_trace_span_apply_span_options(span, span_options);
     }
 }
 
@@ -534,7 +513,7 @@ void opencensus_trace_execute_internal(INTERNAL_FUNCTION_PARAMETERS)
  * Register the provided function for tracing.
  *
  * @param string $functionName
- * @param array|Closure $handler
+ * @param array|callable $handler
  * @return bool
  */
 PHP_FUNCTION(opencensus_trace_function)
@@ -565,7 +544,7 @@ PHP_FUNCTION(opencensus_trace_function)
  *
  * @param string $className
  * @param string $methodName
- * @param array|Closure $handler
+ * @param array|callable $handler
  * @return bool
  */
 PHP_FUNCTION(opencensus_trace_method)

ext/tests/function_callback_array.phpt

@@ -0,0 +1,49 @@
+--TEST--
+OpenCensus Trace: Customize the trace span options for a function with a callback array
+--FILE--
+<?php
+
+require_once(__DIR__ . '/common.php');
+
+class CallbackTest
+{
+    public static function handle()
+    {
+        return ['name' => 'foo', 'startTime' => 0.1, 'labels' => ['asdf' => 'qwer', 'zxcv' => 'jkl;']];
+    }
+}
+
+// 1: Sanity test a simple profile run
+opencensus_trace_function("bar", ['CallbackTest', 'handle']);
+bar();
+$traces = opencensus_trace_list();
+echo "Number of traces: " . count($traces) . "\n";
+$span = $traces[0];
+
+$test = gettype($span->spanId());
+echo "Span id is a $test\n";
+
+echo "Span name is: '{$span->name()}'\n";
+
+$test = gettype($span->startTime()) == 'double';
+echo "Span startTime is a double: $test\n";
+
+echo "Span startTime is: '{$span->startTime()}'\n";
+
+$test = gettype($span->endTime()) == 'double';
+echo "Span endTime is a double: $test\n";
+
+print_r($span->labels());
+?>
+--EXPECT--
+Number of traces: 1
+Span id is a integer
+Span name is: 'foo'
+Span startTime is a double: 1
+Span startTime is: '0.1'
+Span endTime is a double: 1
+Array
+(
+    [asdf] => qwer
+    [zxcv] => jkl;
+)

ext/tests/function_callback_callable_wrong_return.phpt

@@ -0,0 +1,17 @@
+--TEST--
+OpenCensus Trace: Callback returning a non-array response should yield a warning.
+--FILE--
+<?php
+
+require_once(__DIR__ . '/common.php');
+
+function wrong_return($x)
+{
+    return $x;
+}
+
+opencensus_trace_function('foo', 'wrong_return');
+foo(3);
+?>
+--EXPECTF--
+Warning: main(): Trace callback should return array in %s on line %d

ext/tests/function_callback_static_string.phpt

@@ -0,0 +1,49 @@
+--TEST--
+OpenCensus Trace: Customize the trace span options for a function with a callback string with static method
+--FILE--
+<?php
+
+require_once(__DIR__ . '/common.php');
+
+class MyCallback
+{
+    public static function callbackHandle()
+    {
+        return ['name' => 'foo', 'startTime' => 0.1, 'labels' => ['asdf' => 'qwer', 'zxcv' => 'jkl;']];
+    }
+}
+
+// 1: Sanity test a simple profile run
+opencensus_trace_function("bar", 'MyCallback::callbackHandle');
+bar();
+$traces = opencensus_trace_list();
+echo "Number of traces: " . count($traces) . "\n";
+$span = $traces[0];
+
+$test = gettype($span->spanId());
+echo "Span id is a $test\n";
+
+echo "Span name is: '{$span->name()}'\n";
+
+$test = gettype($span->startTime()) == 'double';
+echo "Span startTime is a double: $test\n";
+
+echo "Span startTime is: '{$span->startTime()}'\n";
+
+$test = gettype($span->endTime()) == 'double';
+echo "Span endTime is a double: $test\n";
+
+print_r($span->labels());
+?>
+--EXPECT--
+Number of traces: 1
+Span id is a integer
+Span name is: 'foo'
+Span startTime is a double: 1
+Span startTime is: '0.1'
+Span endTime is a double: 1
+Array
+(
+    [asdf] => qwer
+    [zxcv] => jkl;
+)

ext/tests/function_callback_string.phpt

@@ -0,0 +1,46 @@
+--TEST--
+OpenCensus Trace: Customize the trace span options for a function with a callback string
+--FILE--
+<?php
+
+require_once(__DIR__ . '/common.php');
+
+function callbackHandle()
+{
+    return ['name' => 'foo', 'startTime' => 0.1, 'labels' => ['asdf' => 'qwer', 'zxcv' => 'jkl;']];
+}
+
+// 1: Sanity test a simple profile run
+opencensus_trace_function("bar", 'callbackHandle');
+bar();
+$traces = opencensus_trace_list();
+echo "Number of traces: " . count($traces) . "\n";
+$span = $traces[0];
+
+$test = gettype($span->spanId());
+echo "Span id is a $test\n";
+
+echo "Span name is: '{$span->name()}'\n";
+
+$test = gettype($span->startTime()) == 'double';
+echo "Span startTime is a double: $test\n";
+
+echo "Span startTime is: '{$span->startTime()}'\n";
+
+$test = gettype($span->endTime()) == 'double';
+echo "Span endTime is a double: $test\n";
+
+print_r($span->labels());
+?>
+--EXPECT--
+Number of traces: 1
+Span id is a integer
+Span name is: 'foo'
+Span startTime is a double: 1
+Span startTime is: '0.1'
+Span endTime is a double: 1
+Array
+(
+    [asdf] => qwer
+    [zxcv] => jkl;
+)

ext/tests/method_callback_array.phpt

@@ -0,0 +1,50 @@
+--TEST--
+OpenCensus Trace: Customize the trace span options for a function with a callback array
+--FILE--
+<?php
+
+require_once(__DIR__ . '/common.php');
+
+class CallbackTest
+{
+    public static function handle()
+    {
+        return ['name' => 'foo', 'startTime' => 0.1, 'labels' => ['asdf' => 'qwer', 'zxcv' => 'jkl;']];
+    }
+}
+
+// 1: Sanity test a simple profile run
+opencensus_trace_method("Foo", "bar", ['CallbackTest', 'handle']);
+$f = new Foo();
+$f->bar();
+$traces = opencensus_trace_list();
+echo "Number of traces: " . count($traces) . "\n";
+$span = $traces[0];
+
+$test = gettype($span->spanId());
+echo "Span id is a $test\n";
+
+echo "Span name is: '{$span->name()}'\n";
+
+$test = gettype($span->startTime()) == 'double';
+echo "Span startTime is a double: $test\n";
+
+echo "Span startTime is: '{$span->startTime()}'\n";
+
+$test = gettype($span->endTime()) == 'double';
+echo "Span endTime is a double: $test\n";
+
+print_r($span->labels());
+?>
+--EXPECT--
+Number of traces: 1
+Span id is a integer
+Span name is: 'foo'
+Span startTime is a double: 1
+Span startTime is: '0.1'
+Span endTime is a double: 1
+Array
+(
+    [asdf] => qwer
+    [zxcv] => jkl;
+)