Implemented cUrl handle caching (#877)

* Implemented cUrl handle caching * Added Stringable polyfill * Removed temporary code for tests * Added component test to verify that agent opens only one connection * ci: release automation with GH and Buildkite (scaffolding) with manual dry-run (#873) * Made component test infrastructure shared by tests * Renamed report produced by component tests to fit expected pattern Renamed from build/phpunit-component-junit.xml to build/component-tests-phpunit-junit.xml * Implemented cUrl handle caching * Added Stringable polyfill * Removed temporary code for tests * Added component test to verify that agent opens only one connection * Made component test infrastructure shared by tests * Renamed report produced by component tests to fit expected pattern Renamed from build/phpunit-component-junit.xml to build/component-tests-phpunit-junit.xml * Fixed test report name * Fixed MockApmServer not cleaning test scoped data * Fixed MockApmServer status code * Fixed missing reset in MockApmServerHandle::cleanTestScoped * Minor log formatting improvement --------- Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
elastic · Mar 8, 2023 · 791876a · 791876a
1 parent 08bc34d
commit 791876a
Show file tree

Hide file tree

Showing 91 changed files with 3,150 additions and 721 deletions.
diff --git a/.ci/Jenkinsfile b/.ci/Jenkinsfile
@@ -143,7 +143,7 @@ pipeline {
                 }
                 post {
                   always {
-                    junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/build/*junit.xml,${BASE_DIR}/phpunit-junit.xml")
+                    junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/build/*junit.xml")
                   }
                 }
               }
@@ -559,7 +559,7 @@ def runTestingCommand(def args = [:]) {
       }
     }
   } finally {
-    junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/build/phpunit-*junit.xml")
+    junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/build/*junit.xml")
   }
 }
 

diff --git a/.ci/component-test.sh b/.ci/component-test.sh
@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-set -xe
+set -xe -o pipefail
 
 # Disable Elastic APM for any process outside the component tests to prevent noise in the logs
 export ELASTIC_APM_ENABLED=false
@@ -27,6 +27,6 @@ else
 fi
 
 # Run component tests
-mkdir -p /app/build/
+mkdir -p ./build/
 composer run-script run_component_tests 2>&1 | tee /app/build/run_component_tests_output.txt
 
diff --git a/.ci/loop.sh b/.ci/loop.sh
@@ -6,7 +6,7 @@ PHP_VERSION=${3:-7.2}
 
 OUTPUT_FOLDER="build/loop-$DOCKERFILE-$PHP_VERSION"
 TEST_REPORT_TEST="junit.xml"
-TEST_REPORT_COMPOSER="build/phpunit-component-junit.xml"
+TEST_REPORT_COMPOSER="build/component-tests-phpunit-junit.xml"
 mkdir -p "${OUTPUT_FOLDER}" || true
 
 for (( c=1; c<=LOOPS; c++ ))

diff --git a/.ci/validate_agent_installation.sh b/.ci/validate_agent_installation.sh
@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-set -xe
+set -xe -o pipefail
 
 function printInfoAboutEnvironment () {
     echo 'PHP version:'
@@ -34,12 +34,18 @@ function runComponentTests () {
     run_command_with_timeout_and_retries_args=(--max-tries=3 "${run_command_with_timeout_and_retries_args[@]}")
     run_command_with_timeout_and_retries_args=(--increase-timeout-exponentially=yes "${run_command_with_timeout_and_retries_args[@]}")
 
+    mkdir -p ./build/
+
     set +e
     # shellcheck disable=SC2086 # composerCommand is not wrapped in quotes on purpose because it might contained multiple space separated strings
     .ci/run_command_with_timeout_and_retries.sh "${run_command_with_timeout_and_retries_args[@]}" -- "${composerCommand[@]}"
     local composerCommandExitCode=$?
     set -e
 
+    echo "Content of ./build/ begin"
+    ls -l ./build/
+    echo "Content of ./build/ end"
+
     echo "${composerCommand[*]} exited with an error code ${composerCommandExitCode}"
     if [ ${composerCommandExitCode} -eq 0 ] ; then
         local shouldPrintTheMostRecentSyslogFile=false

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -166,11 +166,11 @@ jobs:
         name: Prepare Upload
         run: >-
           find build
-          -name "phpunit-*junit.xml"
+          -name "*junit.xml"
           -exec bash -c 'mv {} "build/${ELASTIC_APM_PHP_TESTS_MATRIX_ROW}-$(basename {})"'
           \;
       - if: success() || failure()
         uses: actions/upload-artifact@v3
         with:
           name: test-results
-          path: build/*-phpunit-*junit.xml
+          path: build/*junit.xml
diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc
@@ -441,9 +441,9 @@ your communications using HTTPS. Unless you do so, your secret token could be ob
 If a request sending events to the APM server takes longer than the configured timeout,
 the request is canceled and the events are discarded.
 
-This configuration option supports the duration suffixes: `ms`, `s` and `m`.
-For example: `10s`.
-This option's default unit is `s`, so `5` is interpreted as `5s`.
+The value has to be provided in *<<configure-duration-format, duration format>>*.
+
+This option's default unit is `s` (seconds).
 
 If the value is `0` (or `0ms`, `0s`, etc.) the timeout for sending events to the APM Server is disabled.
 

diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -3,5 +3,6 @@
     <rule ref="PSR12" />
     <rule ref="Generic.PHP.RequireStrictTypes" />
 
+    <exclude-pattern>*/polyfills/Stringable.php</exclude-pattern>
     <exclude-pattern>*/polyfills/WeakMap.php</exclude-pattern>
 </ruleset>
diff --git a/phpstan.neon b/phpstan.neon
@@ -8,6 +8,7 @@ parameters:
     reportUnmatchedIgnoredErrors: false
 
     excludePaths:
+        - tests/polyfills/Stringable.php
         - tests/polyfills/WeakMap.php
 
     ignoreErrors:

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -26,7 +26,7 @@
         <ini name="memory_limit" value="2G"/>
     </php>
     <logging>
-        <junit outputFile="build/phpunit-junit.xml"/>
+        <junit outputFile="build/unit-tests-phpunit-junit.xml"/>
     </logging>
     <testsuites>
         <testsuite name="Tests">

diff --git a/phpunit_component_tests.xml b/phpunit_component_tests.xml
@@ -26,7 +26,7 @@
         <ini name="memory_limit" value="2G"/>
     </php>
     <logging>
-        <junit outputFile="build/phpunit-component-junit.xml"/>
+        <junit outputFile="build/component-tests-phpunit-junit.xml"/>
     </logging>
     <testsuites>
         <testsuite name="Tests">

diff --git a/src/ElasticApm/Impl/BackendComm/EventSender.php b/src/ElasticApm/Impl/BackendComm/EventSender.php
@@ -152,10 +152,8 @@ function (MetricSet $metricSet) use (&$serializedEvents) {
             && $loggerProxy->log(
                 'Calling elastic_apm_send_to_server...',
                 [
-                    'disableSend'                 => $this->config->disableSend(),
-                    'serverTimeout'               => $this->config->serverTimeout(),
-                    'strlen(userAgentHttpHeader)' => strlen($this->userAgentHttpHeader),
-                    'strlen(serializedEvents)'    => strlen($serializedEvents),
+                    'userAgentHttpHeader'      => $this->userAgentHttpHeader,
+                    'strlen(serializedEvents)' => strlen($serializedEvents),
                 ]
             );
 
@@ -165,17 +163,7 @@ function (MetricSet $metricSet) use (&$serializedEvents) {
              * @noinspection PhpFullyQualifiedNameUsageInspection, PhpUndefinedFunctionInspection
              * @phpstan-ignore-next-line
              */
-            \elastic_apm_send_to_server(
-                self::boolToInt($this->config->disableSend()),
-                $this->config->serverTimeout(),
-                $this->userAgentHttpHeader,
-                $serializedEvents
-            );
+            \elastic_apm_send_to_server($this->userAgentHttpHeader, $serializedEvents);
         }
     }
-
-    private static function boolToInt(bool $boolVal): int
-    {
-        return $boolVal ? 1 : 0;
-    }
 }
diff --git a/src/ElasticApm/Impl/Util/ArrayUtil.php b/src/ElasticApm/Impl/Util/ArrayUtil.php
@@ -159,17 +159,16 @@ public static function getNullableIntValueIfKeyExistsElse($key, array $array, ?i
     }
 
     /**
-     * @param string        $key
-     * @param mixed         $defaultValue
-     * @param array<mixed>  $array
-     * @return mixed
+     * @template TKey of string|int
+     * @template TValue
      *
-     * @template        T
-     * @phpstan-param   T   $defaultValue
-     * @phpstan-param   T[] $array
-     * @phpstan-return  T
+     * @param TKey                $key
+     * @param TValue              $defaultValue
+     * @param array<TKey, TValue> $array
+     *
+     * @return TValue
      */
-    public static function &getOrAdd(string $key, $defaultValue, array $array)
+    public static function &getOrAdd($key, $defaultValue, array &$array)
     {
         if (!array_key_exists($key, $array)) {
             $array[$key] = $defaultValue;

diff --git a/src/ext/ConfigManager.c b/src/ext/ConfigManager.c
@@ -45,6 +45,7 @@ enum ParsedOptionValueType
     parsedOptionValueType_string,
     parsedOptionValueType_int,
     parsedOptionValueType_duration,
+    parsedOptionValueType_size,
 
     end_parsedOptionValueType
 };
@@ -64,6 +65,7 @@ struct ParsedOptionValue
         String stringValue;
         int intValue;
         Duration durationValue;
+        Size sizeValue;
     } u;
 };
 typedef struct ParsedOptionValue ParsedOptionValue;
@@ -82,13 +84,21 @@ typedef struct EnumOptionAdditionalMetadata EnumOptionAdditionalMetadata;
 struct DurationOptionAdditionalMetadata
 {
     DurationUnits defaultUnits;
+    bool isNegativeValid;
 };
 typedef struct DurationOptionAdditionalMetadata DurationOptionAdditionalMetadata;
 
+struct SizeOptionAdditionalMetadata
+{
+    SizeUnits defaultUnits;
+};
+typedef struct SizeOptionAdditionalMetadata SizeOptionAdditionalMetadata;
+
 union OptionAdditionalMetadata
 {
     EnumOptionAdditionalMetadata enumData;
     DurationOptionAdditionalMetadata durationData;
+    SizeOptionAdditionalMetadata sizeData;
 };
 typedef union OptionAdditionalMetadata OptionAdditionalMetadata;
 
@@ -343,7 +353,15 @@ static ResultCode parseDurationValue( const OptionMetadata* optMeta, String rawV
     ResultCode parseResultCode = parseDuration( stringToView( rawValue )
                                                 , optMeta->additionalData.durationData.defaultUnits
                                                 , /* out */ &parsedValue->u.durationValue );
-    if ( parseResultCode == resultSuccess ) parsedValue->type = parsedOptionValueType_duration;
+    if ( parseResultCode == resultSuccess )
+    {
+        if ( parsedValue->u.durationValue.valueInUnits < 0 && ! optMeta->additionalData.durationData.isNegativeValid )
+        {
+            return resultParsingFailed;
+        }
+        parsedValue->type = parsedOptionValueType_duration;
+    }
+
     return parseResultCode;
 }
 
@@ -368,6 +386,42 @@ static void parsedDurationValueToZval( const OptionMetadata* optMeta, ParsedOpti
     RETURN_DOUBLE( durationToMilliseconds( parsedValue.u.durationValue ) );
 }
 
+static ResultCode parseSizeValue( const OptionMetadata* optMeta, String rawValue, /* out */ ParsedOptionValue* parsedValue )
+{
+    ELASTIC_APM_ASSERT_VALID_PTR( optMeta );
+    ELASTIC_APM_ASSERT_EQ_UINT64( optMeta->defaultValue.type, parsedOptionValueType_size );
+    ELASTIC_APM_ASSERT_VALID_PTR( rawValue );
+    ELASTIC_APM_ASSERT_VALID_PTR( parsedValue );
+    ELASTIC_APM_ASSERT_EQ_UINT64( parsedValue->type, parsedOptionValueType_undefined );
+
+    ResultCode parseResultCode = parseSize( stringToView( rawValue )
+                                                , optMeta->additionalData.sizeData.defaultUnits
+                                                , /* out */ &parsedValue->u.sizeValue );
+    if ( parseResultCode == resultSuccess ) parsedValue->type = parsedOptionValueType_size;
+    return parseResultCode;
+}
+
+static String streamParsedSize( const OptionMetadata* optMeta, ParsedOptionValue parsedValue, TextOutputStream* txtOutStream )
+{
+    ELASTIC_APM_ASSERT_VALID_PTR( optMeta );
+    ELASTIC_APM_ASSERT_EQ_UINT64( optMeta->defaultValue.type, parsedOptionValueType_size );
+    ELASTIC_APM_ASSERT_VALID_PARSED_OPTION_VALUE( parsedValue );
+    ELASTIC_APM_ASSERT_EQ_UINT64( parsedValue.type, optMeta->defaultValue.type );
+
+    return streamSize( parsedValue.u.sizeValue, txtOutStream );
+}
+
+static void parsedSizeValueToZval( const OptionMetadata* optMeta, ParsedOptionValue parsedValue, zval* return_value )
+{
+    ELASTIC_APM_ASSERT_VALID_PTR( optMeta );
+    ELASTIC_APM_ASSERT_EQ_UINT64( optMeta->defaultValue.type, parsedOptionValueType_size );
+    ELASTIC_APM_ASSERT_VALID_PARSED_OPTION_VALUE( parsedValue );
+    ELASTIC_APM_ASSERT_EQ_UINT64( parsedValue.type, optMeta->defaultValue.type );
+    ELASTIC_APM_ASSERT_VALID_PTR( return_value );
+
+    RETURN_DOUBLE( sizeToBytes( parsedValue.u.sizeValue ) );
+}
+
 static
 ResultCode parseEnumValue( const OptionMetadata* optMeta, String rawValue, /* out */ ParsedOptionValue* parsedValue )
 {
@@ -559,7 +613,7 @@ static OptionMetadata buildDurationOptionMetadata(
         , SetConfigSnapshotFieldFunc setFieldFunc
         , GetConfigSnapshotFieldFunc getFieldFunc
         , DurationUnits defaultUnits
-)
+        , bool isNegativeValid )
 {
     return (OptionMetadata)
     {
@@ -575,7 +629,36 @@ static OptionMetadata buildDurationOptionMetadata(
         .setField = setFieldFunc,
         .getField = getFieldFunc,
         .parsedValueToZval = &parsedDurationValueToZval,
-        .additionalData = (OptionAdditionalMetadata){ .durationData = (DurationOptionAdditionalMetadata){ .defaultUnits = defaultUnits } }
+        .additionalData = (OptionAdditionalMetadata){ .durationData = (DurationOptionAdditionalMetadata){ .defaultUnits = defaultUnits, .isNegativeValid = isNegativeValid } }
+    };
+}
+
+static OptionMetadata buildSizeOptionMetadata(
+        String name
+        , StringView iniName
+        , bool isSecret
+        , bool isDynamic
+        , Size defaultValue
+        , SetConfigSnapshotFieldFunc setFieldFunc
+        , GetConfigSnapshotFieldFunc getFieldFunc
+        , SizeUnits defaultUnits
+)
+{
+    return (OptionMetadata)
+    {
+        .name = name,
+        .iniName = iniName,
+        .isSecret = isSecret,
+        .isDynamic = isDynamic,
+        .isLoggingRelated = false,
+        .defaultValue = { .type = parsedOptionValueType_size, .u.sizeValue = defaultValue },
+        .interpretIniRawValue = &interpretStringIniRawValue,
+        .parseRawValue = &parseSizeValue,
+        .streamParsedValue = &streamParsedSize,
+        .setField = setFieldFunc,
+        .getField = getFieldFunc,
+        .parsedValueToZval = &parsedSizeValueToZval,
+        .additionalData = (OptionAdditionalMetadata){ .sizeData = (SizeOptionAdditionalMetadata){ .defaultUnits = defaultUnits } }
     };
 }
 
@@ -686,7 +769,7 @@ ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( boolValue, breakdownMetrics )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( boolValue, captureErrors )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, devInternal )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, disableInstrumentations )
-ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, disableSend )
+ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( boolValue, disableSend )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( boolValue, enabled )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, environment )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, hostname )
@@ -710,7 +793,7 @@ ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, profilingInferredSpansMinDur
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, profilingInferredSpansSamplingInterval )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, sanitizeFieldNames )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, secretToken )
-ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, serverTimeout )
+ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( durationValue, serverTimeout )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, serverUrl )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, serviceName )
 ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( stringValue, serviceNodeName )
@@ -746,8 +829,8 @@ ELASTIC_APM_DEFINE_FIELD_ACCESS_FUNCS( boolValue, verifyServerCert )
 #define ELASTIC_APM_INIT_METADATA( buildFunc, fieldName, optName, defaultValue ) \
     ELASTIC_APM_INIT_METADATA_EX( buildFunc, fieldName, optName, /* isSecret */ false, /* isDynamic */ false, defaultValue )
 
-#define ELASTIC_APM_INIT_DURATION_METADATA( fieldName, optName, defaultValue, defaultUnits ) \
-    ELASTIC_APM_INIT_METADATA_EX( buildDurationOptionMetadata, fieldName, optName, /* isSecret */ false, /* isDynamic */ false, defaultValue, defaultUnits )
+#define ELASTIC_APM_INIT_DURATION_METADATA( fieldName, optName, defaultValue, defaultUnits, isNegativeValid ) \
+    ELASTIC_APM_INIT_METADATA_EX( buildDurationOptionMetadata, fieldName, optName, /* isSecret */ false, /* isDynamic */ false, defaultValue, defaultUnits, isNegativeValid )
 
 #define ELASTIC_APM_INIT_SECRET_METADATA( buildFunc, fieldName, optName, defaultValue ) \
     ELASTIC_APM_INIT_METADATA_EX( buildFunc, fieldName, optName, /* isSecret */ true, /* isDynamic */ false, defaultValue )
@@ -871,10 +954,10 @@ static void initOptionsMetadata( OptionMetadata* optsMeta )
             /* defaultValue: */ NULL );
 
     ELASTIC_APM_INIT_METADATA(
-            buildStringOptionMetadata,
+            buildBoolOptionMetadata,
             disableSend,
             ELASTIC_APM_CFG_OPT_NAME_DISABLE_SEND,
-            /* defaultValue: */ NULL );
+            /* defaultValue: */ false );
 
     ELASTIC_APM_INIT_METADATA(
             buildBoolOptionMetadata,
@@ -974,11 +1057,12 @@ static void initOptionsMetadata( OptionMetadata* optsMeta )
             ELASTIC_APM_CFG_OPT_NAME_SECRET_TOKEN,
             /* defaultValue: */ NULL );
 
-    ELASTIC_APM_INIT_METADATA(
-            buildStringOptionMetadata,
-            serverTimeout,
-            ELASTIC_APM_CFG_OPT_NAME_SERVER_TIMEOUT,
-            /* defaultValue: */ NULL );
+    ELASTIC_APM_INIT_DURATION_METADATA(
+            serverTimeout
+            , ELASTIC_APM_CFG_OPT_NAME_SERVER_TIMEOUT
+            , /* defaultValue */ makeDuration( 30, durationUnits_second )
+            , /* defaultUnits: */ durationUnits_second
+            , /* isNegativeValid */ false );
 
     ELASTIC_APM_INIT_METADATA(
             buildStringOptionMetadata,