Skip to content

Commit

Permalink
fix 32bit tests
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
brettmc committed Sep 8, 2024
1 parent e883528 commit 219af9b
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 23 deletions.
11 changes: 11 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
19 changes: 11 additions & 8 deletions docker/Dockerfile.alpine
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
FROM alpine:3.16 as builder
ARG ALPINE_VERSION=alpine:3.16
FROM ${ALPINE_VERSION} as builder
WORKDIR /usr/src

ENV PHPIZE_DEPS \
Expand All @@ -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
Expand Down
19 changes: 18 additions & 1 deletion ext/otel_observer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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: // Numeric (integer)
case IS_DOUBLE: // Numeric (floating point)
case IS_TRUE:
case IS_FALSE: // Boolean
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,
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 6 additions & 14 deletions ext/tests/span_attribute/function_params_non_simple.phpt
Original file line number Diff line number Diff line change
@@ -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--
Expand Down Expand Up @@ -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"

0 comments on commit 219af9b

Please sign in to comment.