feat(process-tags): Service Override Source Attribution#3948
Draft
Leiyks wants to merge 12 commits into
Draft
Conversation
Leiyks
changed the title
|
…svc_src) Implements the RFC "Service Override Source Attribution": tags every span whose service was actively chosen (vs. tracer's default) with a `_dd.svc_src` meta value identifying the source. Cleared for the default case (DD_SERVICE-set or auto-resolved). Sources emitted, per RFC categories: - `<integration_name>` — when the integration sets the service (Curl, Guzzle, PDO, Mysqli, SQLSRV, MongoDB, Memcached, AMQP, Kafka, ... via `Integration::handleInternalSpanServiceName`) - `opt.<option>` — when a config-driven split changes the service: * `opt.http_client_split_by_domain` (Curl, Guzzle, Psr18, Ratchet) * `opt.db_client_split_by_instance` (PDO, Mysqli, SQLSRV) * `opt.redis_client_split_by_host` (PHPRedis) - Framework root-span overrides (Laravel, LaravelQueue, Drupal, Laminas, Lumen, Slim, SymfonyMessenger, Roadrunner, Ratchet web mode, CakePHP) use the new `Integration::tagFrameworkServiceSource` helper: cleared when user-set DD_SERVICE drove the value, else tagged with the integration name. - `m` — manual API override at serialization-time fallback in `ddtrace_serialize_span_to_rust_span` when the span's service differs from the root's and no other tag was set. Inheritance per RFC: `ddtrace_inherit_span_properties` copies `_dd.svc_src` from parent meta at span creation; no back-propagation. Tests: - 3 new `.phpt` covering default-cleared, manual `'m'`, and parent->child inheritance. - Existing `.phpt` expectations updated where manual service overrides now correctly emit `_dd.svc_src = 'm'`. Out of scope (follow-up): - Predis SPLIT_BY_HOST: stores service via meta override (`service.name`) rather than `$span->service`. Different mechanism; worth a separate discussion. Refs RFC "Service Override Source Attribution".
Mirrors the existing pattern for _dd.p.dm / _dd.p.ksr / _dd.p.tid / runtime-id / _dd.code_origin.*: when not explicitly asserted, drop _dd.svc_src from the strict "extra tags" check so per-integration test suites don't need every test updated. Fixes 14 Memcache (and similar) integration-test failures on CI.
This reverts commit 89d443e.
…lock For each `Tag::COMPONENT => '<name>'` entry across the PHPUnit integration suite, inject `'_dd.svc_src' => '<same value>'` so the RFC contract is verified explicitly per test (instead of being filtered out at the SpanChecker level). Mechanical, 576 sites across 94 test files. Catches drift between the integration's NAME constant and what shows up on spans. (Reverts the SpanChecker filter from 89d443e; this is the proper-per-test enforcement the senior asked for.)
The previous mechanical pass (31f8140) inserted `_dd.svc_src` next to every Tag::COMPONENT line, assuming svc_src always equals the component value. That assumption holds for plain integrations but is wrong in four cases that the runtime actually produces: * Split-by-X paths (Guzzle DD_TRACE_HTTP_CLIENT_SPLIT_BY_DOMAIN, PHPRedis DD_TRACE_REDIS_CLIENT_SPLIT_BY_HOST): svc_src is the option name (opt.<flag>), not the component. * Integrations with DEFAULT_SERVICE_NAME != NAME (Predis): svc_src is the fallback service name (redis), not the integration NAME (predis). * Frameworks that don't call handleInternalSpanServiceName / call tagFrameworkServiceSource with DD_SERVICE set (Symfony, Nette, CodeIgniter, ZendFramework, plus DD_SERVICE-configured CakePHP/Slim scenarios): no svc_src is emitted at all. * Inherited spans (Laravel V4/V5_7 Eloquent): svc_src is inherited from the framework root (laravel), not the eloquent component. * Symfony console commands (CLI scenarios + ConsoleCommandTest): \$span->service is set manually by the integration, so the serializer default-tags svc_src=m. Also extends SnapshotTestTrait::stopAndCompareSnapshotSession to always add 'meta._dd.svc_src' to the ignore list, so snapshot tests don't drift on this new internal field.
…losures trace_method/install_hook rebind closure scope to the target class at runtime, so `self::NAME` inside non-static closures resolves to the hooked class (e.g. Illuminate\View\View) rather than the lexical integration class. PHP 8.x throws "Undefined constant <hooked>::NAME" and the entire integration hook fails. This was breaking Laravel/Lumen/LaravelQueue scenarios across many test targets (Lumen 100/52/81, Laravel 9x/10x/11x/Latest/Octane, etc). Switches all self::NAME / Integration::tagFrameworkServiceSource calls in these integrations to the explicit <Integration>::NAME form, matching the pre-existing pattern in the same files (e.g. LaravelIntegration::getServiceName() on the line above the fix). Also drops two svc_src expected lines my earlier sed missed (Swoole CommonScenariosTest, Zend V1_21 TraceSearchConfigTest with double quotes).
RatchetIntegration calls handleInternalSpanServiceName/tagFrameworkServiceSource which now tags _dd.svc_src='ratchet', and the websocket send/receive/close spans inherit svc_src from their handshake span (see line 264-266 of RatchetIntegration). My earlier mechanical pass missed this file because its assertions use lowercase 'component' => 'ratchet' rather than Tag::COMPONENT constant.
Leiyks
force-pushed
the
leiyks/svc-override-source-attribution
branch
from
48bb658 to
2e14e97
Compare
Critical: tagFrameworkServiceSource declared `: void`, which PHP 7.0 parses as a class reference in the current namespace. This raised TypeError "Return value must be an instance of DDTrace\Integrations\void" on every CakePHP V2_8 / V3_10 / Laravel 4/5_8 invocation, breaking all their integration tests. Drops the return type to match the codebase's PHP 7.0 baseline. Also drops the `string` parameter type for the same reason (consistency). Test expectation fixes: - Curl: split-by-domain svc_src='opt.http_client_split_by_domain' (was 'curl'), and drop svc_src in testNoFakeServices (DD_SERVICE set). - PHPRedis V3/V4 ClusterTest WithClusterName + ClusterNameAndSeeds: add svc_src='opt.redis_client_split_by_host' to the inline + baseTags() overrides. - Laravel V5_8/V8_x EloquentTest, V8_x InternalExceptionsTest / RouteCachingTest / TraceSearchConfigTest, V5_7 TraceSearchConfigTest, Lumen V5_2 TraceSearchConfigTest: add svc_src='laravel' or 'lumen' next to existing TAG::COMPONENT entries (uppercase TAG, missed by the earlier mechanical pass). - CLI CakePHP V3_10 CommonScenariosTest (inherited by V4_5): drop the two 'cakephp.console' svc_src expectations (DD_SERVICE clears them). - Laravel Octane Latest CommonScenariosTest: drop all 12 svc_src=laravel lines (DD_SERVICE='swoole_test_app' clears framework svc_src). - Ratchet V0_4: drop svc_src='ratchet' from the line-83 websocket.send with service='phpunit' (outgoingMessage doesn't set svc_src and the span doesn't inherit from a ratchet root). Add svc_src='ratchet' to testRatchetConnectFail's Connector.__invoke expectation (double-quote style missed by the earlier sweep).
Benchmarks [ tracer ]Benchmark execution time: 2026-06-09 17:31:42 Comparing candidate commit 9c8a45d in PR branch Found 5 performance improvements and 4 performance regressions! Performance is the same for 185 metrics, 0 unstable metrics.
|
- CakePHP V2_8/V3_10 (web): drop stray 'cakephp' svc_src lines my earlier sed missed (only touched V4_5 paths). - Yii Latest (4 files): drop \"yii\" double-quoted svc_src lines; Yii integration doesn't call tagFrameworkServiceSource so spans have no svc_src. - Laravel V8_x EloquentTest / InternalExceptionsTest / RouteCachingTest / TraceSearchConfigTest: change 'laravel' → 'Laravel' (V8 app config default APP_NAME is capitalized; getAppName() returns it as-is). - Laravel V8_x QueueTest: add '_dd.svc_src' to withExistingTagsNames in spanEventJobProcessing/Processed helpers (SpanChecker.php filters existing-tags out of the strict-extra check; without this svc_src triggers an 'extra' failure). - Mysqli: parameterize baseTags($svcSrc='mysqli') so testNoFakeServices can pass null (DD_SERVICE set, no svc_src on span); add '_dd.svc_src' to testConstructorConnectError's withExistingTagsNames.
Last iteration overcorrected — only EloquentTest goes through Eloquent's getAppName() (which returns the V8 app config name 'Laravel'). Every other V8_x span emits from Laravel's own loader via tagFrameworkServiceSource(\$span, LaravelIntegration::NAME), so svc_src='laravel' (lowercase NAME constant). - InternalExceptionsTest / RouteCachingTest / TraceSearchConfigTest: revert 'Laravel' → 'laravel'. - InternalExceptionsTest testNotImplemented / testUnauthorized: add the missing svc_src on the laravel.request and laravel.view.render spans that triggered \"extra\" failures. - ZendFramework V1 TraceSearchConfigTest: drop one stray svc_src line the earlier V1_21-focused sed missed.
Earlier sed loop only matched CommonScenariosTest / TraceSearchConfigTest; these slipped through: - Symfony V3_4 AutofinishedTracesSymfony34Test / TemplateEnginesTest: drop svc_src='symfony' (SymfonyIntegration doesn't tag svc_src). - Frankenphp CommonScenariosTest: drop svc_src='frankenphp' (FrankenphpIntegration sets \$rootSpan->service directly without calling tagFrameworkServiceSource).
…-instance Same pattern as the earlier Predis/Mysqli fix: parameterize baseTags() to accept an optional svc_src override (defaulting to integration name) so testNoFakeServices can pass null when DD_SERVICE is set + flat services strip svc_src from spans. Files: Memcache, Memcached, MongoDB Latest, PDO, PHPRedis V5 Cluster, SQLSRV. PDO split-by-instance: 4 tests (testPDOSplitByDomain, testPDOServiceMappedSplitByDomain, testPDOStatementSplitByDomain, testPDOStatementSplitByDomainAndServiceFlattening) — svc_src='pdo' → 'opt.db_client_split_by_instance' via the new baseTags param. Guzzle V5 testAppendHostnameToServiceName / testAppendHostnameToServiceNameInlineCredentials: svc_src='guzzle' → 'opt.http_client_split_by_domain' (matches the V6/Latest fix that was already in).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the RFC "Service Override Source Attribution" for the PHP tracer.
Every span whose service was actively chosen (vs. the tracer's default) now carries a
_dd.svc_srcmeta tag identifying the source. Cleared for the default case (DD_SERVICE-set or auto-resolved).Sources emitted (per RFC categories)
<integration_name>Integration::handleInternalSpanServiceName(auto-covers Curl, Guzzle, PDO, Mysqli, SQLSRV, MongoDB, Memcached, AMQP, Kafka, …)opt.http_client_split_by_domainopt.db_client_split_by_instanceopt.redis_client_split_by_host<integration_name>(only whenDD_SERVICEis not user-set)Integration::tagFrameworkServiceSourcehelpermddtrace_serialize_span_to_rust_spanwhen span's service differs from root's and no other source tag was setDD_SERVICEor auto)