fix(drupal): hook the theme engine service and guarantee render hook removal

src/DDTrace/Integrations/Drupal/DrupalIntegration.php

@@ -228,115 +228,159 @@ static function ($registry, $scope, $args) {
             }
         );
 
-        trace_method(
-            'Drupal\Core\Theme\ThemeManager',
-            'render',
-            [
-                'recurse' => true,
-                'prehook' => function (SpanData $span, $args) {
-                    $span->name = 'drupal.theme.render';
-                    $span->service = \ddtrace_config_app_name('drupal');
-                    Integration::tagFrameworkServiceSource($span, 'drupal');
-                    $span->type = Type::WEB_SERVLET;
-                    $span->meta[Tag::COMPONENT] = DrupalIntegration::NAME;
+        // The span of the executing ThemeManager::render frame. The render function is hooked in its
+        // own frame, so it cannot reach that span on its own.
+        $renderSpan = null;
+        $legacyRenderHooks = [];
+
+        $tagTemplateFile = static function ($file) use (&$renderSpan) {
+            if ($renderSpan) {
+                $renderSpan->meta['drupal.template.file'] = $file;
+            }
+        };
 
-                    /** @var \Drupal\Core\Theme\ThemeManager $this */
-                    $activeTheme = $this->getActiveTheme();
-                    $themeName = $activeTheme->getName();
-                    $themeEngine = $activeTheme->getEngine();
+        $tagLegacyRender = static function (HookData $renderHook) use ($tagTemplateFile) {
+            $tagTemplateFile($renderHook->args[0]);
+        };
+        // Twig is both the default engine and core's fallback when {engine}_render_template() is
+        // missing, and the deprecated global does not delegate to the 11.3+ service.
+        $legacyRenderHooks['twig'] = install_hook('twig_render_template', $tagLegacyRender);
 
-                    if (!empty($themeName)) {
-                        $span->meta['drupal.render.theme'] = $themeName;
-                    }
+        // Drupal 11.3+ renders through a theme_engine service instead of {engine}_render_template().
+        install_hook(
+            'Drupal\Core\Theme\ThemeEngineInterface::renderTemplate',
+            static function (HookData $hook) use ($tagTemplateFile) {
+                $file = $hook->args[0];
+                // Core passes the path without its extension here; re-append it to keep the pre-11.3 value.
+                if (isset($hook->instance) && $hook->instance instanceof \Drupal\Core\Template\TwigThemeEngine) {
+                    $file .= '.html.twig';
+                }
+                $tagTemplateFile($file);
+            }
+        );
 
-                    if (!empty($themeEngine)) {
-                        $span->meta['drupal.render.engine'] = $themeEngine;
-                        if (function_exists("{$themeEngine}_render_template")) {
-                            $renderFunction = "{$themeEngine}_render_template";
+        install_hook(
+            'Drupal\Core\Theme\ThemeManager::render',
+            function (HookData $hookData) use (&$renderSpan, &$legacyRenderHooks, $tagLegacyRender) {
+                // install_hook, unlike trace_method, still runs both callbacks past the span limit;
+                // clearing $renderSpan keeps this frame's template off the enclosing render's span.
+                if (\dd_trace_tracer_is_limited()) {
+                    $hookData->data = $renderSpan;
+                    $renderSpan = null;
+                    return;
+                }
 
-                            // The theme engine may use a different extension and a different renderer
-                            // Moreover, Drupal can use different themes in the same application
-                            // The render function will always be called during the ThemeManager::render call
-                            install_hook(
-                                $renderFunction,
-                                static function (HookData $hook) use ($span) {
-                                    $span->meta['drupal.template.file'] = $hook->args[0];
-                                    remove_hook($hook->id);
-                                }
-                            );
+                $span = $hookData->span();
+                $hookData->data = $renderSpan;
+                $renderSpan = $span;
+
+                $span->name = 'drupal.theme.render';
+                $span->service = \ddtrace_config_app_name('drupal');
+                Integration::tagFrameworkServiceSource($span, 'drupal');
+                $span->type = Type::WEB_SERVLET;
+                $span->meta[Tag::COMPONENT] = DrupalIntegration::NAME;
+
+                /** @var \Drupal\Core\Theme\ThemeManager $this */
+                $activeTheme = $this->getActiveTheme();
+                $themeName = $activeTheme->getName();
+                $themeEngine = $activeTheme->getEngine();
+
+                if (!empty($themeName)) {
+                    $span->meta['drupal.render.theme'] = $themeName;
+                }
+
+                if (empty($themeEngine)) {
+                    return;
+                }
+                $span->meta['drupal.render.engine'] = $themeEngine;
+
+                // Drupal <= 11.2 renders through the {engine}_render_template() global. One hook per
+                // engine name for the whole request; $renderSpan does the attribution.
+                if (!isset($legacyRenderHooks[$themeEngine])) {
+                    $legacyRenderHooks[$themeEngine] =
+                        install_hook("{$themeEngine}_render_template", $tagLegacyRender);
+                }
+            },
+            function (HookData $hookData) use (&$renderSpan) {
+                // $renderSpan is this frame's span, or null when the begin callback bailed out
+                // past the span limit. $hookData->data holds the enclosing render's span.
+                $span = $renderSpan;
+                $renderSpan = $hookData->data;
+                if (!$span) {
+                    return;
+                }
+
+                /** @var null|\Drupal\Core\Theme\Registry $themeRegistry */
+                $themeRegistry = ObjectKVStore::get($this, 'theme_registry');
+                if (!$themeRegistry) {
+                    return;
+                }
+
+                $runtimeThemeRegistry = $themeRegistry->getRuntime();
+                $hook = $hookData->args[0];
+
+                if (is_array($hook)) {
+                    foreach ($hook as $candidate) {
+                        if ($runtimeThemeRegistry->has($candidate)) {
+                            break;
                         }
                     }
-                },
-                'posthook' => function (SpanData $span, $args) {
-                    /** @var null|\Drupal\Core\Theme\Registry $themeRegistry */
-                    $themeRegistry = ObjectKVStore::get($this, 'theme_registry');
-                    if ($themeRegistry) {
-                        $runtimeThemeRegistry = $themeRegistry->getRuntime();
-                        $hook = $args[0];
-
-                        if (is_array($hook)) {
-                            foreach ($hook as $candidate) {
-                                if ($runtimeThemeRegistry->has($candidate)) {
-                                    break;
-                                }
-                            }
-                            $hook = $candidate;
-                        }
+                    $hook = $candidate;
+                }
 
-                        $originalHook = $hook;
+                $originalHook = $hook;
 
-                        if (!$runtimeThemeRegistry->has($hook)) {
-                            // Iteratively strip everything after the last '__' delimiter, until an
-                            // implementation is found
-                            while ($pos = strrpos($hook, '__')) {
-                                $hook = substr($hook, 0, $pos);
-                                if ($runtimeThemeRegistry->has($hook)) {
-                                    break;
-                                }
-                            }
+                if (!$runtimeThemeRegistry->has($hook)) {
+                    // Iteratively strip everything after the last '__' delimiter, until an
+                    // implementation is found
+                    while ($pos = strrpos($hook, '__')) {
+                        $hook = substr($hook, 0, $pos);
+                        if ($runtimeThemeRegistry->has($hook)) {
+                            break;
                         }
+                    }
+                }
 
-                        if ($runtimeThemeRegistry->has($hook)) {
-                            $span->meta['drupal.render.hook'] = $span->resource = $hook;
-                            $info = $runtimeThemeRegistry->get($hook);
+                if (!$runtimeThemeRegistry->has($hook)) {
+                    $span->meta['drupal.render.hook'] = $span->resource = $originalHook;
+                    return;
+                }
 
-                            if (isset($info['base hook'])) {
-                                $span->meta['drupal.render.base_hook'] = $info['base hook'];
-                            }
+                $span->meta['drupal.render.hook'] = $span->resource = $hook;
+                $info = $runtimeThemeRegistry->get($hook);
 
-                            if (isset($info['type'])) {
-                                $span->meta['drupal.render.type'] = $info['type'];
-                            }
+                if (isset($info['base hook'])) {
+                    $span->meta['drupal.render.base_hook'] = $info['base hook'];
+                }
 
-                            if (isset($info['render element'])) {
-                                $span->meta['drupal.render.element'] = $info['render element'];
-                            }
+                if (isset($info['type'])) {
+                    $span->meta['drupal.render.type'] = $info['type'];
+                }
 
-                            if (isset($info['template'])) {
-                                $span->meta['drupal.template.template'] = $info['template'];
-                            }
+                if (isset($info['render element'])) {
+                    $span->meta['drupal.render.element'] = $info['render element'];
+                }
 
-                            if (isset($info['function'])) {
-                                $span->meta['drupal.render.theme_function'] = $info['function'];
-                            }
+                if (isset($info['template'])) {
+                    $span->meta['drupal.template.template'] = $info['template'];
+                }
 
-                            if (isset($info['path'])) {
-                                // The template can be from a different theme than the active one
-                                // Format: '.../themes/<theme_name>/...'
-                                $path = $info['path'];
-                                $themePathStart = strpos($path, '/themes/');
-                                if ($themePathStart !== false) {
-                                    $themePath = substr($path,  $themePathStart + 8); // Between '/themes/', 8 = strlen('/themes/')
-                                    $themePath = substr($themePath, 0, strpos($themePath, '/')); // Until the next '/'
-                                    $span->meta['drupal.template.theme'] = $themePath;
-                                }
-                            }
-                        } else {
-                            $span->meta['drupal.render.hook'] = $span->resource = $originalHook;
-                        }
+                if (isset($info['function'])) {
+                    $span->meta['drupal.render.theme_function'] = $info['function'];
+                }
+
+                if (isset($info['path'])) {
+                    // The template can be from a different theme than the active one
+                    // Format: '.../themes/<theme_name>/...'
+                    $path = $info['path'];
+                    $themePathStart = strpos($path, '/themes/');
+                    if ($themePathStart !== false) {
+                        $themePath = substr($path,  $themePathStart + 8); // Between '/themes/', 8 = strlen('/themes/')
+                        $themePath = substr($themePath, 0, strpos($themePath, '/')); // Until the next '/'
+                        $span->meta['drupal.template.theme'] = $themePath;
                     }
                 }
-            ]
+            }
         );
 
         hook_method(

tests/ext/integrations/drupal/drupal_integration.inc

@@ -0,0 +1,65 @@
+<?php
+
+require_once __DIR__ . '/drupal_root.inc';
+
+$root = dd_drupal_tracer_root();
+
+// Declared on demand: only a mid-request class declaration exercises hook re-registration.
+spl_autoload_register(function ($class) {
+    if ($class === 'Drupal\Core\Template\TwigThemeEngine') {
+        require __DIR__ . '/drupal_twig_engine.inc';
+    }
+});
+
+spl_autoload_register(function ($class) use ($root) {
+    if (strpos($class, 'DDTrace\\') !== 0) {
+        return;
+    }
+    $candidates = [
+        $root . '/src/' . str_replace('\\', '/', $class) . '.php',
+        // DDTrace\Tag, DDTrace\Type and friends live outside the PSR-4 tree.
+        $root . '/src/api/' . substr(strrchr($class, '\\'), 1) . '.php',
+    ];
+    foreach ($candidates as $file) {
+        if (is_file($file)) {
+            require $file;
+            return;
+        }
+    }
+    echo "autoload miss: $class\n";
+});
+
+// The hook budget is per target, so $probeTargets needs an entry per install site.
+function dd_drupal_render_report(array $spans, array $probeTargets)
+{
+    $tags = [];
+    $engines = [];
+    foreach ($spans as $span) {
+        if ($span['name'] !== 'drupal.theme.render') {
+            continue;
+        }
+        $file = isset($span['meta']['drupal.template.file']) ? $span['meta']['drupal.template.file'] : '<missing>';
+        $tags[$file] = (isset($tags[$file]) ? $tags[$file] : 0) + 1;
+        // Also reported so that a begin hook exception swallowed by the sandbox cannot pass silently.
+        $engine = isset($span['meta']['drupal.render.engine']) ? $span['meta']['drupal.render.engine'] : '<missing>';
+        $engines[$engine] = (isset($engines[$engine]) ? $engines[$engine] : 0) + 1;
+    }
+    ksort($tags);
+    ksort($engines);
+    foreach ($engines as $engine => $count) {
+        echo "drupal.render.engine[$engine] = $count\n";
+    }
+    foreach ($tags as $file => $count) {
+        echo "drupal.template.file[$file] = $count\n";
+    }
+
+    // A leaked per-render hook shows up as an exhausted budget on its target.
+    foreach ($probeTargets as $target) {
+        $probe = DDTrace\install_hook($target, function () {
+        });
+        echo "hook budget left [$target]: " . ($probe ? "yes" : "no") . "\n";
+        if ($probe) {
+            DDTrace\remove_hook($probe);
+        }
+    }
+}

tests/ext/integrations/drupal/drupal_root.inc

@@ -0,0 +1,15 @@
+<?php
+
+// Load the integration straight from src/, without relying on datadog.trace.sources_path.
+function dd_drupal_tracer_root()
+{
+    $dir = __DIR__;
+    while (!is_file($dir . '/src/DDTrace/Integrations/Drupal/DrupalIntegration.php')) {
+        $parent = dirname($dir);
+        if ($parent === $dir) {
+            return false;
+        }
+        $dir = $parent;
+    }
+    return $dir;
+}

tests/ext/integrations/drupal/drupal_twig_engine.inc

@@ -0,0 +1,24 @@
+<?php
+
+namespace Drupal\Core\Template;
+
+// Autoloaded on demand so the interface hook resolves against a class declared after init(),
+// as in a real request. Core passes $template_file without the extension; renderTemplate() appends it.
+class TwigThemeEngine implements \Drupal\Core\Theme\ThemeEngineInterface
+{
+    const EXTENSION = '.html.twig';
+
+    /** @var null|callable Invoked mid-render to model a template embedding another theme hook. */
+    public $nested;
+
+    public function renderTemplate(string $template_file, array $variables): string
+    {
+        $rendered = 'rendered ' . $template_file . self::EXTENSION;
+        if ($this->nested) {
+            $nested = $this->nested;
+            $this->nested = null;
+            $rendered .= $nested();
+        }
+        return $rendered;
+    }
+}