From eb773e757a32a3b5b2c62ecbf33db0b13c3ae59c Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 07:48:25 +0300 Subject: [PATCH 1/9] Added logImportantAgentInfo --- agent/native/ext/lifecycle.cpp | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index bb0429527..9c7130d74 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -500,6 +500,22 @@ static void unregisterErrorAndExceptionHooks() { } +void logImportantAgentInfo( const ConfigSnapshot* config, String calledFromFunc ) +{ + ELASTIC_APM_LOG_INFO( + "Custom build based on version: %s" + "; Custom changes: " + "- agent is disabled by default (enabled: false)" + "; config->enabled: %s." + "; SAPI: %s" + "; Called from: %s" + , PHP_ELASTIC_APM_VERSION + , boolToString( config->enabled ) + , isPhpRunningAsCliScript() + , calledFromFunc + ); +} + void elasticApmModuleInit( int moduleType, int moduleNumber ) { ELASTIC_APM_LOG_DIRECT_DEBUG( "%s entered: moduleType: %d, moduleNumber: %d, parent PID: %d", __FUNCTION__, moduleType, moduleNumber, (int)(getParentProcessId()) ); @@ -526,10 +542,11 @@ void elasticApmModuleInit( int moduleType, int moduleNumber ) ELASTIC_APM_CALL_IF_FAILED_GOTO( ensureLoggerInitialConfigIsLatest( tracer ) ); ELASTIC_APM_CALL_IF_FAILED_GOTO( ensureAllComponentsHaveLatestConfig( tracer ) ); - logSupportabilityInfo( logLevel_debug ); - config = getTracerCurrentConfigSnapshot( tracer ); + logImportantAgentInfo( config, __FUNCTION__ ); + logSupportabilityInfo( logLevel_debug ); + if ( ! config->enabled ) { resultCode = resultSuccess; @@ -580,6 +597,8 @@ void elasticApmModuleShutdown( int moduleType, int moduleNumber ) Tracer* const tracer = getGlobalTracer(); const ConfigSnapshot* const config = getTracerCurrentConfigSnapshot( tracer ); + logImportantAgentInfo( config, __FUNCTION__ ); + if ( ! config->enabled ) { resultCode = resultSuccess; From 1b718249c381851120152fa4c774233509318dc1 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 07:48:45 +0300 Subject: [PATCH 2/9] Changed ENABLED config default to false --- agent/native/ext/ConfigManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/native/ext/ConfigManager.cpp b/agent/native/ext/ConfigManager.cpp index 2f7005e73..60f975af0 100644 --- a/agent/native/ext/ConfigManager.cpp +++ b/agent/native/ext/ConfigManager.cpp @@ -1034,7 +1034,7 @@ static void initOptionsMetadata( OptionMetadata* optsMeta ) buildBoolOptionMetadata, enabled, ELASTIC_APM_CFG_OPT_NAME_ENABLED, - /* defaultValue: */ true ); + /* defaultValue: */ false ); ELASTIC_APM_INIT_METADATA( buildStringOptionMetadata, From e03787b584f73d9960ef0b039b10e07a0311a9ef Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 09:17:07 +0300 Subject: [PATCH 3/9] Adapted CI to handle enabled set to false --- .ci/shared.sh | 2 + .ci/static-check-unit-test.sh | 7 +- .ci/validate_agent_installation.sh | 16 ++- agent/native/ext/lifecycle.cpp | 4 +- agent/native/ext/util_for_PHP.cpp | 5 + agent/native/ext/util_for_PHP.h | 1 + .../Impl/Config/AllOptionsMetadata.php | 2 +- .../AgentEnabledConfigComponentTest.php | 108 ++++++++++++++++++ .../Util/ComponentTestCaseBase.php | 7 +- .../Util/ConfigSnapshotForTests.php | 3 + .../Util/ExpectedEventCounts.php | 7 +- .../UnitTests/AgentEnabledConfigUnitTest.php | 46 ++++++++ 12 files changed, 194 insertions(+), 14 deletions(-) create mode 100644 tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php create mode 100644 tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php diff --git a/.ci/shared.sh b/.ci/shared.sh index 0c6ff0e46..d3440caf8 100644 --- a/.ci/shared.sh +++ b/.ci/shared.sh @@ -23,6 +23,8 @@ export ELASTIC_APM_PHP_TESTS_APP_CODE_HOST_KINDS_SHORT_NAMES=("${ELASTIC_APM_PHP export ELASTIC_APM_PHP_TESTS_LEAF_GROUPS_SHORT_NAMES=(no_ext_svc with_ext_svc) export ELASTIC_APM_PHP_TESTS_GROUPS_SHORT_NAMES=("${ELASTIC_APM_PHP_TESTS_LEAF_GROUPS_SHORT_NAMES[@]}" smoke) +export ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT=false + function ensureSyslogIsRunningImpl () { if ps -ef | grep -v 'grep' | grep -q 'syslogd' ; then echo 'Syslog is already started.' diff --git a/.ci/static-check-unit-test.sh b/.ci/static-check-unit-test.sh index d2c485f69..75da45f7c 100755 --- a/.ci/static-check-unit-test.sh +++ b/.ci/static-check-unit-test.sh @@ -42,8 +42,13 @@ runPhpCoposerInstall composer run-script static_check # Run unit tests +composerCommandToRunUnitTests=(composer run-script --) phpUnitConfigFile=$(php ./tests/ElasticApmTests/Util/runSelectPhpUnitConfigFile.php --tests-type=unit) -composer run-script -- run_unit_tests_custom_config -c "${phpUnitConfigFile}" +composerCommandToRunUnitTests=("${composerCommand[@]}" run_unit_tests_custom_config -c "${phpUnitConfigFile}) +if [ "${ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT:?}" == "false" ]; then + composerCommandToRunUnitTests=("${AgentEnabledConfigUnitTest[@]}" --filter "AgentEnabledConfigUnitTest") +fi +"${composerCommandToRunUnitTests[@]}" ls -l ./build/unit-tests-phpunit-junit.xml # Generate junit output for phpstan diff --git a/.ci/validate_agent_installation.sh b/.ci/validate_agent_installation.sh index 03e78a845..49ce3a76d 100755 --- a/.ci/validate_agent_installation.sh +++ b/.ci/validate_agent_installation.sh @@ -22,12 +22,16 @@ function runComponentTests () { phpUnitConfigFile=$(php ./tests/ElasticApmTests/Util/runSelectPhpUnitConfigFile.php --tests-type=component) composerCommand=("${composerCommand[@]}" -c "${phpUnitConfigFile}") - if [ -n "${ELASTIC_APM_PHP_TESTS_GROUP}" ] ; then - composerCommand=("${composerCommand[@]}" --group "${ELASTIC_APM_PHP_TESTS_GROUP}") - fi - - if [ -n "${ELASTIC_APM_PHP_TESTS_FILTER}" ] ; then - composerCommand=("${composerCommand[@]}" --filter "${ELASTIC_APM_PHP_TESTS_FILTER}") + if [ "${ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT:?}" == "true" ]; then + if [ -n "${ELASTIC_APM_PHP_TESTS_GROUP}" ] ; then + composerCommand=("${composerCommand[@]}" --group "${ELASTIC_APM_PHP_TESTS_GROUP}") + fi + + if [ -n "${ELASTIC_APM_PHP_TESTS_FILTER}" ] ; then + composerCommand=("${composerCommand[@]}" --filter "${ELASTIC_APM_PHP_TESTS_FILTER}") + fi + else + composerCommand=("${composerCommand[@]}" --filter "AgentEnabledConfigComponentTest") fi local initialTimeoutInMinutes=30 diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index 9c7130d74..373181112 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -507,11 +507,11 @@ void logImportantAgentInfo( const ConfigSnapshot* config, String calledFromFunc "; Custom changes: " "- agent is disabled by default (enabled: false)" "; config->enabled: %s." - "; SAPI: %s" + "; SAPI module name: %s" "; Called from: %s" , PHP_ELASTIC_APM_VERSION , boolToString( config->enabled ) - , isPhpRunningAsCliScript() + , getPhpSapiModuleName() , calledFromFunc ); } diff --git a/agent/native/ext/util_for_PHP.cpp b/agent/native/ext/util_for_PHP.cpp index 450e91eca..725cb2b05 100644 --- a/agent/native/ext/util_for_PHP.cpp +++ b/agent/native/ext/util_for_PHP.cpp @@ -275,6 +275,11 @@ ResultCode callPhpFunctionRetZval( StringView phpFunctionName, uint32_t argsCoun return callPhpFunction( phpFunctionName, argsCount, args, consumeZvalRetVal, retVal ); } +String getPhpSapiModuleName() +{ + return sapi_module.name; +} + bool isPhpRunningAsCliScript() { return strcmp( sapi_module.name, "cli" ) == 0; diff --git a/agent/native/ext/util_for_PHP.h b/agent/native/ext/util_for_PHP.h index 7c6c59442..bab557ef2 100644 --- a/agent/native/ext/util_for_PHP.h +++ b/agent/native/ext/util_for_PHP.h @@ -85,6 +85,7 @@ ResultCode callPhpFunctionRetZval( StringView phpFunctionName, uint32_t argsCoun void getArgsFromZendExecuteData( zend_execute_data *execute_data, size_t dstArraySize, zval dstArray[], uint32_t* argsCount ); +String getPhpSapiModuleName(); bool isPhpRunningAsCliScript(); bool detectOpcachePreload(); bool isScriptRestricedByOpcacheAPI(); diff --git a/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php b/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php index e3bd9ecd3..90cbbfbf0 100644 --- a/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php +++ b/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php @@ -93,7 +93,7 @@ public static function get(): array OptionNames::DEV_INTERNAL => new NullableWildcardListOptionMetadata(), OptionNames::DISABLE_INSTRUMENTATIONS => new NullableWildcardListOptionMetadata(), OptionNames::DISABLE_SEND => new BoolOptionMetadata(/* default */ false), - OptionNames::ENABLED => new BoolOptionMetadata(/* default */ true), + OptionNames::ENABLED => new BoolOptionMetadata(/* default */ false), OptionNames::ENVIRONMENT => new NullableStringOptionMetadata(), OptionNames::GLOBAL_LABELS => new NullableLabelsOptionMetadata(), OptionNames::HOSTNAME => new NullableStringOptionMetadata(), diff --git a/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php b/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php new file mode 100644 index 000000000..24d089d62 --- /dev/null +++ b/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php @@ -0,0 +1,108 @@ +getBool(self::ENABLED_CONFIG_SEEN_BY_TEST); + + /** + * elastic_apm_* functions are provided by the elastic_apm extension + * + * @noinspection PhpFullyQualifiedNameUsageInspection, PhpUndefinedFunctionInspection + * @phpstan-ignore-next-line + */ + $enabledConfigSeenByNativePart = \elastic_apm_is_enabled(); + self::assertSame($enabledConfigSeenByTest, $enabledConfigSeenByNativePart); + } + + /** + * @return iterable + */ + public static function dataProviderForTestWhenAgentIsDisabledItShouldNotSendAnyData(): iterable + { + $result = (new DataProviderForTestBuilder()) + ->addKeyedDimensionOnlyFirstValueCombinable(OptionNames::ENABLED, [null, true . false]) + ->build(); + + return self::adaptToSmoke(DataProviderForTestBuilder::convertEachDataSetToMixedMap($result)); + } + + /** + * @dataProvider dataProviderForTestWhenAgentIsDisabledItShouldNotSendAnyData + */ + public function testWhenAgentIsDisabledItShouldNotSendAnyData(MixedMap $testArgs): void + { + AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs()); + + $testCaseHandle = $this->getTestCaseHandle(); + $appCodeHost = $testCaseHandle->ensureMainAppCodeHost( + function (AppCodeHostParams $appCodeParams) use ($testArgs): void { + self::setConfigIfNotNull($testArgs, OptionNames::ENABLED, $appCodeParams); + } + ); + + $enabledConfigSeenByTest = $appCodeHost->appCodeHostParams->getEffectiveAgentConfig()->enabled(); + $dbgCtx->add(compact('enabledConfigSeenByTest')); + $logger = self::getLogger(__NAMESPACE__, __CLASS__, __FILE__); + ($loggerProxy = $logger->ifInfoLevelEnabled(__LINE__, __FUNCTION__)) && $loggerProxy->log('', ['enabledConfigSeenByTest' => $enabledConfigSeenByTest]); + + $appCodeHost->sendRequest( + AppCodeTarget::asRouted([__CLASS__, 'appCodeForTestWhenAgentIsDisabledItShouldNotSendAnyData']), + function (AppCodeRequestParams $appCodeRequestParams) use ($enabledConfigSeenByTest): void { + $appCodeRequestParams->setAppCodeArgs([self::ENABLED_CONFIG_SEEN_BY_TEST => $enabledConfigSeenByTest]); + } + ); + + if ($enabledConfigSeenByTest) { + $dataFromAgent = $this->waitForOneEmptyTransaction($testCaseHandle); + self::assertCount(1, $dataFromAgent->idToTransaction); + self::assertCount(1, $dataFromAgent->metadatas); + } else { + sleep(/* seconds */ 5); + $dataFromAgent = $testCaseHandle->waitForDataFromAgent((new ExpectedEventCounts())->metadatas(0)->transactions(0), /* shouldValidate */ false); + self::assertCount(0, $dataFromAgent->idToTransaction); + } + self::assertCount(0, $dataFromAgent->idToSpan); + self::assertCount(0, $dataFromAgent->idToError); + } +} diff --git a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php index c39134f6e..09276e086 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php @@ -254,11 +254,12 @@ protected static function implTestIsAutoInstrumentationEnabled(string $instrClas } /** - * @template T + * @template TKey + * @template TValue * - * @param iterable $variants + * @param iterable $variants * - * @return iterable + * @return iterable */ public static function adaptToSmoke(iterable $variants): iterable { diff --git a/tests/ElasticApmTests/ComponentTests/Util/ConfigSnapshotForTests.php b/tests/ElasticApmTests/ComponentTests/Util/ConfigSnapshotForTests.php index fd36bc698..b662cc229 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ConfigSnapshotForTests.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ConfigSnapshotForTests.php @@ -60,6 +60,9 @@ final class ConfigSnapshotForTests implements LoggableInterface /** @var bool */ public $deleteTempPhpIni; + /** @var ?bool */ + public $agentEnabledConfigDefault = null; + /** @var ?WildcardListMatcher */ public $envVarsToPassThrough; diff --git a/tests/ElasticApmTests/ComponentTests/Util/ExpectedEventCounts.php b/tests/ElasticApmTests/ComponentTests/Util/ExpectedEventCounts.php index 6f4b1443d..bff87a168 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ExpectedEventCounts.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ExpectedEventCounts.php @@ -43,7 +43,7 @@ public function __construct() { $this->perDataKind = []; $this->errors(0); - $this->setValueForKind(ApmDataKind::metadata(), 1); + $this->metadatas(1); $this->metricSets(0); $this->spans(0); $this->transactions(1); @@ -65,6 +65,11 @@ private function setValueForKind(ApmDataKind $apmDataKind, ?int $count): self return $this; } + public function metadatas(int $count): self + { + return $this->setValueForKind(ApmDataKind::metadata(), $count); + } + public function errors(int $count): self { return $this->setValueForKind(ApmDataKind::error(), $count); diff --git a/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php b/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php new file mode 100644 index 000000000..7d229aba2 --- /dev/null +++ b/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php @@ -0,0 +1,46 @@ +defaultValue(); + if ($configForTests->agentEnabledConfigDefault === null) { + self::assertSame(true, $agentEnabledConfigPhpPartDefault); + } else { + self::assertSame($configForTests->agentEnabledConfigDefault, $agentEnabledConfigPhpPartDefault); + } + } +} From a7aebd70decdc49fbf3ca63e24ade1d748db1879 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 09:23:35 +0300 Subject: [PATCH 4/9] Removed adaptToSmoke from AgentEnabledConfigComponentTest --- .../ComponentTests/AgentEnabledConfigComponentTest.php | 2 +- .../ComponentTests/Util/ComponentTestCaseBase.php | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php b/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php index 24d089d62..c3d9a9fc7 100644 --- a/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php +++ b/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php @@ -64,7 +64,7 @@ public static function dataProviderForTestWhenAgentIsDisabledItShouldNotSendAnyD ->addKeyedDimensionOnlyFirstValueCombinable(OptionNames::ENABLED, [null, true . false]) ->build(); - return self::adaptToSmoke(DataProviderForTestBuilder::convertEachDataSetToMixedMap($result)); + return DataProviderForTestBuilder::convertEachDataSetToMixedMap($result); } /** diff --git a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php index 09276e086..c39134f6e 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php @@ -254,12 +254,11 @@ protected static function implTestIsAutoInstrumentationEnabled(string $instrClas } /** - * @template TKey - * @template TValue + * @template T * - * @param iterable $variants + * @param iterable $variants * - * @return iterable + * @return iterable */ public static function adaptToSmoke(iterable $variants): iterable { From 45a43503e1bee35058707d9b574e6878ab37c33c Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 09:40:42 +0300 Subject: [PATCH 5/9] Fixed AgentEnabledConfigUnitTest --- .../UnitTests/AgentEnabledConfigUnitTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php b/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php index 7d229aba2..82e2c3589 100644 --- a/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php +++ b/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php @@ -36,11 +36,10 @@ final class AgentEnabledConfigUnitTest extends TestCaseBase public function test(): void { $configForTests = ConfigUtilForTests::read(/* additionalConfigSource */ null, NoopLoggerFactory::singletonInstance()); - $agentEnabledConfigPhpPartDefault = AllOptionsMetadata::get()[OptionNames::ENABLED]->defaultValue(); if ($configForTests->agentEnabledConfigDefault === null) { - self::assertSame(true, $agentEnabledConfigPhpPartDefault); - } else { - self::assertSame($configForTests->agentEnabledConfigDefault, $agentEnabledConfigPhpPartDefault); + self::dummyAssert(); + return; } + self::assertSame($configForTests->agentEnabledConfigDefault, AllOptionsMetadata::get()[OptionNames::ENABLED]->defaultValue()); } } From cbb06a6e61ac15d354f811134b6af4622e2bcec8 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 19:52:51 +0300 Subject: [PATCH 6/9] Fixed tests for agent disabled by default case --- .ci/static-check-unit-test.sh | 4 +- .../AgentEnabledConfigComponentTest.php | 46 ++++++++++--------- .../Util/ComponentTestCaseBase.php | 4 +- .../ComponentTests/Util/TestCaseHandle.php | 7 +++ .../UnitTests/AgentEnabledConfigUnitTest.php | 2 +- tests/ElasticApmTests/Util/MixedMap.php | 26 +++++++++-- 6 files changed, 61 insertions(+), 28 deletions(-) diff --git a/.ci/static-check-unit-test.sh b/.ci/static-check-unit-test.sh index 75da45f7c..d20ab80b0 100755 --- a/.ci/static-check-unit-test.sh +++ b/.ci/static-check-unit-test.sh @@ -44,9 +44,9 @@ composer run-script static_check # Run unit tests composerCommandToRunUnitTests=(composer run-script --) phpUnitConfigFile=$(php ./tests/ElasticApmTests/Util/runSelectPhpUnitConfigFile.php --tests-type=unit) -composerCommandToRunUnitTests=("${composerCommand[@]}" run_unit_tests_custom_config -c "${phpUnitConfigFile}) +composerCommandToRunUnitTests=("${composerCommandToRunUnitTests[@]}" run_unit_tests_custom_config -c "${phpUnitConfigFile}) if [ "${ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT:?}" == "false" ]; then - composerCommandToRunUnitTests=("${AgentEnabledConfigUnitTest[@]}" --filter "AgentEnabledConfigUnitTest") + composerCommandToRunUnitTests=("${composerCommandToRunUnitTests[@]}" --filter "AgentEnabledConfigUnitTest") fi "${composerCommandToRunUnitTests[@]}" ls -l ./build/unit-tests-phpunit-junit.xml diff --git a/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php b/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php index c3d9a9fc7..d6414c445 100644 --- a/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php +++ b/tests/ElasticApmTests/ComponentTests/AgentEnabledConfigComponentTest.php @@ -24,11 +24,11 @@ namespace ElasticApmTests\ComponentTests; use Elastic\Apm\Impl\Config\OptionNames; +use ElasticApmTests\ComponentTests\Util\AmbientContextForTests; use ElasticApmTests\ComponentTests\Util\AppCodeHostParams; use ElasticApmTests\ComponentTests\Util\AppCodeRequestParams; use ElasticApmTests\ComponentTests\Util\AppCodeTarget; use ElasticApmTests\ComponentTests\Util\ComponentTestCaseBase; -use ElasticApmTests\ComponentTests\Util\ExpectedEventCounts; use ElasticApmTests\Util\AssertMessageStack; use ElasticApmTests\Util\DataProviderForTestBuilder; use ElasticApmTests\Util\MixedMap; @@ -39,11 +39,9 @@ */ final class AgentEnabledConfigComponentTest extends ComponentTestCaseBase { - private const ENABLED_CONFIG_SEEN_BY_TEST = 'enabled_config_seen_by_test'; - - public static function appCodeForTestWhenAgentIsDisabledItShouldNotSendAnyData(MixedMap $appCodeArgs): void + public static function appCodeForTestAgentEnabledConfig(MixedMap $appCodeArgs): void { - $enabledConfigSeenByTest = $appCodeArgs->getBool(self::ENABLED_CONFIG_SEEN_BY_TEST); + $agentEnabledExpected = $appCodeArgs->getBool(OptionNames::ENABLED); /** * elastic_apm_* functions are provided by the elastic_apm extension @@ -51,26 +49,26 @@ public static function appCodeForTestWhenAgentIsDisabledItShouldNotSendAnyData(M * @noinspection PhpFullyQualifiedNameUsageInspection, PhpUndefinedFunctionInspection * @phpstan-ignore-next-line */ - $enabledConfigSeenByNativePart = \elastic_apm_is_enabled(); - self::assertSame($enabledConfigSeenByTest, $enabledConfigSeenByNativePart); + $agentEnabledAsSeenByNativePart = \elastic_apm_is_enabled(); + self::assertSame($agentEnabledExpected, $agentEnabledAsSeenByNativePart); } /** * @return iterable */ - public static function dataProviderForTestWhenAgentIsDisabledItShouldNotSendAnyData(): iterable + public static function dataProviderForTestAgentEnabledConfig(): iterable { $result = (new DataProviderForTestBuilder()) - ->addKeyedDimensionOnlyFirstValueCombinable(OptionNames::ENABLED, [null, true . false]) + ->addKeyedDimensionAllValuesCombinable(OptionNames::ENABLED, [null, true, false]) ->build(); return DataProviderForTestBuilder::convertEachDataSetToMixedMap($result); } /** - * @dataProvider dataProviderForTestWhenAgentIsDisabledItShouldNotSendAnyData + * @dataProvider dataProviderForTestAgentEnabledConfig */ - public function testWhenAgentIsDisabledItShouldNotSendAnyData(MixedMap $testArgs): void + public function testAgentEnabledConfig(MixedMap $testArgs): void { AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs()); @@ -81,26 +79,32 @@ function (AppCodeHostParams $appCodeParams) use ($testArgs): void { } ); - $enabledConfigSeenByTest = $appCodeHost->appCodeHostParams->getEffectiveAgentConfig()->enabled(); - $dbgCtx->add(compact('enabledConfigSeenByTest')); - $logger = self::getLogger(__NAMESPACE__, __CLASS__, __FILE__); - ($loggerProxy = $logger->ifInfoLevelEnabled(__LINE__, __FUNCTION__)) && $loggerProxy->log('', ['enabledConfigSeenByTest' => $enabledConfigSeenByTest]); + $agentEnabledTestArg = $testArgs->getNullableBool(OptionNames::ENABLED); + $agentEnabledActual = $appCodeHost->appCodeHostParams->getEffectiveAgentConfig()->enabled(); + if (($agentEnabledExpected = $agentEnabledTestArg ?? AmbientContextForTests::testConfig()->agentEnabledConfigDefault) !== null) { + self::assertSame($agentEnabledExpected, $agentEnabledActual); + } $appCodeHost->sendRequest( - AppCodeTarget::asRouted([__CLASS__, 'appCodeForTestWhenAgentIsDisabledItShouldNotSendAnyData']), - function (AppCodeRequestParams $appCodeRequestParams) use ($enabledConfigSeenByTest): void { - $appCodeRequestParams->setAppCodeArgs([self::ENABLED_CONFIG_SEEN_BY_TEST => $enabledConfigSeenByTest]); + AppCodeTarget::asRouted([__CLASS__, 'appCodeForTestAgentEnabledConfig']), + function (AppCodeRequestParams $appCodeRequestParams) use ($agentEnabledActual): void { + $appCodeRequestParams->setAppCodeArgs([OptionNames::ENABLED => $agentEnabledActual]); } ); - if ($enabledConfigSeenByTest) { + if ($agentEnabledActual) { $dataFromAgent = $this->waitForOneEmptyTransaction($testCaseHandle); self::assertCount(1, $dataFromAgent->idToTransaction); self::assertCount(1, $dataFromAgent->metadatas); } else { - sleep(/* seconds */ 5); - $dataFromAgent = $testCaseHandle->waitForDataFromAgent((new ExpectedEventCounts())->metadatas(0)->transactions(0), /* shouldValidate */ false); + $logger = self::getLoggerStatic(__NAMESPACE__, __CLASS__, __FILE__); + $waitTimeSeconds = 5; + ($loggerProxy = $logger->ifInfoLevelEnabled(__LINE__, __FUNCTION__)) + && $loggerProxy->log('Sleeping ' . $waitTimeSeconds . ' seconds to give agent enough time to send data (which it should not since it is disabled)...'); + sleep($waitTimeSeconds); + $dataFromAgent = $testCaseHandle->getDataFromAgentWithoutWaiting(); self::assertCount(0, $dataFromAgent->idToTransaction); + self::assertCount(0, $dataFromAgent->metadatas); } self::assertCount(0, $dataFromAgent->idToSpan); self::assertCount(0, $dataFromAgent->idToError); diff --git a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php index c39134f6e..45a5d2399 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php @@ -567,8 +567,10 @@ protected static function disableTimingDependentFeatures(AppCodeHostParams $appC protected static function setConfigIfNotNull(MixedMap $testArgs, string $optName, AppCodeHostParams $appCodeParams): void { - $optVal = $testArgs->getNullableString($optName); + $optVal = $testArgs->get($optName); if ($optVal !== null) { + self::assertTrue(is_string($optVal) || is_int($optVal) || is_float($optVal) || is_bool($optVal)); + /** @var string|int|float|bool $optVal */ $appCodeParams->setAgentOption($optName, $optVal); } } diff --git a/tests/ElasticApmTests/ComponentTests/Util/TestCaseHandle.php b/tests/ElasticApmTests/ComponentTests/Util/TestCaseHandle.php index 5f6ea5c75..f46391bac 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/TestCaseHandle.php +++ b/tests/ElasticApmTests/ComponentTests/Util/TestCaseHandle.php @@ -156,6 +156,13 @@ function (HttpAppCodeHostParams $appCodeHostParams) use ($setParamsFunc): void { return $this->additionalHttpAppCodeHost; } + public function getDataFromAgentWithoutWaiting(): DataFromAgentPlusRaw + { + $dataFromAgentAccumulator = new DataFromAgentPlusRawAccumulator(); + $dataFromAgentAccumulator->addReceiverEvents($this->mockApmServer->fetchNewData(/* shouldWait */ false)); + return $dataFromAgentAccumulator->getAccumulatedData(); + } + public function waitForDataFromAgent(ExpectedEventCounts $expectedEventCounts, bool $shouldValidate = true): DataFromAgentPlusRaw { TestCaseBase::assertNotEmpty($this->appCodeInvocations); diff --git a/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php b/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php index 82e2c3589..6ecef8e6f 100644 --- a/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php +++ b/tests/ElasticApmTests/UnitTests/AgentEnabledConfigUnitTest.php @@ -33,7 +33,7 @@ final class AgentEnabledConfigUnitTest extends TestCaseBase { - public function test(): void + public function testDefaultAsSeenByPhpPart(): void { $configForTests = ConfigUtilForTests::read(/* additionalConfigSource */ null, NoopLoggerFactory::singletonInstance()); if ($configForTests->agentEnabledConfigDefault === null) { diff --git a/tests/ElasticApmTests/Util/MixedMap.php b/tests/ElasticApmTests/Util/MixedMap.php index 6815f8bff..75c9a0b3f 100644 --- a/tests/ElasticApmTests/Util/MixedMap.php +++ b/tests/ElasticApmTests/Util/MixedMap.php @@ -101,13 +101,33 @@ public function getIfKeyExistsElse(string $key, $fallbackValue) * @param string $key * @param array $from * - * @return bool + * @return ?bool */ - public static function getBoolFrom(string $key, array $from): bool + public static function getNullableBoolFrom(string $key, array $from): ?bool { AssertMessageStack::newScope(/* out */ $dbgCtx, AssertMessageStack::funcArgs()); $value = self::getFrom($key, $from); - TestCaseBase::assertIsBool($value); + if ($value !== null) { + TestCaseBase::assertIsBool($value); + } + return $value; + } + + public function getNullableBool(string $key): ?bool + { + return self::getNullableBoolFrom($key, $this->map); + } + + /** + * @param string $key + * @param array $from + * + * @return bool + */ + public static function getBoolFrom(string $key, array $from): bool + { + $value = self::getNullableBoolFrom($key, $from); + TestCaseBase::assertNotNull($value); return $value; } From adb43fcaa55abd01cf2954606640618cdad3dc0c Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 20:31:53 +0300 Subject: [PATCH 7/9] Fixed composer command line to run unit tests --- .ci/static-check-unit-test.sh | 5 +++-- .ci/validate_agent_installation.sh | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.ci/static-check-unit-test.sh b/.ci/static-check-unit-test.sh index d20ab80b0..9c4ada0c4 100755 --- a/.ci/static-check-unit-test.sh +++ b/.ci/static-check-unit-test.sh @@ -44,10 +44,11 @@ composer run-script static_check # Run unit tests composerCommandToRunUnitTests=(composer run-script --) phpUnitConfigFile=$(php ./tests/ElasticApmTests/Util/runSelectPhpUnitConfigFile.php --tests-type=unit) -composerCommandToRunUnitTests=("${composerCommandToRunUnitTests[@]}" run_unit_tests_custom_config -c "${phpUnitConfigFile}) +composerCommandToRunUnitTests=("${composerCommandToRunUnitTests[@]}" run_unit_tests_custom_config -c "${phpUnitConfigFile}") if [ "${ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT:?}" == "false" ]; then - composerCommandToRunUnitTests=("${composerCommandToRunUnitTests[@]}" --filter "AgentEnabledConfigUnitTest") + composerCommandToRunUnitTests=("${composerCommandToRunUnitTests[@]}" --filter AgentEnabledConfigUnitTest) fi +echo "composerCommandToRunUnitTests: ${composerCommandToRunUnitTests[@]}" "${composerCommandToRunUnitTests[@]}" ls -l ./build/unit-tests-phpunit-junit.xml diff --git a/.ci/validate_agent_installation.sh b/.ci/validate_agent_installation.sh index 49ce3a76d..23d6f7309 100755 --- a/.ci/validate_agent_installation.sh +++ b/.ci/validate_agent_installation.sh @@ -31,7 +31,7 @@ function runComponentTests () { composerCommand=("${composerCommand[@]}" --filter "${ELASTIC_APM_PHP_TESTS_FILTER}") fi else - composerCommand=("${composerCommand[@]}" --filter "AgentEnabledConfigComponentTest") + composerCommand=("${composerCommand[@]}" --filter AgentEnabledConfigComponentTest) fi local initialTimeoutInMinutes=30 From 4da951ae08857041551acbb860aaf3c9b3cfaa28 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Thu, 14 Sep 2023 21:53:35 +0300 Subject: [PATCH 8/9] Fixed .phpt tests --- agent/native/ext/tests/config_defaults.phpt | 8 +++++--- .../config_setting_to_invalid_values_using_env_vars.phpt | 8 +++++--- .../tests/config_setting_to_invalid_values_using_ini.phpt | 8 +++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/agent/native/ext/tests/config_defaults.phpt b/agent/native/ext/tests/config_defaults.phpt index d949cbaae..dbc3618c4 100644 --- a/agent/native/ext/tests/config_defaults.phpt +++ b/agent/native/ext/tests/config_defaults.phpt @@ -25,9 +25,11 @@ elasticApmAssertSame("getenv('ELASTIC_APM_ENABLED')", getenv('ELASTIC_APM_ENABLE elasticApmAssertEqual("ini_get('elastic_apm.enabled')", ini_get('elastic_apm.enabled'), false); -elasticApmAssertSame("elastic_apm_is_enabled()", elastic_apm_is_enabled(), true); - -elasticApmAssertSame("elastic_apm_get_config_option_by_name('enabled')", elastic_apm_get_config_option_by_name('enabled'), true); +if (($agentEnabledConfigDefaultEnvVar = getenv('ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT')) !== false) { + $agentEnabledConfigDefault = json_decode($agentEnabledConfigDefaultEnvVar); + elasticApmAssertSame("elastic_apm_is_enabled()", elastic_apm_is_enabled(), $agentEnabledConfigDefault); + elasticApmAssertSame("elastic_apm_get_config_option_by_name('enabled')", elastic_apm_get_config_option_by_name('enabled'), $agentEnabledConfigDefault); +} ////////////////////////////////////////////// /////////////// log_file diff --git a/agent/native/ext/tests/config_setting_to_invalid_values_using_env_vars.phpt b/agent/native/ext/tests/config_setting_to_invalid_values_using_env_vars.phpt index 78ee5e4bb..cdc65f351 100644 --- a/agent/native/ext/tests/config_setting_to_invalid_values_using_env_vars.phpt +++ b/agent/native/ext/tests/config_setting_to_invalid_values_using_env_vars.phpt @@ -19,9 +19,11 @@ require __DIR__ . '/../tests_util/tests_util.php'; elasticApmAssertSame("getenv('ELASTIC_APM_ENABLED')", getenv('ELASTIC_APM_ENABLED'), 'not_valid_boolean_value'); -elasticApmAssertSame("elastic_apm_is_enabled()", elastic_apm_is_enabled(), true); - -elasticApmAssertSame("elastic_apm_get_config_option_by_name('enabled')", elastic_apm_get_config_option_by_name('enabled'), true); +if (($agentEnabledConfigDefaultEnvVar = getenv('ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT')) !== false) { + $agentEnabledConfigDefault = json_decode($agentEnabledConfigDefaultEnvVar); + elasticApmAssertSame("elastic_apm_is_enabled()", elastic_apm_is_enabled(), $agentEnabledConfigDefault); + elasticApmAssertSame("elastic_apm_get_config_option_by_name('enabled')", elastic_apm_get_config_option_by_name('enabled'), $agentEnabledConfigDefault); +} ////////////////////////////////////////////// /////////////// assert_level diff --git a/agent/native/ext/tests/config_setting_to_invalid_values_using_ini.phpt b/agent/native/ext/tests/config_setting_to_invalid_values_using_ini.phpt index df8e03fb1..99321c964 100644 --- a/agent/native/ext/tests/config_setting_to_invalid_values_using_ini.phpt +++ b/agent/native/ext/tests/config_setting_to_invalid_values_using_ini.phpt @@ -19,9 +19,11 @@ require __DIR__ . '/../tests_util/tests_util.php'; elasticApmAssertEqual("ini_get('elastic_apm.enabled')", ini_get('elastic_apm.enabled'), 'not valid boolean value'); -elasticApmAssertSame("elastic_apm_get_config_option_by_name('enabled')", elastic_apm_get_config_option_by_name('enabled'), true); - -elasticApmAssertSame("elastic_apm_is_enabled()", elastic_apm_is_enabled(), true); +if (($agentEnabledConfigDefaultEnvVar = getenv('ELASTIC_APM_PHP_TESTS_AGENT_ENABLED_CONFIG_DEFAULT')) !== false) { + $agentEnabledConfigDefault = json_decode($agentEnabledConfigDefaultEnvVar); + elasticApmAssertSame("elastic_apm_is_enabled()", elastic_apm_is_enabled(), $agentEnabledConfigDefault); + elasticApmAssertSame("elastic_apm_get_config_option_by_name('enabled')", elastic_apm_get_config_option_by_name('enabled'), $agentEnabledConfigDefault); +} ////////////////////////////////////////////// /////////////// assert_level From 2f0839c2300edbd0e3de525c6d5fb71dfe689f5e Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Fri, 15 Sep 2023 10:54:00 +0300 Subject: [PATCH 9/9] Fixed formatting --- agent/native/ext/lifecycle.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index 373181112..873165c5c 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -504,8 +504,8 @@ void logImportantAgentInfo( const ConfigSnapshot* config, String calledFromFunc { ELASTIC_APM_LOG_INFO( "Custom build based on version: %s" - "; Custom changes: " - "- agent is disabled by default (enabled: false)" + "; Custom changes:" + " * agent is disabled by default (enabled: false)" "; config->enabled: %s." "; SAPI module name: %s" "; Called from: %s"