Merge pull request #941 from DataDog/levi/build/legacy-warning

Warn on dd_trace usage if DD_TRACE_WARN_LEGACY_DD_TRACE
DataDog · Jul 7, 2020 · 4eb3ac1 · 4eb3ac1
2 parents 4f8d8c1 + 0a65f8b
commit 4eb3ac1
Show file tree

Hide file tree

Showing 54 changed files with 136 additions and 4 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -440,6 +440,7 @@ jobs:
             cd /usr/local/src/php
             export DD_TRACE_CLI_ENABLED=true
             export DD_TRACE_STARTUP_LOGS=0
+            export DD_TRACE_WARN_LEGACY_DD_TRACE=0
             export REPORT_EXIT_STATUS=1
             export TEST_PHP_JUNIT=/tmp/artifacts/tests/php-tests.xml
             mkdir -p /tmp/artifacts/tests

diff --git a/package.xml b/package.xml
@@ -340,6 +340,7 @@
                         <file name="with_params_function_hook.phpt" role="test" />
                         <file name="with_params_method_hook.phpt" role="test" />
                     </dir>
+                    <file name="dd_trace_warning.phpt" role="test" />
                     <file name="destructor_called_when_this_gets_out_of_scope.phpt" role="test" />
                     <file name="enable_throw_exception_if_overridable_doesnt_exist.phpt" role="test" />
                     <file name="from_php_7_3_test_user_streams_consumed_bug.phpt" role="test" />

diff --git a/src/ext/configuration.h b/src/ext/configuration.h
@@ -136,6 +136,7 @@ void ddtrace_config_shutdown(void);
     INT(get_dd_trace_beta_high_memory_pressure_percent, "DD_TRACE_BETA_HIGH_MEMORY_PRESSURE_PERCENT", 80,            \
         "reaching this percent threshold of a span buffer will trigger background thread "                           \
         "to attempt to flush existing data to trace agent")                                                          \
+    BOOL(get_dd_trace_warn_legacy_dd_trace, "DD_TRACE_WARN_LEGACY_DD_TRACE", true)                                   \
     CHAR(get_dd_version, "DD_VERSION", "")
 
 // render all configuration getters and define memoization struct

diff --git a/src/ext/ddtrace.c b/src/ext/ddtrace.c
@@ -51,6 +51,8 @@
 bool ddtrace_blacklisted_disable_legacy;
 bool ddtrace_has_blacklisted_module;
 
+atomic_int ddtrace_warn_legacy_api;
+
 ZEND_DECLARE_MODULE_GLOBALS(ddtrace)
 
 PHP_INI_BEGIN()
@@ -284,6 +286,7 @@ static PHP_MINIT_FUNCTION(ddtrace) {
     // config initialization needs to be at the top
     ddtrace_initialize_config(TSRMLS_C);
     _dd_disable_if_incompatible_sapi_detected(TSRMLS_C);
+    atomic_init(&ddtrace_warn_legacy_api, 1);
 
     /* This allows an extension (e.g. extension=ddtrace.so) to have zend_engine
      * hooks too, but not loadable as zend_extension=ddtrace.so.
@@ -652,6 +655,12 @@ static BOOL_T _parse_config_array(zval *config_array, zval **tracing_closure, ui
     return TRUE;
 }
 
+static bool ddtrace_should_warn_legacy(void) {
+    int expected = 1;
+    return atomic_compare_exchange_strong(&ddtrace_warn_legacy_api, &expected, 0) &&
+           get_dd_trace_warn_legacy_dd_trace();
+}
+
 static PHP_FUNCTION(dd_trace) {
     PHP5_UNUSED(return_value_used, this_ptr, return_value_ptr);
     zval *function = NULL;
@@ -680,10 +689,23 @@ static PHP_FUNCTION(dd_trace) {
 
         RETURN_BOOL(0);
     }
-    if (class_name) {
-        DD_PRINTF("Class name: %s", Z_STRVAL_P(class_name));
+
+    if (ddtrace_should_warn_legacy()) {
+#if PHP_VERSION_ID < 50500
+        char *message =
+            "dd_trace DEPRECATION NOTICE: the function `dd_trace` is deprecated and will become a no-op in the next "
+            "release, and eventually will be removed. Set DD_TRACE_WARN_LEGACY_DD_TRACE=0 to suppress this warning.";
+        ddtrace_log_err(message);
+#else
+        char *message =
+            "dd_trace DEPRECATION NOTICE: the function `dd_trace` (target: %s%s%s) is deprecated and will become a "
+            "no-op in the next release, and eventually will be removed. Please follow "
+            "https://github.com/DataDog/dd-trace-php/issues/924 for instructions to update your code; set "
+            "DD_TRACE_WARN_LEGACY_DD_TRACE=0 to suppress this warning.";
+        ddtrace_log_errf(message, class_name ? Z_STRVAL_P(class_name) : "", class_name ? "::" : "",
+                         Z_STRVAL_P(function));
+#endif
     }
-    DD_PRINTF("Function name: %s", Z_STRVAL_P(function));
 
     if (ddtrace_blacklisted_disable_legacy && !get_dd_trace_ignore_legacy_blacklist()) {
         ddtrace_log_debugf(

diff --git a/tests/ext/access_modifier_method_access_hook.phpt b/tests/ext/access_modifier_method_access_hook.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check object's private and protected methods can be invoked from a callback.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Foo

diff --git a/tests/ext/access_modifier_property_access_hook.phpt b/tests/ext/access_modifier_property_access_hook.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check object's private and protected properties can be accessed from a callback.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test

diff --git a/tests/ext/allow_overriding_before_overrided_methods_functions_are_defined.phpt b/tests/ext/allow_overriding_before_overrided_methods_functions_are_defined.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Override function/method before its defined
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 dd_trace("Test", "m", function() {

diff --git a/tests/ext/case_insensitive_class_hook.phpt b/tests/ext/case_insensitive_class_hook.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check if for case insensitive class name support
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Base {
@@ -19,4 +21,4 @@ dd_trace("base", "method", function () {
 ?>
 --EXPECT--
 HOOK Base::method
-HOOK Base::method
+HOOK Base::method
diff --git a/tests/ext/case_insensitive_method_hook.phpt b/tests/ext/case_insensitive_method_hook.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check if we can override method from a parent class using case insensitive matching
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Ancestor {

diff --git a/tests/ext/closure_accessing_outside_variables.phpt b/tests/ext/closure_accessing_outside_variables.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check if closure can safely use variable names also present in outside scope
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 // variable present in outside scope

diff --git a/tests/ext/closure_set_inside_object_methods.phpt b/tests/ext/closure_set_inside_object_methods.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check if closure can safely use variable names also present in outside scope
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test {

diff --git a/tests/ext/dd_trace_api_error.phpt b/tests/ext/dd_trace_api_error.phpt
@@ -2,6 +2,7 @@
 dd_trace() declarative API error cases
 --ENV--
 DD_TRACE_DEBUG=1
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 # Functions

diff --git a/tests/ext/dd_trace_can_skip_original_call.phpt b/tests/ext/dd_trace_can_skip_original_call.phpt
@@ -1,5 +1,7 @@
 --TEST--
 dd_trace can skip over the call it instruments (LEGACY BEHAVIOR -- DO NOT RELY ON)
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 

diff --git a/tests/ext/dd_trace_forward_call_error.phpt b/tests/ext/dd_trace_forward_call_error.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Error conditions for dd_trace_forward_call()
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 // Out of closure context

diff --git a/tests/ext/dd_trace_forward_call_for_functions.phpt b/tests/ext/dd_trace_forward_call_for_functions.phpt
@@ -1,5 +1,7 @@
 --TEST--
 The original function call is invoked from the closure
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --SKIPIF--
 <?php if (PHP_MAJOR_VERSION > 5) die('skip: test requires legacy API'); ?>
 --FILE--

diff --git a/tests/ext/dd_trace_forward_call_from_include.phpt b/tests/ext/dd_trace_forward_call_from_include.phpt
@@ -1,5 +1,7 @@
 --TEST--
 The original method call is invoked from an include file
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 function doStuff($foo)

diff --git a/tests/ext/dd_trace_forward_call_with_inheritance.phpt b/tests/ext/dd_trace_forward_call_with_inheritance.phpt
@@ -1,5 +1,7 @@
 --TEST--
 The original method call is invoked from a sub class
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 

diff --git a/tests/ext/dd_trace_forward_call_with_private_callback.phpt b/tests/ext/dd_trace_forward_call_with_private_callback.phpt
@@ -1,5 +1,7 @@
 --TEST--
 A private method can be used as callback with dd_trace_forward_call()
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --SKIPIF--
 <?php if (PHP_MAJOR_VERSION > 5) die('skip: test requires legacy API'); ?>
 --FILE--

diff --git a/tests/ext/dd_trace_tracer_is_limited_hard.phpt b/tests/ext/dd_trace_tracer_is_limited_hard.phpt
@@ -4,6 +4,7 @@ dd_trace_tracer_is_limited() limits the tracer with a hard span limit
 <?php if (PHP_MAJOR_VERSION > 5) die('skip: test requires legacy API'); ?>
 --ENV--
 DD_TRACE_SPANS_LIMIT=1000
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 dd_trace('array_sum', function () {

diff --git a/tests/ext/dd_trace_warning.phpt b/tests/ext/dd_trace_warning.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Warn on dd_trace usage only once
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=1
+--SKIPIF--
+<?php if (PHP_VERSION_ID < 50500) die("skip: the warning is different on PHP 5.4"); ?>
+--FILE--
+<?php
+dd_trace('dd_trace_noop', function () {});
+dd_trace('dd_trace_noop', function () {});
+echo "Done.\n";
+?>
+--EXPECT--
+dd_trace DEPRECATION NOTICE: the function `dd_trace` (target: dd_trace_noop) is deprecated and will become a no-op in the next release, and eventually will be removed. Please follow https://github.com/DataDog/dd-trace-php/issues/924 for instructions to update your code; set DD_TRACE_WARN_LEGACY_DD_TRACE=0 to suppress this warning.
+Done.
diff --git a/tests/ext/destructor_called_when_this_gets_out_of_scope.phpt b/tests/ext/destructor_called_when_this_gets_out_of_scope.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Test if desctructor is called when variable goes out of scope
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 

diff --git a/tests/ext/disable_tracing_disables_tracing.phpt b/tests/ext/disable_tracing_disables_tracing.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Disable tracing disables all tracing from happening
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test {

diff --git a/tests/ext/do_not_check_if_class_or_function_exists_by_default.phpt b/tests/ext/do_not_check_if_class_or_function_exists_by_default.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Do not throw exceptions when veryfying if class/method and function exists.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 

diff --git a/tests/ext/enable_throw_exception_if_overridable_doesnt_exist.phpt b/tests/ext/enable_throw_exception_if_overridable_doesnt_exist.phpt
@@ -3,6 +3,8 @@ Toggle checking if overrided class doesn't exist
 --INI--
 ddtrace.strict_mode=1
 ddtrace.request_init_hook=
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 try {

diff --git a/tests/ext/keep_spans_in_limited_tracing_internal_functions.phpt b/tests/ext/keep_spans_in_limited_tracing_internal_functions.phpt
@@ -4,6 +4,7 @@
 <?php if (PHP_MAJOR_VERSION > 5) die('skip: test requires legacy API'); ?>
 --ENV--
 DD_TRACE_SPANS_LIMIT=5
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 dd_trace('array_sum', function () {

diff --git a/tests/ext/keep_spans_in_limited_tracing_internal_methods.phpt b/tests/ext/keep_spans_in_limited_tracing_internal_methods.phpt
@@ -4,6 +4,7 @@
 <?php if (PHP_MAJOR_VERSION > 5) die('skip: test requires legacy API'); ?>
 --ENV--
 DD_TRACE_SPANS_LIMIT=5
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 date_default_timezone_set('UTC');

diff --git a/tests/ext/keep_spans_in_limited_tracing_userland_functions.phpt b/tests/ext/keep_spans_in_limited_tracing_userland_functions.phpt
@@ -2,6 +2,7 @@
 [Legacy] Keep spans in limited mode (userland functions)
 --ENV--
 DD_TRACE_SPANS_LIMIT=5
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 function myFunc1($foo) {

diff --git a/tests/ext/keep_spans_in_limited_tracing_userland_methods.phpt b/tests/ext/keep_spans_in_limited_tracing_userland_methods.phpt
@@ -2,6 +2,7 @@
 [Legacy] Keep spans in limited mode (userland methods)
 --ENV--
 DD_TRACE_SPANS_LIMIT=5
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class MyClass

diff --git a/tests/ext/method_invoked_via_reflection.phpt b/tests/ext/method_invoked_via_reflection.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Method invoked via refloction correctly returning created object
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test {

diff --git a/tests/ext/method_returning_array.phpt b/tests/ext/method_returning_array.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check method can be overwritten and we're able to call original method returning an array
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test {

diff --git a/tests/ext/multiple_instrumentations.phpt b/tests/ext/multiple_instrumentations.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Verify Multiple functions and methods will be instrumented successfully
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 function test_a($a){

diff --git a/tests/ext/namespaces.phpt b/tests/ext/namespaces.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Verify functions and methods can be overriden even when in namespaces.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 namespace Func {

diff --git a/tests/ext/new_static.phpt b/tests/ext/new_static.phpt
@@ -1,5 +1,7 @@
 --TEST--
 New static instantiates from expected class
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 abstract class Foo {

diff --git a/tests/ext/overriding_construct.phpt b/tests/ext/overriding_construct.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check if we can override method from a parent class in a descendant class
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test {

diff --git a/tests/ext/overriding_method_defined_in_parent.phpt b/tests/ext/overriding_method_defined_in_parent.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check if we can override method from a parent class in a descendant class
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Ancestor {

diff --git a/tests/ext/private_method_hook.phpt b/tests/ext/private_method_hook.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check private method can be overwritten and we are able to call original.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test

diff --git a/tests/ext/private_self_access.phpt b/tests/ext/private_self_access.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Private self access
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 

diff --git a/tests/ext/protected_method_hook.phpt b/tests/ext/protected_method_hook.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check protected method can be overwritten and we are able to call original.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 

diff --git a/tests/ext/recursion.phpt b/tests/ext/recursion.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Verify recursive execution works by only overriding outermost invocation.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 function test($c, $end){

diff --git a/tests/ext/reset_configured_overrides.phpt b/tests/ext/reset_configured_overrides.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Configured overrides can be safely reset.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --FILE--
 <?php
 class Test {

diff --git a/tests/ext/reset_function_tracing.phpt b/tests/ext/reset_function_tracing.phpt
@@ -1,5 +1,7 @@
 --TEST--
 Check a function can be untraced.
+--ENV--
+DD_TRACE_WARN_LEGACY_DD_TRACE=0
 --SKIPIF--
 <?php if (PHP_MAJOR_VERSION > 5) die('skip: test requires legacy API'); ?>
 --FILE--