-
Notifications
You must be signed in to change notification settings - Fork 462
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.
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
[chore] Build docker image for PHP auto-intrumentation #3409
Open
SergeyKleyman
wants to merge
19
commits into
open-telemetry:main
Choose a base branch
from
SergeyKleyman:Docker_image_for_PHP
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
7554b61
Build docker image for PHP autointrumentation
SergeyKleyman fcf6a25
Fixed parts were commented out during debugging
SergeyKleyman 7d8e7ad
Fixed path to relative since it's used in Dockerfile
SergeyKleyman a96d715
Marked autoinstrumentation/php/prepare_files_for_docker_image.sh as e…
SergeyKleyman fd6f9bb
Fixed error message in autoinstrumentation/php/prepare_files_for_dock…
SergeyKleyman 65101bf
Build docker image for PHP autointrumentation
SergeyKleyman b367df6
Fixed parts were commented out during debugging
SergeyKleyman 2a85196
Fixed path to relative since it's used in Dockerfile
SergeyKleyman 9d803b3
Marked autoinstrumentation/php/prepare_files_for_docker_image.sh as e…
SergeyKleyman 7c47b95
Fixed error message in autoinstrumentation/php/prepare_files_for_dock…
SergeyKleyman aa9f1c2
Merge remote-tracking branch 'origin/Docker_image_for_PHP' into Docke…
SergeyKleyman 25abe1f
Merge branch 'main' into Docker_image_for_PHP
SergeyKleyman a235b0f
Added missing autoinstrumentation/php/version.txt
SergeyKleyman 37ddb18
Removed support for PHP 8.0
SergeyKleyman 364c7b9
Updated versions of instrumentation packages to the latest
SergeyKleyman cfbbc25
Fixed publish-autoinstrumentation-php.yaml Prepare files for docker i…
SergeyKleyman 3df842b
Deleted composer_PHP_8.0.json that was used only for PHP 8.0
SergeyKleyman ae5600b
Merge branch 'main' into Docker_image_for_PHP
SergeyKleyman 4b11a19
Added PHP to /versions.txt
SergeyKleyman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
name: "Publish PHP Auto-Instrumentation" | ||
|
||
on: | ||
push: | ||
paths: | ||
- 'autoinstrumentation/php/**' | ||
- '.github/workflows/publish-autoinstrumentation-php.yaml' | ||
branches: | ||
- main | ||
pull_request: | ||
paths: | ||
- 'autoinstrumentation/php/**' | ||
- '.github/workflows/publish-autoinstrumentation-php.yaml' | ||
workflow_dispatch: | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
publish: | ||
runs-on: ubuntu-22.04 | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- name: Read version | ||
run: echo "VERSION=$(cat autoinstrumentation/php/version.txt)" >> $GITHUB_ENV | ||
|
||
- name: Docker meta | ||
id: meta | ||
uses: docker/metadata-action@v5 | ||
with: | ||
images: | | ||
otel/autoinstrumentation-php | ||
ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-php | ||
tags: | | ||
type=match,pattern=v(.*),group=1,value=v${{ env.VERSION }} | ||
|
||
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v3 | ||
|
||
- name: Cache Docker layers | ||
uses: actions/cache@v4 | ||
with: | ||
path: /tmp/.buildx-cache | ||
key: ${{ runner.os }}-buildx-${{ github.sha }} | ||
restore-keys: | | ||
${{ runner.os }}-buildx- | ||
|
||
- name: Log into Docker.io | ||
uses: docker/login-action@v3 | ||
if: ${{ github.event_name == 'push' }} | ||
with: | ||
username: ${{ secrets.DOCKER_USERNAME }} | ||
password: ${{ secrets.DOCKER_PASSWORD }} | ||
|
||
- name: Login to GitHub Package Registry | ||
uses: docker/login-action@v3 | ||
if: ${{ github.event_name == 'push' }} | ||
with: | ||
registry: ghcr.io | ||
username: ${{ github.repository_owner }} | ||
password: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Prepare files for docker image | ||
run: ./autoinstrumentation/php/prepare_files_for_docker_image.sh --ext-ver ${{ env.VERSION }} --dest-dir ${PWD}/autoinstrumentation/php/files_for_docker_image | ||
|
||
- name: Build and push | ||
uses: docker/build-push-action@v6 | ||
with: | ||
context: autoinstrumentation/php | ||
push: ${{ github.event_name == 'push' }} | ||
build-args: SUB_DIR_WITH_FILES_FOR_DOCKER_IMAGE=${PWD}/autoinstrumentation/php/files_for_docker_image | ||
tags: ${{ steps.meta.outputs.tags }} | ||
labels: ${{ steps.meta.outputs.labels }} | ||
cache-from: type=local,src=/tmp/.buildx-cache | ||
cache-to: type=local,dest=/tmp/.buildx-cache |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# To build one auto-instrumentation image for php, please: | ||
# - Download your php auto-instrumentation artefacts to `/autoinstrumentation` directory. This is required as when instrumenting the pod, | ||
# one init container will be created to copy the files to your app's container. | ||
# - Grant the necessary access to the files in the `/autoinstrumentation` directory. | ||
# - Following environment variables are injected to the application container to enable the auto-instrumentation. | ||
# PHP_INI_SCAN_DIR=:/otel-auto-instrumentation-php/php_ini_scan_dir | ||
# OTEL_PHP_AUTOLOAD_ENABLED=true | ||
# - For auto-instrumentation by container injection, the Linux command cp is | ||
# used and must be availabe in the image. | ||
|
||
FROM busybox | ||
|
||
ARG SUB_DIR_WITH_FILES_FOR_DOCKER_IMAGE | ||
|
||
COPY ${SUB_DIR_WITH_FILES_FOR_DOCKER_IMAGE} /autoinstrumentation/ |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
{ | ||
"name": "open-telemetry/operator-autoinstrumentation-php", | ||
"description": "OpenTelemetry PHP auto-instrumentation packages to include in the image used by OpenTelemetry Operator for Kubernetes", | ||
"type": "project", | ||
"require": { | ||
"open-telemetry/exporter-otlp": "1.0.4", | ||
"open-telemetry/opentelemetry-auto-guzzle": "1.0.1", | ||
"open-telemetry/opentelemetry-auto-http-async": "1.0.1", | ||
"open-telemetry/opentelemetry-auto-laravel": "1.0.0", | ||
"open-telemetry/opentelemetry-auto-psr15": "1.0.6", | ||
"open-telemetry/opentelemetry-auto-psr18": "1.0.4", | ||
"open-telemetry/opentelemetry-auto-slim": "1.0.7", | ||
"open-telemetry/opentelemetry-auto-symfony": "1.0.0beta30", | ||
"open-telemetry/opentelemetry-auto-wordpress": "0.0.16", | ||
"open-telemetry/sdk": "1.0.8", | ||
"php-http/guzzle7-adapter": "1.0.0" | ||
}, | ||
"provide": { | ||
"psr/http-client": "*", | ||
"psr/http-server-middleware": "*", | ||
"laravel/framework": "*", | ||
"slim/slim": "*", | ||
"symfony/http-client-contracts": "*", | ||
"symfony/http-kernel": "*" | ||
}, | ||
"config": { | ||
"process-timeout": 0, | ||
"sort-packages": true, | ||
"allow-plugins": { | ||
"dealerdirect/phpcodesniffer-composer-installer": true, | ||
"php-http/discovery": true | ||
} | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
{ | ||
"name": "open-telemetry/operator-autoinstrumentation-php", | ||
"description": "OpenTelemetry PHP auto-instrumentation packages to include in the image used by OpenTelemetry Operator for Kubernetes", | ||
"type": "project", | ||
"require": { | ||
"open-telemetry/exporter-otlp": "1.1.0", | ||
"open-telemetry/opentelemetry-auto-guzzle": "1.0.1", | ||
"open-telemetry/opentelemetry-auto-http-async": "1.0.1", | ||
"open-telemetry/opentelemetry-auto-laravel": "1.0.0", | ||
"open-telemetry/opentelemetry-auto-psr15": "1.0.6", | ||
"open-telemetry/opentelemetry-auto-psr18": "1.0.4", | ||
"open-telemetry/opentelemetry-auto-slim": "1.0.7", | ||
"open-telemetry/opentelemetry-auto-symfony": "1.0.0beta30", | ||
"open-telemetry/opentelemetry-auto-wordpress": "0.0.16", | ||
"open-telemetry/sdk": "1.1.2", | ||
"php-http/guzzle7-adapter": "1.0.0" | ||
}, | ||
"provide": { | ||
"psr/http-client": "*", | ||
"psr/http-server-middleware": "*", | ||
"laravel/framework": "*", | ||
"slim/slim": "*", | ||
"symfony/http-client-contracts": "*", | ||
"symfony/http-kernel": "*" | ||
}, | ||
"config": { | ||
"process-timeout": 0, | ||
"sort-packages": true, | ||
"allow-plugins": { | ||
"dealerdirect/phpcodesniffer-composer-installer": true, | ||
"php-http/discovery": true | ||
} | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"name": "open-telemetry/operator-autoinstrumentation-php", | ||
"description": "OpenTelemetry PHP auto-instrumentation packages to include in the image used by OpenTelemetry Operator for Kubernetes", | ||
"type": "project", | ||
"require": { | ||
"open-telemetry/exporter-otlp": "1.1.0", | ||
"open-telemetry/opentelemetry-auto-guzzle": "1.0.1", | ||
"open-telemetry/opentelemetry-auto-http-async": "1.0.1", | ||
"open-telemetry/opentelemetry-auto-laravel": "1.0.0", | ||
"open-telemetry/opentelemetry-auto-pdo": "0.0.16", | ||
"open-telemetry/opentelemetry-auto-psr15": "1.0.6", | ||
"open-telemetry/opentelemetry-auto-psr18": "1.0.4", | ||
"open-telemetry/opentelemetry-auto-slim": "1.0.7", | ||
"open-telemetry/opentelemetry-auto-symfony": "1.0.0beta30", | ||
"open-telemetry/opentelemetry-auto-wordpress": "0.0.16", | ||
"open-telemetry/sdk": "1.1.2", | ||
"php-http/guzzle7-adapter": "1.0.0" | ||
}, | ||
"provide": { | ||
"psr/http-client": "*", | ||
"psr/http-server-middleware": "*", | ||
"laravel/framework": "*", | ||
"slim/slim": "*", | ||
"symfony/http-client-contracts": "*", | ||
"symfony/http-kernel": "*" | ||
}, | ||
"config": { | ||
"process-timeout": 0, | ||
"sort-packages": true, | ||
"allow-plugins": { | ||
"dealerdirect/phpcodesniffer-composer-installer": true, | ||
"php-http/discovery": true | ||
} | ||
} | ||
} |
191 changes: 191 additions & 0 deletions
191
autoinstrumentation/php/prepare_files_for_docker_image.sh
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,191 @@ | ||
#!/usr/bin/env bash | ||
set -e | ||
|
||
PHP_versions=(8.0) | ||
#PHP_versions=(8.0 8.1 8.2 8.3) | ||
#libc_variants=(musl) | ||
libc_variants=(glibc musl) | ||
|
||
show_help() { | ||
echo "Usage: $0 --ext-ver <opentelemetry extension version> --dest-dir <destination directory>" | ||
echo | ||
echo "Arguments:" | ||
echo " <opentelemetry extension version> - opentelemetry PHP extension version to use. This argument is mandatory." | ||
echo " <destination directory> - Directory to store files for docker image. All existing files in this directory will be deleted. This argument is mandatory." | ||
echo | ||
echo "Example:" | ||
echo " $0 ./files_for_docker_image" | ||
} | ||
|
||
parse_args() { | ||
while [[ "$#" -gt 0 ]]; do | ||
case $1 in | ||
--ext-ver) | ||
opentelemetry_extension_version="$2" | ||
shift | ||
;; | ||
--dest-dir) | ||
destination_directory="$2" | ||
shift | ||
;; | ||
--help) | ||
show_help | ||
exit 0 | ||
;; | ||
*) | ||
echo "Unknown parameter passed: $1" | ||
show_help | ||
exit 1 | ||
;; | ||
esac | ||
shift | ||
done | ||
|
||
if [ -z "${opentelemetry_extension_version}" ] ; then | ||
echo "<opentelemetry extension version> argument is missing" | ||
show_help | ||
exit 1 | ||
fi | ||
if [ -z "${destination_directory}" ] ; then | ||
echo "<destination directory> argument is missing" | ||
show_help | ||
exit 1 | ||
fi | ||
} | ||
|
||
ensure_dir_exists_and_empty() { | ||
local dir_to_clean="${1:?}" | ||
|
||
if [ -d "${dir_to_clean}" ]; then | ||
rm -rf "${dir_to_clean}" | ||
if [ -d "${dir_to_clean}" ]; then | ||
echo "Directory ${dir_to_clean} still exists. Directory content:" | ||
ls -l "${dir_to_clean}" | ||
exit 1 | ||
fi | ||
else | ||
mkdir -p "${dir_to_clean}" | ||
fi | ||
} | ||
|
||
build_native_binaries_for_PHP_version_libc_variant() { | ||
local PHP_version="${1:?}" | ||
local libc_variant="${2:?}" | ||
local dest_dir_for_current_args | ||
dest_dir_for_current_args="${destination_directory}/native_binaries/PHP_${PHP_version}_${libc_variant}" | ||
|
||
echo "Building extension binaries for PHP version: ${PHP_version} and libc variant: ${libc_variant} to ${dest_dir_for_current_args} ..." | ||
|
||
ensure_dir_exists_and_empty "${dest_dir_for_current_args}" | ||
|
||
local PHP_docker_image="php:${PHP_version}-cli" | ||
local install_compiler_command="" | ||
case "${libc_variant}" in | ||
glibc) | ||
;; | ||
musl) | ||
PHP_docker_image="${PHP_docker_image}-alpine" | ||
install_compiler_command="&& apk update && apk add autoconf build-base" | ||
;; | ||
*) | ||
echo "Unexpected PHP version: ${PHP_version}" | ||
show_help | ||
exit 1 | ||
;; | ||
esac | ||
|
||
local current_user_id | ||
current_user_id="$(id -u)" | ||
local current_user_group_id | ||
current_user_group_id="$(id -g)" | ||
docker run --rm \ | ||
-v "${dest_dir_for_current_args}:/dest_dir" \ | ||
${PHP_docker_image} sh -c "\ | ||
mkdir -p /app && cd /app \ | ||
${install_compiler_command} \ | ||
&& pecl install opentelemetry-${opentelemetry_extension_version} \ | ||
&& cp /usr/local/lib/php/extensions/no-debug-non-zts-*/opentelemetry.so /dest_dir/ \ | ||
&& chown -R ${current_user_id}:${current_user_group_id} /dest_dir/" | ||
|
||
echo "Built extension binaries for PHP version: ${PHP_version} and libc variant: ${libc_variant}" | ||
} | ||
|
||
build_native_binaries() { | ||
echo "Building extension binaries..." | ||
|
||
for PHP_version in "${PHP_versions[@]}" ; do | ||
for libc_variant in "${libc_variants[@]}" ; do | ||
build_native_binaries_for_PHP_version_libc_variant "${PHP_version}" "${libc_variant}" | ||
done | ||
done | ||
|
||
echo "Built extension binaries" | ||
} | ||
|
||
select_composer_json_for_PHP_version() { | ||
local PHP_version="${1:?}" | ||
case "${PHP_version}" in | ||
8.0|8.1|8.2) | ||
echo "composer_PHP_${PHP_version}.json" | ||
;; | ||
8.3) | ||
echo "composer_PHP_8.2.json" | ||
;; | ||
*) | ||
echo "Unexpected PHP version: ${PHP_version}" | ||
show_help | ||
exit 1 | ||
;; | ||
esac | ||
} | ||
|
||
download_PHP_packages_for_PHP_version() { | ||
local PHP_version="${1:?}" | ||
local dest_dir_for_current_args | ||
dest_dir_for_current_args="${destination_directory}/PHP_packages/PHP_${PHP_version}" | ||
|
||
echo "Downloading PHP packages for PHP version: ${PHP_version} to ${dest_dir_for_current_args} ..." | ||
|
||
ensure_dir_exists_and_empty "${dest_dir_for_current_args}" | ||
local composer_json_file_name | ||
composer_json_file_name=$(select_composer_json_for_PHP_version "${PHP_version}") | ||
local current_user_id | ||
current_user_id="$(id -u)" | ||
local current_user_group_id | ||
current_user_group_id="$(id -g)" | ||
docker run --rm \ | ||
-v "${dest_dir_for_current_args}:/app/vendor" \ | ||
-v "${PWD}/${composer_json_file_name}:/app/composer.json" \ | ||
-w /app \ | ||
php:${PHP_version}-cli sh -c "\ | ||
apt-get update && apt-get install -y unzip \ | ||
&& curl -sS https://getcomposer.org/installer | php -- --filename=composer --install-dir=/usr/local/bin \ | ||
&& composer --ignore-platform-req=ext-opentelemetry --no-dev install \ | ||
&& chown -R ${current_user_id}:${current_user_group_id} ./vendor/" | ||
|
||
echo "Downloaded PHP packages for PHP version: ${PHP_version} to ${dest_dir_for_current_args}" | ||
} | ||
|
||
download_PHP_packages() { | ||
echo "Downloading PHP packages..." | ||
|
||
for PHP_version in "${PHP_versions[@]}" ; do | ||
download_PHP_packages_for_PHP_version "${PHP_version}" | ||
done | ||
|
||
echo "Downloaded PHP packages" | ||
} | ||
|
||
main() { | ||
parse_args "$@" | ||
|
||
echo "Preparing files for docker image into directory ${destination_directory} ..." | ||
|
||
ensure_dir_exists_and_empty "${destination_directory}" | ||
# build_native_binaries | ||
download_PHP_packages | ||
|
||
echo "Prepared files for docker image into directory ${destination_directory}" | ||
} | ||
|
||
main "$@" |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about running this inside the Dockerfile? As part of a multi-stage build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you - good idea. I'll try - I wonder if it will work considering that the script runs other docker containers...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried running the script in Dockerfile during build but in order for it to work
/var/run/docker.sock
has to be mounted from the host (because the script spawns docker containers to build various files). Unfortunately it seems it is not possible to mount/var/run/docker.sock
from the host during the build phase of docker image.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it runs other containers, I think it doesnt make much sense to mount the docker sock. It simply makes it harder to execute it on machines with only e.g. podman available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to second the request to run all of this as a multi-stage docker build instead. That will make it much easier to maintain. I see that the script requires running docker images - in my view, you should instead rework it so the Dockerfile itself accepts the PHP version and libc flavor as arguments. If you can't fit everything in a single Dockerfile, multiple different ones are also fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for clarification. I understand that multiple images built by the workflow using QEMU each image for the corresponding CPU architecture, the part about which I am not clear is how the corresponding image is selected at the runtime? Namely how
pkg/instrumentation/python.go
knows which image to copy files from? Will the correct image with auto-instrumentation files be selected based on CPU architecture used by docker image with the instrumented application? Do I understand correctly that that determination will occur afterpkg/instrumentation/python.go
execution?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I did rebase your changes on
main
- should I have done a merge instead of rebase? Is there a way to fix the current messy state?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't switching to multi-stage image approach (either by generating Dockerfile or writing it manually) handle CPU architecture automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The operator doesn't know the CPU architecture of the image. It could find out, but it doesn't care what it is. The container runtime on the K8s Node (containerd in most cases) will simply download the right image for the Node's CPU architecture.
It will, but a Dockerfile with 6+ build stages, one for each combination of libc+php, is going to be messy and repetitive. I'm wondering if there's an elegant way of handing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that neither of the discussed proposals is perfect but this docker image is just a bag of files and it's not used directly by the end user.
Maybe for now we can implement one of the proposals that is good enough, we will get the feature out, receive feedback and then if/when necessary we can improve upon it?