Merge pull request #559 from DataDog/labbati/sandboxing-eloquent

Migrate eloquent integration to sandboxed API
DataDog · Sep 18, 2019 · 8c3ba51 · 8c3ba51
2 parents 3a0d28c + 2aa0a1a
commit 8c3ba51
Show file tree

Hide file tree

Showing 34 changed files with 794 additions and 63 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -37,8 +37,9 @@ aliases:
     name: memcached_integration
 
   - &IMAGE_DOCKER_MYSQL
-    image: mysql:5.6
+    image: docker.pkg.github.com/datadog/dd-trace-ci/php-mysql-dev:5.6
     name: mysql_integration
+    <<: *DD_TRACE_CI_DOCKER_CREDENTIALS
     environment:
       - MYSQL_ROOT_PASSWORD=test
       - MYSQL_PASSWORD=test

diff --git a/bridge/dd_require_all.php b/bridge/dd_require_all.php
@@ -68,6 +68,7 @@
 require __DIR__ . '/../src/DDTrace/Integrations/PDO/PDOIntegration.php';
 require __DIR__ . '/../src/DDTrace/Integrations/PDO/PDOSandboxedIntegration.php';
 require __DIR__ . '/../src/DDTrace/Integrations/Eloquent/EloquentIntegration.php';
+require __DIR__ . '/../src/DDTrace/Integrations/Eloquent/EloquentSandboxedIntegration.php';
 require __DIR__ . '/../src/DDTrace/Integrations/Memcached/MemcachedIntegration.php';
 require __DIR__ . '/../src/DDTrace/Integrations/Curl/CurlIntegration.php';
 require __DIR__ . '/../src/DDTrace/Integrations/Mysqli/MysqliIntegration.php';

diff --git a/composer.json b/composer.json
@@ -323,7 +323,7 @@
         "laravel-42-test": "@composer test -- tests/Integrations/Laravel/V4",
 
         "laravel-57-update": "@composer --working-dir=tests/Frameworks/Laravel/Version_5_7 update",
-        "laravel-57-test": "@composer test -- tests/Integrations/Laravel/V5_7/CommonScenariosTest.php",
+        "laravel-57-test": "@composer test -- tests/Integrations/Laravel/V5_7",
 
         "laravel-58-update": "@composer --working-dir=tests/Frameworks/Laravel/Version_5_8 update",
         "laravel-58-test": "@composer test -- --testsuite=laravel-58-test",

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -73,7 +73,7 @@ services:
   'fpm': { <<: *base_php_service, image: 'circleci/ruby:2.5', depends_on: [] }
 
   mysql_integration:
-    image: mysql:5.6
+    image: docker.pkg.github.com/datadog/dd-trace-ci/php-mysql-dev:5.6
     ports:
       - "3306:3306"
     environment:

diff --git a/phpunit.xml b/phpunit.xml
@@ -24,7 +24,7 @@
             <directory>tests/Integrations/CLI/CakePHP/V2_8</directory>
         </testsuite>
         <testsuite name="laravel-58-test">
-            <file>tests/Integrations/Laravel/V5_8/CommonScenariosTest.php</file>
+            <directory>tests/Integrations/Laravel/V5_8</directory>
             <directory>tests/Integrations/CLI/Laravel/V5_8</directory>
         </testsuite>
         <testsuite name="slim-312-test">

diff --git a/src/DDTrace/Integrations/Eloquent/EloquentIntegration.php b/src/DDTrace/Integrations/Eloquent/EloquentIntegration.php
@@ -63,12 +63,9 @@ public static function load()
                 return dd_trace_forward_call();
             }
 
-            list($eloquentQueryBuilder) = func_get_args();
             $scope = $tracer->startIntegrationScopeAndSpan($integration, 'eloquent.insert');
             $span = $scope->getSpan();
-            $sql = $eloquentQueryBuilder->getQuery()->toSql();
-            $span->setTag(Tag::RESOURCE_NAME, $sql);
-            $span->setTag(Tag::DB_STATEMENT, $sql);
+            $span->setTag(Tag::RESOURCE_NAME, get_class($this));
             $span->setTag(Tag::SPAN_TYPE, Type::SQL);
 
             return include __DIR__ . '/../../try_catch_finally.php';
@@ -84,9 +81,7 @@ public static function load()
 
             $scope = $tracer->startIntegrationScopeAndSpan($integration, 'eloquent.update');
             $span = $scope->getSpan();
-            $sql = $eloquentQueryBuilder->getQuery()->toSql();
-            $span->setTag(Tag::RESOURCE_NAME, $sql);
-            $span->setTag(Tag::DB_STATEMENT, $sql);
+            $span->setTag(Tag::RESOURCE_NAME, get_class($this));
             $span->setTag(Tag::SPAN_TYPE, Type::SQL);
 
             return include __DIR__ . '/../../try_catch_finally.php';
@@ -100,7 +95,9 @@ public static function load()
             }
 
             $scope = $tracer->startIntegrationScopeAndSpan($integration, 'eloquent.delete');
-            $scope->getSpan()->setTag(Tag::SPAN_TYPE, Type::SQL);
+            $span = $scope->getSpan();
+            $span->setTag(Tag::SPAN_TYPE, Type::SQL);
+            $span->setTag(Tag::RESOURCE_NAME, get_class($this));
 
             return include __DIR__ . '/../../try_catch_finally.php';
         });

diff --git a/src/DDTrace/Integrations/Eloquent/EloquentSandboxedIntegration.php b/src/DDTrace/Integrations/Eloquent/EloquentSandboxedIntegration.php
@@ -0,0 +1,130 @@
+<?php
+
+namespace DDTrace\Integrations\Eloquent;
+
+use DDTrace\Configuration;
+use DDTrace\Integrations\Integration;
+use DDTrace\Integrations\SandboxedIntegration;
+use DDTrace\SpanData;
+use DDTrace\Tag;
+use DDTrace\Type;
+
+class EloquentSandboxedIntegration extends SandboxedIntegration
+{
+    const NAME = 'eloquent';
+
+    /**
+     * @var string The app name. Note that this value is used as a cache, you should use method getAppName().
+     */
+    private $appName;
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getName()
+    {
+        return self::NAME;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function init()
+    {
+        $integration = $this;
+
+        dd_trace_method(
+            'Illuminate\Database\Eloquent\Builder',
+            'getModels',
+            function (SpanData $span) use ($integration) {
+                $span->name = 'eloquent.get';
+                $sql = $this->getQuery()->toSql();
+                $span->resource = $sql;
+                $span->meta[Tag::DB_STATEMENT] = $sql;
+                $integration->setCommonValues($span);
+            }
+        );
+
+        dd_trace_method(
+            'Illuminate\Database\Eloquent\Model',
+            'performInsert',
+            function (SpanData $span) use ($integration) {
+                $span->name = 'eloquent.insert';
+                $span->resource = get_class($this);
+                $integration->setCommonValues($span);
+            }
+        );
+
+        dd_trace_method(
+            'Illuminate\Database\Eloquent\Model',
+            'performUpdate',
+            function (SpanData $span) use ($integration) {
+                $span->name = 'eloquent.update';
+                $span->resource = get_class($this);
+                $integration->setCommonValues($span);
+            }
+        );
+
+        dd_trace_method(
+            'Illuminate\Database\Eloquent\Model',
+            'delete',
+            function (SpanData $span) use ($integration) {
+                $span->name = 'eloquent.delete';
+                $span->resource = get_class($this);
+                $integration->setCommonValues($span);
+            }
+        );
+
+        dd_trace_method(
+            'Illuminate\Database\Eloquent\Model',
+            'destroy',
+            function (SpanData $span) use ($integration) {
+                $span->name = 'eloquent.destroy';
+                $span->resource = get_called_class();
+                $integration->setCommonValues($span);
+            }
+        );
+
+        dd_trace_method(
+            'Illuminate\Database\Eloquent\Model',
+            'refresh',
+            function (SpanData $span) use ($integration) {
+                $span->name = 'eloquent.refresh';
+                $span->resource = get_class($this);
+                $integration->setCommonValues($span);
+            }
+        );
+
+        return Integration::LOADED;
+    }
+
+    /**
+     * Set common values shared by many different spans.
+     *
+     * @param SpanData $span
+     */
+    public function setCommonValues(SpanData $span)
+    {
+        $span->type = Type::SQL;
+        $span->service = $this->getAppName();
+        $span->meta[Tag::INTEGRATION_NAME] = $this->getName();
+    }
+
+    /**
+     * @return string
+     */
+    public function getAppName()
+    {
+        if (null !== $this->appName) {
+            return $this->appName;
+        }
+
+        $name = Configuration::get()->appName();
+        if (empty($name) && is_callable('config')) {
+            $name = config('app.name');
+        }
+
+        $this->appName = $name ?: 'laravel';
+        return $this->appName;
+    }
+}
diff --git a/src/DDTrace/Integrations/Integration.php b/src/DDTrace/Integrations/Integration.php
@@ -29,7 +29,6 @@ abstract class Integration
      */
     abstract public function getName();
 
-
     public function __construct()
     {
         $this->configuration = $this->buildConfiguration();

diff --git a/src/DDTrace/Integrations/IntegrationsLoader.php b/src/DDTrace/Integrations/IntegrationsLoader.php
@@ -7,6 +7,7 @@
 use DDTrace\Integrations\Curl\CurlIntegration;
 use DDTrace\Integrations\ElasticSearch\V1\ElasticSearchIntegration;
 use DDTrace\Integrations\Eloquent\EloquentIntegration;
+use DDTrace\Integrations\Eloquent\EloquentSandboxedIntegration;
 use DDTrace\Integrations\Guzzle\GuzzleIntegration;
 use DDTrace\Integrations\Laravel\LaravelIntegration;
 use DDTrace\Integrations\Lumen\LumenIntegration;
@@ -74,7 +75,10 @@ public function __construct(array $integrations)
         $this->integrations = $integrations;
         // Sandboxed integrations get loaded with a feature flag
         if (Configuration::get()->isSandboxEnabled()) {
-            $this->integrations[PDOSandboxedIntegration::NAME] = '\DDTrace\Integrations\PDO\PDOSandboxedIntegration';
+            $this->integrations[EloquentSandboxedIntegration::NAME] =
+                '\DDTrace\Integrations\Eloquent\EloquentSandboxedIntegration';
+            $this->integrations[PDOSandboxedIntegration::NAME] =
+                '\DDTrace\Integrations\PDO\PDOSandboxedIntegration';
         }
     }
 

diff --git a/src/DDTrace/Tag.php b/src/DDTrace/Tag.php
@@ -17,6 +17,7 @@ class Tag
     const ERROR_MSG = 'error.msg'; // string representing the error message
     const ERROR_TYPE = 'error.type'; // string representing the type of the error
     const ERROR_STACK = 'error.stack'; // human readable version of the stack
+    const INTEGRATION_NAME = 'integration.name';
     const HTTP_METHOD = 'http.method';
     const HTTP_STATUS_CODE = 'http.status_code';
     const HTTP_URL = 'http.url';

diff --git a/tests/Common/IntegrationTestCase.php b/tests/Common/IntegrationTestCase.php
@@ -2,6 +2,8 @@
 
 namespace DDTrace\Tests\Common;
 
+use DDTrace\Integrations\IntegrationsLoader;
+use DDTrace\Util\Versions;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -11,15 +13,46 @@ abstract class IntegrationTestCase extends TestCase
 {
     use TracerTestTrait, SpanAssertionTrait;
 
+    const IS_SANDBOX = false;
+
+    public static function setUpBeforeClass()
+    {
+        parent::setUpBeforeClass();
+        if (!static::isSandboxed()) {
+            putenv('DD_TRACE_SANDBOX_ENABLED=false');
+        }
+        IntegrationsLoader::reload();
+    }
+
+    public static function tearDownAfterClass()
+    {
+        parent::tearDownAfterClass();
+        putenv('DD_TRACE_SANDBOX_ENABLED');
+    }
+
+    protected static function isSandboxed()
+    {
+        return static::IS_SANDBOX === true;
+    }
+
+    protected function setUp()
+    {
+        parent::setUp();
+        if (Versions::phpVersionMatches('5.4') && self::isSandboxed()) {
+            $this->markTestSkipped('Sandboxed tests are skipped on PHP 5.4.');
+        }
+    }
+
     /**
      * Checks the exact match of a set of SpanAssertion with the provided Spans.
      *
      * @param array[] $traces
      * @param SpanAssertion[] $expectedSpans
      * @param bool $isSandbox
      */
-    public function assertSpans($traces, $expectedSpans, $isSandbox = false)
+    public function assertSpans($traces, $expectedSpans, $isSandbox = null)
     {
+        $isSandbox = null === $isSandbox ? self::isSandboxed() : $isSandbox;
         $this->assertExpectedSpans($traces, $expectedSpans, $isSandbox);
     }
 

diff --git a/tests/Common/SpanAssertionTrait.php b/tests/Common/SpanAssertionTrait.php
@@ -28,9 +28,9 @@ public function assertOneExpectedSpan($traces, SpanAssertion $expectedSpan)
     {
         $spanChecker = new SpanChecker();
 
-        $found = array_filter($spanChecker->flattenTraces($traces), function ($span) use ($expectedSpan) {
+        $found = array_values(array_filter($spanChecker->flattenTraces($traces), function ($span) use ($expectedSpan) {
             return $span['name'] === $expectedSpan->getOperationName();
-        });
+        }));
 
         if (empty($found)) {
             TestCase::fail('Span not found in traces: ' . $expectedSpan->getOperationName());

diff --git a/tests/Common/SpanChecker.php b/tests/Common/SpanChecker.php
@@ -63,6 +63,8 @@ public function assertSpan($span, SpanAssertion $exp)
     {
         TestCase::assertNotNull($span, 'Expected span was not \'' . $exp->getOperationName() . '\' found.');
 
+        $spanMeta = isset($span['meta']) ? $span['meta'] : [];
+
         if ($exp->isOnlyCheckExistence()) {
             return;
         }
@@ -81,7 +83,7 @@ public function assertSpan($span, SpanAssertion $exp)
         );
         if ($exp->getExactTags() !== SpanAssertion::NOT_TESTED) {
             $filtered = [];
-            foreach ($span['meta'] as $key => $value) {
+            foreach ($spanMeta as $key => $value) {
                 if (!in_array($key, $exp->getExistingTagNames())) {
                     $filtered[$key] = $value;
                 }
@@ -116,7 +118,7 @@ public function assertSpan($span, SpanAssertion $exp)
                 $namePrefix . "Unexpected extra values for 'tags':\n" . print_r($filtered, true)
             );
             foreach ($exp->getExistingTagNames(isset($span['parent_id'])) as $tagName) {
-                TestCase::assertArrayHasKey($tagName, $span['meta']);
+                TestCase::assertArrayHasKey($tagName, $spanMeta);
             }
         }
         if ($exp->getExactMetrics() !== SpanAssertion::NOT_TESTED) {

diff --git a/tests/Common/TracerTestTrait.php b/tests/Common/TracerTestTrait.php
@@ -155,7 +155,7 @@ private function parseTracesFromDumpedData()
                 $span = new Span(
                     $rawSpan['name'],
                     $spanContext,
-                    $rawSpan['service'],
+                    isset($rawSpan['service']) ? $rawSpan['service'] : null,
                     $rawSpan['resource'],
                     $rawSpan['start']
                 );