Add Laminas Integration #1990
Add Laminas Integration #1990
Conversation
Note: Not sure if this is the way this should be done.
| $this->integrations[LaminasIntegration::NAME] = | ||
| '\DDTrace\Integrations\Laminas\LaminasIntegration'; |
There was a problem hiding this comment.
This shall be removed (you have deferred loading already in integrations.c, that's enough)
There was a problem hiding this comment.
Can we explicit that with a comment in the code somewhere?
| $className = get_class($listener[0]); | ||
| $methodName = $listener[1]; |
There was a problem hiding this comment.
$listener is a callable. Probably all uses you've checked are [$class, $method]? But I think this should at least check whether it's actually an array.
(I suppose DDTrace\install_hook should be extended to generally accept any callable, also these written as ["class", "method"] ...)
| $this->integrations[LaminasIntegration::NAME] = | ||
| '\DDTrace\Integrations\Laminas\LaminasIntegration'; |
There was a problem hiding this comment.
Can we explicit that with a comment in the code somewhere?
|
|
||
| $rootSpan->meta[Tag::HTTP_METHOD] = $method; | ||
| $rootSpan->meta[Tag::HTTP_URL] = Normalizer::urlSanitize($request->getUriString()); | ||
| $rootSpan->meta['laminas.route.name'] = $routeName; |
There was a problem hiding this comment.
Check the span attributes tag doc I shared with you, I suppose those tags should be http. not laminas.
Description
Adds a Laminas integration. The goal was to capture the overall order of events and their corresponding listeners to understand the flow of execution better.
Since Laminas follows semantic versioning and does not carry a single framework version, the versions tested in this integration correspond to the skeleton application ones.
Readiness checklist
Reviewer checklist