NH-146966 PHP: explore and adopt declarative config - #219
Open
jerrytfleung wants to merge 37 commits into
Open
jerrytfleung wants to merge 37 commits into
jerrytfleung wants to merge 37 commits into
Conversation
jerrytfleung
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical nullable-settings regressions, a breaking K8s constructor signature, and malformed example attribute keys remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds declarative OpenTelemetry configuration support for PHP components and updates PHPStan tooling.
Changes:
- Adds component providers for samplers, processors, detectors, and propagators.
- Adds YAML/PHP configuration examples and validation.
- Updates SDK initialization, defaults, compatibility, and PHPStan configuration.
File summaries
| File | Summary |
|---|---|
tests/Unit/Trace/Sampler/SamplerTest.php |
Updated sampler tests. |
tests/Unit/Resource/Detectors/K8sTest.php |
Updated Kubernetes detector tests. |
src/Trace/SwoSamplerFactory.php |
Uses centralized defaults. |
src/Trace/SpanProcessor/TransactionNameSpanProcessor.php |
Supports configured transaction names. |
src/Trace/SpanProcessor/ResponseTimeSpanProcessor.php |
Updates metrics integration. |
src/Trace/Sampler/Settings.php |
Merges sampler settings. |
src/Trace/Sampler/OboeSampler.php |
Updates sampler settings handling. |
src/Trace/Sampler/JsonSampler.php |
Uses centralized path defaults. |
src/Trace/Sampler/HttpSampler.php |
Updates metrics integration. |
src/SdkAutoloader.php |
Loads distribution configuration. |
src/Resource/Detectors/K8s.php |
Adds declarative Kubernetes settings. |
src/ComponentProvider/Validation/Validation.php |
Adds configuration validation. |
src/ComponentProvider/Trace/SpanProcessorTransactionName.php |
Provides transaction-name configuration. |
src/ComponentProvider/Trace/SpanProcessorResponseTime.php |
Provides response-time configuration. |
src/ComponentProvider/Trace/SamplerSolarwindsJson.php |
Provides JSON sampler configuration. |
src/ComponentProvider/Trace/SamplerSolarwindsHttp.php |
Provides HTTP sampler configuration. |
src/ComponentProvider/ResponsePropagator/ResponsePropagatorXTraceOptionsResponse.php |
Registers X-Trace options response propagation. |
src/ComponentProvider/ResponsePropagator/ResponsePropagatorXTrace.php |
Registers X-Trace response propagation. |
src/ComponentProvider/Propagator/PropagatorXTraceOptions.php |
Registers X-Trace options propagation. |
src/ComponentProvider/Propagator/PropagatorSwoTraceState.php |
Registers SWO trace-state propagation. |
src/ComponentProvider/Detector/DetectorUams.php |
Registers the UAMS detector. |
src/ComponentProvider/Detector/DetectorSwo.php |
Registers the SWO detector. |
src/ComponentProvider/Detector/DetectorK8s.php |
Registers Kubernetes detector configuration. |
src/Common/Configuration/KnownValues.php |
Centralizes configuration defaults. |
src/_autoload.php |
Avoids duplicate Composer initialization. |
phpstan.neon.dist |
Updates PHPStan settings. |
examples/basic_file.yaml |
Adds declarative configuration example. |
examples/basic_file.php |
Adds executable configuration example. |
composer.json |
Updates dependencies and provider registrations. |
Review details
Suppressed comments (1)
examples/basic_file.php:26
- The two attribute keys contain an extra trailing quote (
http.method"andhttp.route"), so the example emits non-standard attributes instead of the intended HTTP semantic-convention keys. Remove the stray quotes.
$child1->setAttribute('http.method"', 'GET');
$child1->setAttribute('http.route"', '/posts/:id');
- Files reviewed: 29/29 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| public function updateSettings(Settings $settings): void | ||
| { | ||
| if ($settings->timestamp > ($this->settings?->timestamp ?? 0)) { | ||
| if ($settings->timestamp > ($this->settings->timestamp ?? 0)) { |
| public static function merge(Settings $remote, LocalSettings $local): Settings | ||
| { | ||
| $flags = $local->getTracingMode()?->value ?? $remote->flags; | ||
| $flags = $local->getTracingMode()->value ?? $remote->flags; |
Comment on lines
30
to
36
| public function __construct( | ||
| ?string $namespace = null, | ||
| ?string $pod_name = null, | ||
| ?string $pod_uid = null, | ||
| ?string $namespaceFile = null, | ||
| ?string $mountInfoFile = null, | ||
| ) { |
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
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.
Description of changes:
Related issues
NH-146966