[php]: add endpoints to support OpenTelemetry logs#6646
Open
rachelyangdog wants to merge 11 commits into
Open
Conversation
Contributor
|
|
…ridge - server.php: replace manual LogsExporterFactory + BatchLogRecordProcessor + SDKLoggerProvider wiring with Globals::loggerProvider(), aligning with the Node.js and Python parametric servers. Drop the hardcoded putenv that forced OTEL_EXPORTER_OTLP_ENDPOINT to port 4318 and prevented the tracer's DatadogResolver from picking the correct port per protocol. - /log/otel/flush: wrap forceFlush in try/catch and return get_class on the provider, matching the Python/Node.js shape. - Dockerfile: set DD_AUTOLOAD_NO_COMPILE=true so ddtrace loads the per-file bridge (src/bridge/_files_*.php) rather than the stale bundled src/bridge/_generated_*.php from the released package, allowing local PR source overrides in /binaries/src to take effect. - Remove GrpcTransport.php: orphaned shim, never registered with the OTel Registry and required PECL ext-grpc which isn't installed in the image. gRPC support belongs as an opt-in dep on the user side (parallels dd-trace-py's opentelemetry-exporter-otlp[grpc]). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
✨ Fix all issues with BitsAI or with Cursor
|
Mirrors the metrics-style pattern: dd-trace-php's DatadogResolver supplies agent-aware defaults (OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, etc.) and the parametric server, standing in for user code, builds the LoggerProvider itself with LogsExporterFactory + BatchLogRecordProcessor + ResourceInfoFactory::defaultResource. Avoids relying on an OTEL_PHP_AUTOLOAD_ENABLED override in the tracer, which would also stand up unrequested Tracer/Meter providers and OTel auto-instrumentations. Wraps construction in try/catch so the server keeps responding when the configured protocol's transport isn't available (e.g. grpc without ext-grpc + open-telemetry/transport-grpc), letting that test fail cleanly on missing payloads instead of erroring at server startup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Test_FR06_OTLP_Protocols: gRPC requires user-installed open-telemetry/transport-grpc + ext-grpc; dd-trace-php does not bundle either (parallels dd-trace-py's opt-in posture for grpc). HTTP/protobuf is exercised via FR01/FR05/FR08, so no real coverage loss. - Test_FR10_Timeout_Configuration / Test_FR11_Telemetry: OTel config telemetry tracking (otel.log_records count metric, OTEL_EXPORTER_OTLP_* configuration reporting) is out of scope for the initial logs PR; will follow up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manifest validator requires sorted keys. Test_FR06 now precedes Test_FR07. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rachelyangdog
changed the title
bwoebi
reviewed
May 11, 2026
bwoebi
reviewed
May 11, 2026
| // DatadogResolver fills in OTEL_EXPORTER_OTLP_LOGS_ENDPOINT from the agent host | ||
| // and the resource detector hooks (Service / Environment / Host) populate the | ||
| // ResourceInfo with DD_SERVICE / DD_ENV / DD_VERSION / DD_HOSTNAME. | ||
| $sdkLoggerProvider = SDKLoggerProvider::builder()->build(); |
Contributor
There was a problem hiding this comment.
I would actually add open-telemetry/opentelemetry-logger-monolog as a composer dependency (and use monolog then) - so that the test actually tests what it's intended to test: that the log injection doesn't mess with this.
bwoebi
reviewed
May 11, 2026
Comment on lines
+565
to
+572
| $severityMap = [ | ||
| 'TRACE' => ['number' => 1, 'text' => 'TRACE'], | ||
| 'DEBUG' => ['number' => 5, 'text' => 'DEBUG'], | ||
| 'INFO' => ['number' => 9, 'text' => 'INFO'], | ||
| 'WARN' => ['number' => 13, 'text' => 'WARN'], | ||
| 'ERROR' => ['number' => 17, 'text' => 'ERROR'], | ||
| 'FATAL' => ['number' => 21, 'text' => 'FATAL'], | ||
| ]; |
Contributor
There was a problem hiding this comment.
why map to itself, is ['TRACE' => 1, ...] not good enough?
Contributor
Author
There was a problem hiding this comment.
That map was in the pre-Monolog version of /otel/logger/write and got removed in the commit e3ab113. The current severity map lives inside the custom handler at server.php:580 and uses Monolog level_name keys, where the text differs from the key on 5 of 8 rows (NOTICE→INFO, WARNING→WARN, CRITICAL→FATAL, ALERT→FATAL, EMERGENCY→FATAL). The 3 self-mapping rows (DEBUG/INFO/ERROR) are kept inline for symmetry.
bwoebi
reviewed
May 11, 2026
Comment on lines
+434
to
+436
| if [ "${library}" = "php" ] && [ "${pytest_numprocesses}" = "auto" ]; then | ||
| pytest_numprocesses=4 | ||
| fi |
Contributor
Author
There was a problem hiding this comment.
This was added because PHP parametric workers each launch a dd-apm-test-agent + php-test-client container pair so i capped the concurrency to keep the docker daemon responsive.
Per Bob's review feedback: the FR09 log-injection test was passing
trivially before because the parametric server emitted OTel LogRecords
directly via the SDK Logger — so dd-trace-php's LogsIntegration PSR-3
hook (the real log-injection path) never fired, and the
shouldSkipForOtelLogs guard was never actually validated.
This change adds open-telemetry/opentelemetry-logger-monolog to the
parametric server's composer and routes /otel/logger/{create,write}
through a Monolog\Logger with an OTel Handler attached, so the PSR-3
hook in LogsIntegration runs and the integration's OTel-routed-Monolog
detection is genuinely exercised.
Two small wrinkles in the OTel Monolog handler that needed handling:
- The handler calls $provider->getLogger($monologChannel) with no scope
params, so FR13 (version / schema_url / attributes) would lose what
create_logger sent. Wrap the SDK provider in an anonymous class that
stores the create_logger scope params and re-injects them when the
handler asks for a Logger.
- The handler writes $record->level_name through verbatim as
severity_text, so Monolog's "WARNING"/"CRITICAL" surface instead of
OTel-canonical "WARN"/"FATAL" (FR12 [warning] failure).
Monolog\LogRecord::with() doesn't allow overriding level_name (it's
derived from the Level enum, not a stored property), so subclass the
handler and re-implement write() with our own severity map. The
subclass keeps `instanceof OpenTelemetry\Contrib\Logs\Monolog\Handler`
true so LogsIntegration's detection still matches.
Full parametric suite still: 24 passed, 5 xfailed (out of scope: grpc +
telemetry), 1 xpassed (FR06 http_protobuf — collateral of the
class-level missing_feature gate), 1 skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes two changes that only helped local PR-iteration and shouldn't ship in the parametric image: - ENV DD_AUTOLOAD_NO_COMPILE=true (made ddtrace load src/bridge/_files_*.php instead of the bundled _generated_*.php, so /binaries/src overrides could take effect without regenerating the bundle). - The cp /binaries/src/. -> dd-trace-sources/src/ hunk (the override mechanism itself). Local developers iterating on dd-trace-php PHP-level sources should now either regenerate _generated_*.php locally (tooling/generation, composer generate) and rsync that with the rest of src/, or ship a full tarball. The parametric image is back to production-faithful: it runs the released package's bundle, not a developer's loose source files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cbeauchesne
requested changes
May 15, 2026
Comment on lines
+434
to
+436
| if [ "${library}" = "php" ] && [ "${pytest_numprocesses}" = "auto" ]; then | ||
| pytest_numprocesses=4 | ||
| fi |
Collaborator
There was a problem hiding this comment.
I'm surprised to see this, what is the link with the PR ?
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
3 participants
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.
Motivation
Changes
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present