From 707e776d89bc5679e29cba0697486db45fc18f66 Mon Sep 17 00:00:00 2001 From: Chris Schneider Date: Mon, 19 Aug 2019 13:27:55 -0600 Subject: [PATCH 1/3] Add Ignore setting --- src/Agent.php | 29 +++++++++++++++++++-- src/Config.php | 1 + src/IgnoredEndpoints.php | 35 +++++++++++++++++++++++++ tests/IgnoredEndpointsTest.php | 47 ++++++++++++++++++++++++++++++++++ tests/Unit/AgentTest.php | 9 +++++++ tests/Unit/ConfigTest.php | 9 +++++++ 6 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 src/IgnoredEndpoints.php create mode 100644 tests/IgnoredEndpointsTest.php diff --git a/src/Agent.php b/src/Agent.php index ee2c70b3..f7b42950 100644 --- a/src/Agent.php +++ b/src/Agent.php @@ -12,7 +12,7 @@ class Agent { const VERSION = '1.0'; - const NAME = 'scoutapm-php'; + const NAME = 'scout-apm-php'; private $config; @@ -23,11 +23,20 @@ class Agent private $logger; + // Class that helps check urls vs. the configured ignore list + private $ignoredEndpoints; + + // If this request was marked as ignored + private $ignored; + public function __construct() { $this->config = new Config($this); $this->request = new Request($this); $this->logger = new NullLogger(); + + $this->ignoredEndpoints = new IgnoredEndpoints($this); + $this->ignored = false; } public function connect() @@ -137,14 +146,30 @@ public function tagRequest(string $tag, $value) return $this->request->tag($tag, $value); } + public function ignored(string $url) : bool + { + return $this->ignoredEndpoints->ignored($url); + } + + public function ignore() + { + $this->ignored = true; + } + public function send() : bool { + // Don't send if the agent is not enabled. if (! $this->enabled()) { return true; } - $status = $this->connector->sendRequest($this->request); + // Don't send it if the request was ignored + if ($this->ignored()) { + return true; + } + // Send this request off to the CoreAgent + $status = $this->connector->sendRequest($this->request); return $status; } diff --git a/src/Config.php b/src/Config.php index 5ae0c591..3322be54 100644 --- a/src/Config.php +++ b/src/Config.php @@ -28,6 +28,7 @@ public function __construct(\Scoutapm\Agent $agent) $this->coercions = [ "monitor" => Config\BoolCoercion::class, + "ignore" => Config\JSONCoercion::class, ]; } diff --git a/src/IgnoredEndpoints.php b/src/IgnoredEndpoints.php new file mode 100644 index 00000000..1308d5ea --- /dev/null +++ b/src/IgnoredEndpoints.php @@ -0,0 +1,35 @@ +agent = $agent; + $this->config = $agent->getConfig(); + } + + public function ignored(string $url) : bool + { + $ignored = $this->config->get("ignore"); + if ($ignored == null) { + return false; + } + + foreach ($ignored as $ignore) { + if (substr($url, 0, strlen($ignore)) === $ignore) { + return true; + } + } + + // None Matched + return false; + } +} diff --git a/tests/IgnoredEndpointsTest.php b/tests/IgnoredEndpointsTest.php new file mode 100644 index 00000000..9c581134 --- /dev/null +++ b/tests/IgnoredEndpointsTest.php @@ -0,0 +1,47 @@ +getConfig(); + $config->set("ignore", [ + "/health", + "/status", + ]); + $ignoredEndpoints = new IgnoredEndpoints($agent); + + // Exact Match + $this->assertEquals(true, $ignoredEndpoints->ignored("/health")); + $this->assertEquals(true, $ignoredEndpoints->ignored("/status")); + + // Prefix Match + $this->assertEquals(true, $ignoredEndpoints->ignored("/health/database")); + $this->assertEquals(true, $ignoredEndpoints->ignored("/status/time")); + + // No Match + $this->assertEquals(false, $ignoredEndpoints->ignored("/signup")); + + // Not-prefix doesn't Match + $this->assertEquals(false, $ignoredEndpoints->ignored("/hero/1/health")); + } + + public function testWorksWithNullIgnoreSetting() + { + $agent = new Agent(); + $config = $agent->getConfig(); + $ignoredEndpoints = new IgnoredEndpoints($agent); + + // No Match + $this->assertEquals(false, $ignoredEndpoints->ignored("/signup")); + } +} diff --git a/tests/Unit/AgentTest.php b/tests/Unit/AgentTest.php index ce8d43c4..9de28ee9 100644 --- a/tests/Unit/AgentTest.php +++ b/tests/Unit/AgentTest.php @@ -148,4 +148,13 @@ public function testEnabled() $this->assertEquals(true, $agent->enabled()); } + + public function testIgnoredEndpoints() + { + $agent = new Agent(); + $agent->getConfig()->set("ignore", ["/foo"]); + + $this->assertEquals(true, $agent->ignored("/foo")); + $this->assertEquals(false, $agent->ignored("/bar")); + } } diff --git a/tests/Unit/ConfigTest.php b/tests/Unit/ConfigTest.php index bfead0b6..f5f6e07e 100644 --- a/tests/Unit/ConfigTest.php +++ b/tests/Unit/ConfigTest.php @@ -47,4 +47,13 @@ public function testBooleanCoercionOfMonitor() $config->set("monitor", "true"); $this->assertSame(true, $config->get("monitor")); } + + public function testJSONCoercionOfIgnore() + { + $config = new Config(new Agent()); + + // Set a user config. This won't be looked up + $config->set("ignore", '["/foo", "/bar"]'); + $this->assertSame(["/foo", "/bar"], $config->get("ignore")); + } } From 2fca75f97f85c36537d1650a43bdf91a82c66c27 Mon Sep 17 00:00:00 2001 From: Chris Schneider Date: Mon, 19 Aug 2019 14:17:06 -0600 Subject: [PATCH 2/3] NOOP further instrumentation after ignoring a request --- src/Agent.php | 36 +++++++++++++++++++++++++++++++----- tests/Unit/AgentTest.php | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/Agent.php b/src/Agent.php index f7b42950..97d4d2a9 100644 --- a/src/Agent.php +++ b/src/Agent.php @@ -23,7 +23,7 @@ class Agent private $logger; - // Class that helps check urls vs. the configured ignore list + // Class that helps check incoming http paths vs. the configured ignore list private $ignoredEndpoints; // If this request was marked as ignored @@ -107,11 +107,21 @@ public function getConfig() : Config */ public function startSpan(string $operation, float $overrideTimestamp = null) { + if ($this->request === null) { + // Must return a Span object to match API. This is a dummy span + // that is not ever used for anything. + return new Span($this, "Ignored", "ignored-request"); + } + return $this->request->startSpan($operation, $overrideTimestamp); } public function stopSpan() { + if ($this->request === null) { + return null; + } + $this->request->stopSpan(); } @@ -143,29 +153,45 @@ public function addContext(string $tag, $value) public function tagRequest(string $tag, $value) { + if ($this->request === null) { + return null; + } + return $this->request->tag($tag, $value); } - public function ignored(string $url) : bool + /* + * Check if a given URL was configured as ignored. + * Does not alter the running request. If you wish to abort tracing of this + * request, use ignore() + */ + public function ignored(string $path) : bool { - return $this->ignoredEndpoints->ignored($url); + return $this->ignoredEndpoints->ignored($path); } + /* + * Mark the running request as ignored. Triggers optimizations in various + * tracing and tagging methods to turn them into NOOPs + */ public function ignore() { + $this->request = null; $this->ignored = true; } + // Returns true only if the request was sent onward to the core agent. + // False otherwise. public function send() : bool { // Don't send if the agent is not enabled. if (! $this->enabled()) { - return true; + return false; } // Don't send it if the request was ignored if ($this->ignored()) { - return true; + return false; } // Send this request off to the CoreAgent diff --git a/tests/Unit/AgentTest.php b/tests/Unit/AgentTest.php index 9de28ee9..2a467ddb 100644 --- a/tests/Unit/AgentTest.php +++ b/tests/Unit/AgentTest.php @@ -157,4 +157,34 @@ public function testIgnoredEndpoints() $this->assertEquals(true, $agent->ignored("/foo")); $this->assertEquals(false, $agent->ignored("/bar")); } + + // Many instrumentation calls are NOOPs when ignore is called. Make sure + // the sequence works as expected + public function testIgnoredAgentSequence() + { + $agent = new Agent(); + $agent->ignore(); + + // Start a Parent Controller Span + $span = $agent->startSpan("Controller/Test"); + + // Tag Whole Request + $agent->tagRequest("uri", "example.com/foo/bar.php"); + + // Start a Child Span + $span = $agent->startSpan("SQL/Query"); + + // Tag the span + $span->tag("sql.query", "select * from foo"); + + // Finish Child Span + $agent->stopSpan(); + + // Stop Controller Span + $agent->stopSpan(); + + $agent->send(); + + $this->assertNotNull($agent); + } } From 518f6de22894be1c776ea28ce9e0c0fdd65622c1 Mon Sep 17 00:00:00 2001 From: Chris Schneider Date: Mon, 19 Aug 2019 14:51:06 -0600 Subject: [PATCH 3/3] Rename ignored to isIgnored for clarity --- src/Agent.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Agent.php b/src/Agent.php index 97d4d2a9..dfd3b26f 100644 --- a/src/Agent.php +++ b/src/Agent.php @@ -27,7 +27,7 @@ class Agent private $ignoredEndpoints; // If this request was marked as ignored - private $ignored; + private $isIgnored; public function __construct() { @@ -36,7 +36,7 @@ public function __construct() $this->logger = new NullLogger(); $this->ignoredEndpoints = new IgnoredEndpoints($this); - $this->ignored = false; + $this->isIgnored = false; } public function connect() @@ -177,7 +177,7 @@ public function ignored(string $path) : bool public function ignore() { $this->request = null; - $this->ignored = true; + $this->isIgnored = true; } // Returns true only if the request was sent onward to the core agent. @@ -190,7 +190,7 @@ public function send() : bool } // Don't send it if the request was ignored - if ($this->ignored()) { + if ($this->isIgnored) { return false; }