From 0d985742fa4e619946c42a8c9e8adacf7b1bc47b Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Mon, 11 Sep 2023 13:55:10 +0300 Subject: [PATCH 01/10] For custom build change ast_process_enabled 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..fbf664a32 100644 --- a/agent/native/ext/ConfigManager.cpp +++ b/agent/native/ext/ConfigManager.cpp @@ -962,7 +962,7 @@ static void initOptionsMetadata( OptionMetadata* optsMeta ) buildBoolOptionMetadata, astProcessEnabled, ELASTIC_APM_CFG_OPT_NAME_AST_PROCESS_ENABLED, - /* defaultValue: */ true ); + /* defaultValue: */ false ); ELASTIC_APM_INIT_METADATA( buildBoolOptionMetadata, From ff0415951378747957194d5b13632a2abe98741d Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Mon, 11 Sep 2023 13:56:52 +0300 Subject: [PATCH 02/10] For custom build change stack_trace_min_durationdefault to -1 --- agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php b/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php index b45bb2adb..c1703ff97 100644 --- a/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php +++ b/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php @@ -34,7 +34,7 @@ final class OptionDefaultValues { use StaticClassTrait; - public const SPAN_STACK_TRACE_MIN_DURATION = 5; + public const SPAN_STACK_TRACE_MIN_DURATION = -1; public const STACK_TRACE_LIMIT = 50; public const TRANSACTION_MAX_SPANS = 500; } From 5291275fb08f80c93bf572d20b4189d5c7ca0868 Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Mon, 11 Sep 2023 13:57:27 +0300 Subject: [PATCH 03/10] For custom build added log statement indicating that it's a custom build --- agent/native/ext/lifecycle.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index bb0429527..07b2351be 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -526,6 +526,11 @@ void elasticApmModuleInit( int moduleType, int moduleNumber ) ELASTIC_APM_CALL_IF_FAILED_GOTO( ensureLoggerInitialConfigIsLatest( tracer ) ); ELASTIC_APM_CALL_IF_FAILED_GOTO( ensureAllComponentsHaveLatestConfig( tracer ) ); + ELASTIC_APM_LOG_INFO( + "Custom build (based on version: " PHP_ELASTIC_APM_VERSION ") with " + "- span stack trace disabled by default (stack_trace_min_duration: -1)" + "- WordPress instrumentation disabled by default (ast_process_enabled: false)" + ); logSupportabilityInfo( logLevel_debug ); config = getTracerCurrentConfigSnapshot( tracer ); From 1ee49af2910306c2a7b8b9e0d8f7185e966f0d3a Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Mon, 11 Sep 2023 14:25:37 +0300 Subject: [PATCH 04/10] Fixed unit tests assuming that span stack trace is captured by default --- .../UnitTests/InferredSpansBuilderTest.php | 51 ++++++++++++------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/tests/ElasticApmTests/UnitTests/InferredSpansBuilderTest.php b/tests/ElasticApmTests/UnitTests/InferredSpansBuilderTest.php index bfacb222a..e892d629f 100644 --- a/tests/ElasticApmTests/UnitTests/InferredSpansBuilderTest.php +++ b/tests/ElasticApmTests/UnitTests/InferredSpansBuilderTest.php @@ -143,6 +143,17 @@ private function helperForTestOneStackTrace(InferredSpansBuilder $builder): arra return $stackTrace; } + public function isSpanStackTraceEnabled(SpanDto $span): bool + { + $dummyTx = $this->tracer->beginTransaction('dummy_temp_TX_name', 'dummy_temp_TX_type'); + $result = + $dummyTx instanceof Transaction + && $dummyTx->shouldCollectStackTraceForSpanDuration($span->duration) + && (StackTraceUtil::convertLimitConfigToMaxNumberOfFrames($dummyTx->getStackTraceLimitConfig()) !== 0); + $dummyTx->discard(); + return $result; + } + public function testOneStackTrace(): void { AssertMessageStack::newScope(/* out */ $dbgCtx); @@ -190,8 +201,12 @@ function (InferredSpansBuilder $builder) use (&$expectedStackTrace, $expectedTim TraceValidator::validate(new TraceActual($this->mockEventSink->idToTransaction(), $this->mockEventSink->idToSpan())); $expectedStackTraceConvertedToApm = StackTraceUtil::convertClassicToApmFormat($expectedStackTrace, /* maxNumberOfFrames */ null); - self::assertNotNull($span->stackTrace); - StackTraceExpectations::fromFrames($expectedStackTraceConvertedToApm)->assertMatches($span->stackTrace); + if ($this->isSpanStackTraceEnabled($span)) { + self::assertNotNull($span->stackTrace); + StackTraceExpectations::fromFrames($expectedStackTraceConvertedToApm)->assertMatches($span->stackTrace); + } else { + self::assertNull($span->stackTrace); + } } private function charDiagramFuncNameToStackTraceFrame(string $funcName): ClassicFormatStackTraceFrame @@ -1358,22 +1373,24 @@ function (InferredSpansBuilder $builder) use ($stackDepthVariants): void { TestCaseBase::assertNull($span->stackTrace); continue; } - TestCaseBase::assertNotNull($span->stackTrace); - - if ($expectedMaxNumberOfFrames !== null) { - TestCaseBase::assertLessThanOrEqual($expectedMaxNumberOfFrames, count($span->stackTrace)); - } - - $dbgCtx->pushSubScope(); - foreach (RangeUtil::generateUpTo(count($span->stackTrace)) as $stackFrameIndex) { - $dbgCtx->add(['stackFrameIndex' => $stackFrameIndex]); - $currentExpectedClassicFormatFrame = self::genFrameForSpanWithStackDepth($spanIndex, $stackFrameIndex, $stackDepth); - $dbgCtx->add(['currentExpectedClassicFormatFrame' => $currentExpectedClassicFormatFrame]); - $prevExpectedClassicFormatFrame = $stackFrameIndex === 0 ? null : self::genFrameForSpanWithStackDepth($spanIndex, $stackFrameIndex - 1, $stackDepth); - $dbgCtx->add(['prevExpectedClassicFormatFrame' => $prevExpectedClassicFormatFrame]); - StackTraceFrameExpectations::fromClassicFormat($currentExpectedClassicFormatFrame, $prevExpectedClassicFormatFrame)->assertMatches($span->stackTrace[$stackFrameIndex]); + if ($this->isSpanStackTraceEnabled($span)) { + self::assertNotNull($span->stackTrace); + if ($expectedMaxNumberOfFrames !== null) { + TestCaseBase::assertLessThanOrEqual($expectedMaxNumberOfFrames, count($span->stackTrace)); + } + $dbgCtx->pushSubScope(); + foreach (RangeUtil::generateUpTo(count($span->stackTrace)) as $stackFrameIndex) { + $dbgCtx->add(['stackFrameIndex' => $stackFrameIndex]); + $currentExpectedClassicFormatFrame = self::genFrameForSpanWithStackDepth($spanIndex, $stackFrameIndex, $stackDepth); + $dbgCtx->add(['currentExpectedClassicFormatFrame' => $currentExpectedClassicFormatFrame]); + $prevExpectedClassicFormatFrame = $stackFrameIndex === 0 ? null : self::genFrameForSpanWithStackDepth($spanIndex, $stackFrameIndex - 1, $stackDepth); + $dbgCtx->add(['prevExpectedClassicFormatFrame' => $prevExpectedClassicFormatFrame]); + StackTraceFrameExpectations::fromClassicFormat($currentExpectedClassicFormatFrame, $prevExpectedClassicFormatFrame)->assertMatches($span->stackTrace[$stackFrameIndex]); + } + $dbgCtx->popSubScope(); + } else { + self::assertNull($span->stackTrace); } - $dbgCtx->popSubScope(); } $dbgCtx->popSubScope(); } From 9d278d6fae4d315053db462cf2bc63d71bd59835 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Mon, 11 Sep 2023 15:18:49 +0300 Subject: [PATCH 05/10] Fixed component tests assuming ast_process_enabled is true --- agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php | 2 +- agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php | 1 + .../ComponentTests/Util/ComponentTestCaseBase.php | 7 +++++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php b/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php index e3bd9ecd3..b343dfa8f 100644 --- a/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php +++ b/agent/php/ElasticApm/Impl/Config/AllOptionsMetadata.php @@ -82,7 +82,7 @@ public static function get(): array /** @var array> $value */ $value = [ OptionNames::API_KEY => new NullableStringOptionMetadata(), - OptionNames::AST_PROCESS_ENABLED => new BoolOptionMetadata(/* defaultValue: */ true), + OptionNames::AST_PROCESS_ENABLED => new BoolOptionMetadata(OptionDefaultValues::AST_PROCESS_ENABLED), OptionNames::AST_PROCESS_DEBUG_DUMP_CONVERTED_BACK_TO_SOURCE => new BoolOptionMetadata(/* defaultValue: */ true), OptionNames::AST_PROCESS_DEBUG_DUMP_FOR_PATH_PREFIX => new NullableStringOptionMetadata(), diff --git a/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php b/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php index c1703ff97..2a491c131 100644 --- a/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php +++ b/agent/php/ElasticApm/Impl/Config/OptionDefaultValues.php @@ -34,6 +34,7 @@ final class OptionDefaultValues { use StaticClassTrait; + public const AST_PROCESS_ENABLED = false; public const SPAN_STACK_TRACE_MIN_DURATION = -1; public const STACK_TRACE_LIMIT = 50; public const TRANSACTION_MAX_SPANS = 500; diff --git a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php index c39134f6e..f219e918d 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php @@ -24,6 +24,7 @@ namespace ElasticApmTests\ComponentTests\Util; use Elastic\Apm\Impl\AutoInstrument\AutoInstrumentationBase; +use Elastic\Apm\Impl\Config\OptionDefaultValues; use Elastic\Apm\Impl\Config\OptionNames; use Elastic\Apm\Impl\GlobalTracerHolder; use Elastic\Apm\Impl\Log\Level as LogLevel; @@ -184,6 +185,7 @@ protected function verifyOneTransactionNoSpans(DataFromAgent $dataFromAgent): Tr /** * @param class-string $instrClassName * @param string[] $expectedNames + * @param bool $isEnabledByDefault * * @return void */ @@ -197,7 +199,8 @@ protected static function implTestIsAutoInstrumentationEnabled(string $instrClas $actualNames = $instr->keywords(); $actualNames[] = $instr->name(); self::assertEqualAsSets($expectedNames, $actualNames); - self::assertTrue($instr->isEnabled()); + $isEnabledByDefault = OptionDefaultValues::AST_PROCESS_ENABLED || (!$instr->requiresUserlandCodeInstrumentation()); + self::assertSame($isEnabledByDefault, $instr->isEnabled()); /** * @param string $name @@ -238,7 +241,7 @@ protected static function implTestIsAutoInstrumentationEnabled(string $instrClas $dbgCtx->clearCurrentSubScope(['disableInstrumentationsOptVal' => $disableInstrumentationsOptVal]); $tracer = self::buildTracerForTests()->withConfig(OptionNames::DISABLE_INSTRUMENTATIONS, $disableInstrumentationsOptVal)->build(); $instr = new $instrClassName($tracer); - self::assertTrue($instr->isEnabled()); + self::assertSame($isEnabledByDefault, $instr->isEnabled()); } $dbgCtx->popSubScope(); From 4c297b7ed9f3cceb2d45cdb2888bc7c019904a80 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Mon, 11 Sep 2023 15:31:36 +0300 Subject: [PATCH 06/10] Fixed static analysis issue --- .../ComponentTests/Util/ComponentTestCaseBase.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php index f219e918d..2390115c0 100644 --- a/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php +++ b/tests/ElasticApmTests/ComponentTests/Util/ComponentTestCaseBase.php @@ -185,7 +185,6 @@ protected function verifyOneTransactionNoSpans(DataFromAgent $dataFromAgent): Tr /** * @param class-string $instrClassName * @param string[] $expectedNames - * @param bool $isEnabledByDefault * * @return void */ @@ -199,7 +198,7 @@ protected static function implTestIsAutoInstrumentationEnabled(string $instrClas $actualNames = $instr->keywords(); $actualNames[] = $instr->name(); self::assertEqualAsSets($expectedNames, $actualNames); - $isEnabledByDefault = OptionDefaultValues::AST_PROCESS_ENABLED || (!$instr->requiresUserlandCodeInstrumentation()); + $isEnabledByDefault = OptionDefaultValues::AST_PROCESS_ENABLED || (!$instr->requiresUserlandCodeInstrumentation()); // @phpstan-ignore-line self::assertSame($isEnabledByDefault, $instr->isEnabled()); /** From a4f38dfedf61c3fd0441e0302056f685ca20f1f2 Mon Sep 17 00:00:00 2001 From: Sergey Kleyman Date: Mon, 11 Sep 2023 15:51:40 +0300 Subject: [PATCH 07/10] Log value of enabled configuration --- agent/native/ext/lifecycle.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index 07b2351be..e4f8df3bf 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -526,15 +526,19 @@ void elasticApmModuleInit( int moduleType, int moduleNumber ) ELASTIC_APM_CALL_IF_FAILED_GOTO( ensureLoggerInitialConfigIsLatest( tracer ) ); ELASTIC_APM_CALL_IF_FAILED_GOTO( ensureAllComponentsHaveLatestConfig( tracer ) ); + config = getTracerCurrentConfigSnapshot( tracer ); + ELASTIC_APM_LOG_INFO( - "Custom build (based on version: " PHP_ELASTIC_APM_VERSION ") with " - "- span stack trace disabled by default (stack_trace_min_duration: -1)" - "- WordPress instrumentation disabled by default (ast_process_enabled: false)" + "Custom build based on version: %s." + " Enabled: %s." + " Custom changes: " + "- span stack trace disabled by default (stack_trace_min_duration: -1)" + "- WordPress instrumentation disabled by default (ast_process_enabled: false)" + , PHP_ELASTIC_APM_VERSION + , boolToString(config->enabled) ); logSupportabilityInfo( logLevel_debug ); - config = getTracerCurrentConfigSnapshot( tracer ); - if ( ! config->enabled ) { resultCode = resultSuccess; From 51156d480975c641419ca3c6086cc6610391291b Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Fri, 15 Sep 2023 08:14:25 +0300 Subject: [PATCH 08/10] Log SAPI module name --- agent/native/ext/lifecycle.cpp | 29 ++++++++++++++++++++--------- agent/native/ext/util_for_PHP.cpp | 5 +++++ agent/native/ext/util_for_PHP.h | 1 + 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index e4f8df3bf..4cc0ead3e 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -500,6 +500,23 @@ static void unregisterErrorAndExceptionHooks() { } +void logImportantAgentInfo( const ConfigSnapshot* config, String calledFromFunc ) +{ + ELASTIC_APM_LOG_INFO( + "Custom build based on version: %s" + "; Custom changes: " + "- span stack trace disabled by default (stack_trace_min_duration: -1)" + "- WordPress instrumentation disabled by default (ast_process_enabled: false)" + "; config->enabled: %s." + "; SAPI module name: %s" + "; Called from: %s" + , PHP_ELASTIC_APM_VERSION + , boolToString( config->enabled ) + , getPhpSapiModuleName() + , 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()) ); @@ -528,15 +545,7 @@ void elasticApmModuleInit( int moduleType, int moduleNumber ) config = getTracerCurrentConfigSnapshot( tracer ); - ELASTIC_APM_LOG_INFO( - "Custom build based on version: %s." - " Enabled: %s." - " Custom changes: " - "- span stack trace disabled by default (stack_trace_min_duration: -1)" - "- WordPress instrumentation disabled by default (ast_process_enabled: false)" - , PHP_ELASTIC_APM_VERSION - , boolToString(config->enabled) - ); + logImportantAgentInfo( config, __FUNCTION__ ); logSupportabilityInfo( logLevel_debug ); if ( ! config->enabled ) @@ -589,6 +598,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; 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(); From eef8976b758c7f13400c565f4335c725edeecb94 Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Fri, 15 Sep 2023 10:53:04 +0300 Subject: [PATCH 09/10] 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 4cc0ead3e..7f149c394 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -505,8 +505,8 @@ void logImportantAgentInfo( const ConfigSnapshot* config, String calledFromFunc ELASTIC_APM_LOG_INFO( "Custom build based on version: %s" "; Custom changes: " - "- span stack trace disabled by default (stack_trace_min_duration: -1)" - "- WordPress instrumentation disabled by default (ast_process_enabled: false)" + " * span stack trace disabled by default (stack_trace_min_duration: -1)" + " * WordPress instrumentation disabled by default (ast_process_enabled: false)" "; config->enabled: %s." "; SAPI module name: %s" "; Called from: %s" From 45b2e3f233e1b1a22196e74cc682ad36a91644d5 Mon Sep 17 00:00:00 2001 From: SergeyKleyman Date: Fri, 15 Sep 2023 10:54:22 +0300 Subject: [PATCH 10/10] Fixed formatting --- agent/native/ext/lifecycle.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/native/ext/lifecycle.cpp b/agent/native/ext/lifecycle.cpp index 7f149c394..6d115590d 100644 --- a/agent/native/ext/lifecycle.cpp +++ b/agent/native/ext/lifecycle.cpp @@ -504,7 +504,7 @@ void logImportantAgentInfo( const ConfigSnapshot* config, String calledFromFunc { ELASTIC_APM_LOG_INFO( "Custom build based on version: %s" - "; Custom changes: " + "; Custom changes:" " * span stack trace disabled by default (stack_trace_min_duration: -1)" " * WordPress instrumentation disabled by default (ast_process_enabled: false)" "; config->enabled: %s."