New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Init the new `loader` extension #2684

Merged

iamluc merged 45 commits into master from luc/ssi-library-loader

Jun 26, 2024

Contributor

iamluc commented May 31, 2024

Description

This extension will be used to find and load the right ddtrace extension to support PHP Single Step Instrumentation

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

iamluc force-pushed the luc/ssi-library-loader branch 11 times, most recently from 2f4bdd9 to ccce898 Compare

May 31, 2024 14:46

bwoebi reviewed

View reviewed changes

.circleci/continue_config.yml Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

.circleci/continue_config.yml Outdated

+                        root: '.'
+                        paths: [ './loader/modules/dd_library_loader.so', './loader/dd-library-php' ]
+                "test_loader":

Collaborator

bwoebi May 31, 2024

I'd like to have on one PHP version run the test suite with opcache. Just to make sure the load ordering doesn't mess with the delicate callback replacing we do.

iamluc force-pushed the luc/ssi-library-loader branch 2 times, most recently from cbd3854 to b0276b6 Compare

June 3, 2024 09:54

bwoebi reviewed

View reviewed changes

loader/compat_php.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/php_dd_library_loader.h Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

.circleci/continue_config.yml Outdated

+                        matrix:
+                          parameters:
+                            php_major_minor:
+              #                - "5.6"

Collaborator

bwoebi Jun 6, 2024

We still need a test that, on 5.6, that the loader will disable itself. Without doing anything or emitting warnings. (Change the url to 1.0.0 to test without PHP 5 binaries included).

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

Collaborator

bwoebi commented Jun 6, 2024

Just adding a note so that we don't forget about it: if getenv("DD_TELEMETRY_FORWARDER_PATH") is set, then fork(), execve() to it with the telemetry payload telling why it wasn't loaded.

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

iamluc added 20 commits

June 21, 2024 10:56


          Fix some review's comments

38705bc


          Log error when registering module

fb1feed


          Make the code generic to load every Datadog extensions

228d34f


          Init telemetry support

63cf249


          Workaround PHP 7.2 segfault

0a1de34


          Cleanup .gitignore and config.w32

2dc8e19


          Improve telemetry

d5d5135


          Retrieve the runtime_version from the engine to work even with future…

e7b33be

… unknown versions


          Compile with -Wextra -Werror

3acfb1b


          Add tests

86ed5dd


          Merge loader pipeline back into build_packages

410269d


          Fix typo

e28d8a2


          Fix zend_error_cb handling

2fd53b8


          Fix CI

89144fb


          Set 'datadog.trace.sources_path' INI setting when ddtrace_injected is…

376f07c

… used


          Fix coding style (using clang-format)

f4b1399


          Don't use symbols that don't exist in PHP5 to avoid a missing symbol …

2cabbb1

…error


          Fix telemetry_forwarder arguments

a7c66b3


          Fix segfault on PHP 7.2 when calling zend_string_release

1f2d7c6


          Add test loading using PHP_INI_SCAN_DIR env var

dc05737

iamluc force-pushed the luc/ssi-library-loader branch from 3b584b1 to dc05737 Compare

June 21, 2024 09:18

bwoebi reviewed

View reviewed changes

loader/compat_php.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/compat_php.c Outdated Show resolved Hide resolved

bwoebi reviewed

View reviewed changes

loader/dd_library_loader.c Outdated Show resolved Hide resolved

iamluc added 2 commits

June 21, 2024 14:19


          Address Bob's comments

937c177


          Make DD_LOADER_PACKAGE_PATH env var mandatory

bbb9448

bwoebi approved these changes

View reviewed changes

Collaborator

bwoebi left a comment

Looks good by me now - nice work :-)

iamluc merged commit 72632b8 into master

657 of 659 checks passed

iamluc deleted the luc/ssi-library-loader branch

June 26, 2024 11:28

github-actions bot added this to the 1.2.0 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet