Add a TYPO3 CMS integration #1538

LiquidPL · 2022-03-09T16:26:26Z

Description

Resolves #1074.

This PR adds an integration for the TYPO3 CMS and its headless extension. It covers tracing for most common core framework methods, including cache, database, and templating.

Readiness checklist

(only for Members) Changelog has been added to the release document.
Tests added for this feature/bug.

Reviewer checklist

Appropriate labels assigned.
Milestone is set.
Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

LiquidPL · 2022-03-09T16:32:57Z

One thing I'd like to ask about: is there a way to locally build a debian package, without going through CircleCI? I'd like to test the built extension against the copy of this integration we have internally, but I couldn't manage to build that package.

* remove tracing for apc and wincache caching backends, which are deprecated * remove tracing for null/transient memory cache backends, which are very fast and negligible for tracing purposes

labbati

Thank you @LiquidPL for the work on this PR, I left some comments specific to the lines, plus I have to questions here.

Can we add some tests like we do for other frameworks? You can use Laravel's test suite as a reference https://github.com/DataDog/dd-trace-php/blob/master/tests/Integrations/Laravel/V8_x/CommonScenariosTest.php), no need to replicate all tests, only the ones in the file I linked to.

Can you remove tracing of methods that are expected to have a very high cardinality, if any. Just to give you an example, a method that is expected to be called hundreds of times, like a DTO hydrator or similar) should not be traced because of the added overhead.

ext/php5/integrations/integrations.h

src/Integrations/Integrations/IntegrationsLoader.php

src/Integrations/Integrations/TYPO3/TYPO3Integration.php

labbati · 2022-04-05T08:46:22Z

src/Integrations/Integrations/TYPO3/TYPO3Integration.php

+ \DDTrace\trace_method(\TYPO3\CMS\Extbase\Mvc\Dispatcher::class, 'dispatch', $setCommonValues);
+
+ \DDTrace\trace_method(\TYPO3\CMS\Extbase\Persistence\Generic\Query::class, 'execute', $setCommonValues);
+ \DDTrace\trace_method(\TYPO3\CMS\Extbase\Property\PropertyMapper::class, 'convert', $setCommonValues);


What is the expected cardinality of PropertyMapper::convert, I am not familiar with typo3 but it looks like this method might be potentially called hundreds of times? Int his case we should not trace it and trace maybe one layer above.

This one is called when processing properties passed in the request, shouldn't be called a lot, depending on the amount of properties passed.

labbati · 2022-04-05T08:47:00Z

src/Integrations/Integrations/TYPO3/TYPO3Integration.php

+
+ \DDTrace\trace_method(
+ \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::class,
+ 'acquireLock',


What is the expected cardinality of TypoScriptFrontendController:: acquireLock , I am not familiar with typo3 but it looks like this method might be potentially called hundreds of times? Int his case we should not trace it and trace maybe one layer above.

This is mainly used for caching, and should generally only be used whenever a new page is saved into the cache, and is not called when a page is pulled from the cache. Optimistically it should only be called at most once per request (potentially more if a request fails and subsequently retries to render a page).

src/Integrations/Integrations/TYPO3/TYPO3Integration.php

labbati · 2022-05-30T08:53:25Z

Hello @LiquidPL , I was just curious to know if you had a chance to review my feedback.

LiquidPL · 2022-05-30T08:55:40Z

Yeah, I'm working my way through it. I should have fixes ready in 1-2 days.

bwoebi · 2022-06-28T11:56:25Z

Hey @LiquidPL

Has there been any progress on this? :-) Given that it's still a draft?

LiquidPL · 2022-06-28T12:56:35Z

I've cleaned up most of the C-side code as expected, currently I'm working on writing TYPO3 tests for the tracing, however it's going slower than I expected, since this I need to get the framework configured to run properly in the tests.

bwoebi · 2022-08-29T10:51:00Z

Hey @LiquidPL,

May I ask you for another update on this? :-)

LiquidPL · 2022-10-24T11:15:21Z

Hello,

Apologies for the long delays, I have provided tests for both TYPO3 v10, and the recently released TYPO3 v11.

There's two things to consider:

the testing Docker images need to be updated to use Composer 2 (I don't know if this is already done for the CI images, but at least locally the v11 tests failed because of that).

circleci(buster):~/app$ make test_web_typo3_11x 
COMPOSER_MEMORY_LIMIT=-1 composer --no-interaction --working-dir=./tests update
Loading composer repositories with package information
Warning from https://repo.packagist.org: Support for Composer 1 is deprecated and some packages will not be available. You should upgrade to Composer 2. See https://blog.packagist.com/deprecating-composer-1-support/
Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies (including require-dev)
Nothing to install or update
Generating autoload files
27 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Create scenario 'elasticsearch1'.
Create scenario 'elasticsearch7'.
Create scenario 'guzzle5'.
Create scenario 'guzzle6'.
Create scenario 'mongodb1'.
Create scenario 'opentracing_beta5'.
Create scenario 'opentracing_beta6'.
Create scenario 'opentracing10'.
Create scenario 'predis1'.
Create scenario 'phpredis3'.
Create scenario 'phpredis4'.
Create scenario 'phpredis5'.
COMPOSER_MEMORY_LIMIT=-1 composer --no-interaction --working-dir=tests/Frameworks/TYPO3/Version_11_x update
Loading composer repositories with package information
Warning from https://repo.packagist.org: Support for Composer 1 is deprecated and some packages will not be available. You should upgrade to Composer 2. See https://blog.packagist.com/deprecating-composer-1-support/
Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - typo3/cms-core v11.5.17 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.9 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.8 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.7 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.6 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.5 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.4 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.3 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.2 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.17 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.16 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.15 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.14 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.13 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.12 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.11 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.10 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.1 requires composer-runtime-api ^2.1 -> no matching package found.
    - typo3/cms-core v11.5.0 requires composer-runtime-api ^2.1 -> no matching package found.
    - Installation request for typo3/cms-core ^11.5.0 -> satisfiable by typo3/cms-core[v11.5.0, v11.5.1, v11.5.10, v11.5.11, v11.5.12, v11.5.13, v11.5.14, v11.5.15, v11.5.16, v11.5.17, v11.5.2, v11.5.3, v11.5.4, v11.5.5, v11.5.6, v11.5.7, v11.5.8, v11.5.9].

Potential causes:
 - This package is not installable via Composer 1.x, see <https://blog.packagist.com/deprecating-composer-1-support/>
 - A typo in the package name
 - The package is not available in a stable-enough version according to your minimum-stability setting
   see <https://getcomposer.org/doc/04-schema.md#minimum-stability> for more details.
 - It's a private package and you forgot to add a custom repository to find it

Read <https://getcomposer.org/doc/articles/troubleshooting.md> for further common problems.
make: *** [Makefile:961: test_web_typo3_11x] Error 2
circleci(buster):~/app$

there seems to be a problem with the SpanChecker, where some of the spans traced from TYPO3 are so short they get reported as having 0 duration, causing it to fail.

circleci(buster):~/app$ make test_web_typo3_11x 
COMPOSER_MEMORY_LIMIT=-1 composer --no-interaction --working-dir=./tests update
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Generating autoload files
27 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Create scenario 'elasticsearch1'.
Create scenario 'elasticsearch7'.
Create scenario 'guzzle5'.
Create scenario 'guzzle6'.
Create scenario 'mongodb1'.
Create scenario 'opentracing_beta5'.
Create scenario 'opentracing_beta6'.
Create scenario 'opentracing10'.
Create scenario 'predis1'.
Create scenario 'phpredis3'.
Create scenario 'phpredis4'.
Create scenario 'phpredis5'.
No security vulnerability advisories found
COMPOSER_MEMORY_LIMIT=-1 composer --no-interaction --working-dir=tests/Frameworks/TYPO3/Version_11_x update
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Generating autoload files
> typo3cms install:fixfolderstructure
No action performed!
46 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
No security vulnerability advisories found
php tests/Frameworks/TYPO3/Version_11_x/vendor/bin/typo3cms cache:flush --group=system
php tests/Frameworks/TYPO3/Version_11_x/vendor/bin/typo3cms cache:flush --group=di
DD_AUTOLOAD_NO_COMPILE=true DD_TRACE_CLI_ENABLED=true php  -d ddtrace.request_init_hook=/home/circleci/app/bridge/dd_wrap_autoloader.php ./tests/vendor/bin/phpunit  --config=./tests/phpunit.xml tests/Integrations/TYPO3/V11_x --filter=.
PHPUnit 9.5.25 #StandWithUkraine

Runtime:       PHP 8.0.17
Configuration: ./tests/phpunit.xml
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

F.F.                                                                                                                                                                                            4 / 4 (100%)

Time: 00:43.475, Memory: 14.00 MB

There were 2 failures:

1) DDTrace\Tests\Integration\TYPO3\V11_x\CommonScenariosTest::testScenario with data set "A simple GET request returning a string" (DDTrace\Tests\Frameworks\Util\Request\GetSpec Object (...), array(DDTrace\Tests\Common\SpanAssertion Object (...)))
Failed asserting that 0 is greater than 0.
parent operation/resource: TYPO3\CMS\Core\TypoScript\TemplateService.generateConfig/
parent operation/resource: TYPO3\CMS\Core\TypoScript\TemplateService.start/
parent operation/resource: TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController.getConfigArray/
parent operation/resource: typo3.Application.run/
parent operation/resource: web.request/GET /simple

/home/circleci/app/tests/Common/SpanChecker.php:343
/home/circleci/app/tests/Common/SpanChecker.php:56
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:45
/home/circleci/app/tests/Common/SpanAssertionTrait.php:29
/home/circleci/app/tests/Integrations/TYPO3/V11_x/CommonScenariosTest.php:42

2) DDTrace\Tests\Integration\TYPO3\V11_x\CommonScenariosTest::testScenario with data set "A GET request with an exception" (DDTrace\Tests\Frameworks\Util\Request\GetSpec Object (...), array(DDTrace\Tests\Common\SpanAssertion Object (...)))
Failed asserting that 0 is greater than 0.
parent operation/resource: TYPO3\CMS\Core\TypoScript\TemplateService.generateConfig/
parent operation/resource: TYPO3\CMS\Core\TypoScript\TemplateService.start/
parent operation/resource: TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController.getConfigArray/
parent operation/resource: typo3.Application.run/
parent operation/resource: web.request/GET /error

/home/circleci/app/tests/Common/SpanChecker.php:343
/home/circleci/app/tests/Common/SpanChecker.php:56
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:89
/home/circleci/app/tests/Common/SpanChecker.php:45
/home/circleci/app/tests/Common/SpanAssertionTrait.php:29
/home/circleci/app/tests/Integrations/TYPO3/V11_x/CommonScenariosTest.php:42

FAILURES!
Tests: 4, Assertions: 6071, Failures: 2.
make: *** [Makefile:964: test_web_typo3_11x] Error 1
circleci(buster):~/app$

LiquidPL · 2023-01-13T13:13:12Z

Hello, can we take another look at this PR?

tmotyl · 2023-06-29T08:18:15Z

@labbati @bwoebi can you please take a look?

Add a TYPO3 CMS integration

0ac2d71

krakjoe added the 🎉 new-integration A new integration label Mar 10, 2022

Remove tracing for deprecated/unnecessary methods

bddaa70

* remove tracing for apc and wincache caching backends, which are deprecated * remove tracing for null/transient memory cache backends, which are very fast and negligible for tracing purposes

labbati requested changes Apr 5, 2022

View reviewed changes

LiquidPL added 2 commits May 31, 2022 23:22

Load integration only for PHP 7 and above

323866b

Remove unwanted call

615a00f

LiquidPL changed the title ~~Add a TYPO3 CMS integration~~ [WIP] Add a TYPO3 CMS integration May 31, 2022

LiquidPL changed the title ~~[WIP] Add a TYPO3 CMS integration~~ Add a TYPO3 CMS integration May 31, 2022

LiquidPL marked this pull request as draft May 31, 2022 22:48

LiquidPL added 2 commits October 24, 2022 03:22

Merge branch 'master' into typo3

92cf28c

Add tests for the TYPO3 integration

cd5b170

LiquidPL marked this pull request as ready for review October 24, 2022 11:15

LiquidPL requested a review from a team as a code owner October 24, 2022 11:15

Fix linting issues

05db438

LiquidPL requested a review from labbati April 5, 2023 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a TYPO3 CMS integration #1538

Add a TYPO3 CMS integration #1538

LiquidPL commented Mar 9, 2022

LiquidPL commented Mar 9, 2022

labbati left a comment

labbati Apr 5, 2022

LiquidPL Oct 25, 2022

labbati Apr 5, 2022

LiquidPL Oct 25, 2022

labbati commented May 30, 2022

LiquidPL commented May 30, 2022

bwoebi commented Jun 28, 2022

LiquidPL commented Jun 28, 2022

bwoebi commented Aug 29, 2022

LiquidPL commented Oct 24, 2022

LiquidPL commented Jan 13, 2023

tmotyl commented Jun 29, 2023

Add a TYPO3 CMS integration #1538

Are you sure you want to change the base?

Add a TYPO3 CMS integration #1538

Conversation

LiquidPL commented Mar 9, 2022

Description

Readiness checklist

Reviewer checklist

LiquidPL commented Mar 9, 2022

labbati left a comment

Choose a reason for hiding this comment

labbati Apr 5, 2022

Choose a reason for hiding this comment

LiquidPL Oct 25, 2022

Choose a reason for hiding this comment

labbati Apr 5, 2022

Choose a reason for hiding this comment

LiquidPL Oct 25, 2022

Choose a reason for hiding this comment

labbati commented May 30, 2022

LiquidPL commented May 30, 2022

bwoebi commented Jun 28, 2022

LiquidPL commented Jun 28, 2022

bwoebi commented Aug 29, 2022

LiquidPL commented Oct 24, 2022

LiquidPL commented Jan 13, 2023

tmotyl commented Jun 29, 2023