fix 8.4 segfaults, test against PHP nightly (#153)

* test against PHP nightly
  - build against PHP nightly on PR and daily schedule
  - use setup-php for linux builds (which drops Alpine builds)
* fix 8.4 segfaults
* fix 32bit tests
  - remove the ability to pass complex items as attributes (since the spec already
  disallows them, they'd get dropped later anyway)
  - update alpine dockerfile to also support 32bit builds, which enables local
  testing against a 32bit platform

.github/workflows/build.yml

@@ -16,21 +16,23 @@ jobs:
       fail-fast: false
       matrix:
         php: ['8.0', '8.1', '8.2', '8.3']
-        os: ['debian', 'alpine']
-    container:
-      image: ghcr.io/open-telemetry/opentelemetry-php-instrumentation/php:${{ matrix.php }}-${{ matrix.os }}-debug
 
     steps:
-      -   uses: actions/checkout@v4
-      -   name: Build
-          run: |
-            phpize
-            ./configure
-            make
-      -   name: Test
-          env:
-            TEST_PHP_ARGS: "-q" #do not try to submit failures
-          run: make test TESTS=--show-diff
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php }}
+      - name: Build
+        run: |
+          phpize
+          ./configure
+          make
+      - name: Test
+        env:
+          TEST_PHP_ARGS: "-q" #do not try to submit failures
+        run: make test TESTS=--show-diff
 
   macos:
     runs-on: macos-latest

.github/workflows/nightly.yml

@@ -0,0 +1,31 @@
+name: Build and test against PHP nightly
+
+on:
+  push:
+  pull_request:
+    branches: [ main ]
+  schedule:
+    - cron: '37 5 * * *'
+
+defaults:
+  run:
+    working-directory: ext
+
+jobs:
+  nightly:
+    if: github.repository == 'open-telemetry/opentelemetry-php-instrumentation'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.4
+      - name: Build
+        run: |
+          phpize
+          ./configure
+          make
+      - name: Test
+        env:
+          TEST_PHP_ARGS: "-q"
+        run:  make test TESTS=--show-diff

docker-compose.yaml

@@ -19,3 +19,14 @@ services:
       - ./ext:/usr/src/myapp
     environment:
       TEST_PHP_ARGS: "-q"
+  32bit:
+    build:
+      context: docker
+      dockerfile: Dockerfile.alpine
+      args:
+        ALPINE_VERSION: i386/alpine
+        PHP_VERSION: ${PHP_VERSION:-8.4.0beta4}
+    volumes:
+      - ./ext:/usr/src/myapp
+    environment:
+      TEST_PHP_ARGS: "-q"

docker/Dockerfile.alpine

@@ -1,4 +1,5 @@
-FROM alpine:3.16 as builder
+ARG ALPINE_VERSION=alpine:3.20
+FROM ${ALPINE_VERSION} as builder
 WORKDIR /usr/src
 
 ENV PHPIZE_DEPS \
@@ -22,23 +23,25 @@ RUN apk add --no-cache \
     xz
 
 RUN apk add --no-cache \
+    bison \
     coreutils \
     curl-dev \
     libxml2-dev \
     linux-headers \
+    re2c \
     readline-dev \
     sqlite-dev
 
 ARG PHP_VERSION
-ENV PHP_URL="https://www.php.net/distributions/php-${PHP_VERSION}.tar.xz"
-
-RUN echo "$PHP_URL" && curl -fsSL -o php.tar.xz "$PHP_URL"
-RUN cd /usr/src \
-  && tar -xf php.tar.xz
+ENV PHP_URL="https://github.com/php/php-src/archive/refs/tags/php-${PHP_VERSION}.tar.gz"
 
 ARG PHP_CONFIG_OPTS="--enable-debug --with-pear --with-zlib"
-RUN cd php-${PHP_VERSION} \
-  && ./buildconf \
+RUN echo "$PHP_URL" && curl -fsSL -o php.tar.gz "$PHP_URL" \
+  && cd /usr/src \
+  && mkdir php-src \
+  && tar -xzf php.tar.gz -C php-src --strip-components=1 \
+  && cd php-src \
+  && ./buildconf --force \
   && ./configure ${PHP_CONFIG_OPTS} \
   && make -j $(nproc) \
   && make install

ext/otel_observer.c

@@ -152,6 +152,23 @@ static bool func_has_withspan_attribute(zend_execute_data *ex) {
     return attr != NULL;
 }
 
+/*
+ * OpenTelemetry attribute values may only be of limited types
+ */
+static bool is_valid_attribute_value(zval *val) {
+    switch (Z_TYPE_P(val)) {
+    case IS_STRING:
+    case IS_LONG:
+    case IS_DOUBLE:
+    case IS_TRUE:
+    case IS_FALSE:
+    case IS_ARRAY:
+        return true;
+    default:
+        return false;
+    }
+}
+
 // get function args. any args with the
 // SpanAttributes attribute are added to the attributes HashTable
 static void func_get_args(zval *zv, HashTable *attributes,
@@ -198,7 +215,7 @@ static void func_get_args(zval *zv, HashTable *attributes,
                     zend_string *arg_name = ex->func->op_array.vars[i];
                     zend_attribute *attribute =
                         find_spanattribute_attribute(ex->func, i);
-                    if (attribute != NULL) {
+                    if (attribute != NULL && is_valid_attribute_value(p)) {
                         if (attribute->argc) {
                             zend_string *key = Z_STR(attribute->args[0].value);
                             zend_hash_del(attributes, key);
@@ -1149,5 +1166,8 @@ void opentelemetry_observer_init(INIT_FUNC_ARGS) {
         zend_observer_fcall_register(observer_fcall_init);
         op_array_extension =
             zend_get_op_array_extension_handle("opentelemetry");
+#if PHP_VERSION_ID >= 80400
+        zend_get_internal_function_extension_handle("opentelemetry");
+#endif
     }
 }

ext/tests/span_attribute/function_params_non_simple.phpt

@@ -1,5 +1,5 @@
 --TEST--
-Check if function non-simple types can be passed as function params
+Check if function non-simple types are ignored
 --SKIPIF--
 <?php if (PHP_VERSION_ID < 80100) die('skip requires PHP >= 8.1'); ?>
 --EXTENSIONS--
@@ -28,28 +28,20 @@ function foo(
 }
 
 foo(
-    ['foo' => 'bar'],
-    new \stdClass(),
-    function(){return 'fn';},
-    null,
+    one: ['foo' => 'bar'],
+    two: new \stdClass(),
+    three: function(){return 'fn';},
+    four: null,
 );
 ?>
 --EXPECTF--
 string(3) "pre"
-array(4) {
+array(1) {
   ["one"]=>
   array(1) {
     ["foo"]=>
     string(3) "bar"
   }
-  ["two"]=>
-  object(stdClass)#1 (0) {
-  }
-  ["three"]=>
-  object(Closure)#2 (%d) {%A
-  }
-  ["four"]=>
-  NULL
 }
 string(3) "foo"
 string(4) "post"