From 642c2937b78f01951565749ff9b6cefc7e1b24b5 Mon Sep 17 00:00:00 2001 From: Andrii Andreiev <129078694+AndriiAndreiev@users.noreply.github.com> Date: Wed, 4 Sep 2024 02:53:21 +0300 Subject: [PATCH] feat(dotnet, node, php, python): exclude OPTIONS requests from being logged (#1070) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit | 🚥 Resolves #58 | | :------------------- | ## 🧰 Changes Add a middleware checks for skipping OPTIONS requests --- packages/dotnet/ReadMe/Metrics.cs | 6 +++ packages/node/src/lib/log.ts | 1 + packages/node/test/lib/log.test.ts | 46 +++++++++++++++++++ packages/php/src/Metrics.php | 4 ++ packages/php/tests/MetricsTest.php | 23 ++++++++++ packages/python/readme_metrics/django.py | 3 ++ .../python/readme_metrics/flask_readme.py | 3 ++ .../readme_metrics/tests/django_test.py | 8 ++++ .../python/readme_metrics/tests/flask_test.py | 12 +++++ 9 files changed, 106 insertions(+) create mode 100644 packages/node/test/lib/log.test.ts diff --git a/packages/dotnet/ReadMe/Metrics.cs b/packages/dotnet/ReadMe/Metrics.cs index abfaae40a3..6bf275b585 100644 --- a/packages/dotnet/ReadMe/Metrics.cs +++ b/packages/dotnet/ReadMe/Metrics.cs @@ -21,6 +21,12 @@ public Metrics(RequestDelegate next, IConfiguration configuration) public async Task InvokeAsync(HttpContext context) { + if (context.Request.Method == HttpMethods.Options) + { + await this.next(context); + return; + } + if (!context.Request.Path.Value.Contains("favicon.ico")) { this.group = new Group() diff --git a/packages/node/src/lib/log.ts b/packages/node/src/lib/log.ts index 68a38fbc91..bf2d7d49ac 100644 --- a/packages/node/src/lib/log.ts +++ b/packages/node/src/lib/log.ts @@ -109,6 +109,7 @@ export function log( group: GroupingObject, options: Options = {}, ) { + if (req.method === 'OPTIONS') return undefined; if (!readmeApiKey) throw new Error('You must provide your ReadMe API key'); if (!group) throw new Error('You must provide a group'); if (options.logger) { diff --git a/packages/node/test/lib/log.test.ts b/packages/node/test/lib/log.test.ts new file mode 100644 index 0000000000..0875ba3f80 --- /dev/null +++ b/packages/node/test/lib/log.test.ts @@ -0,0 +1,46 @@ +import type { ExtendedIncomingMessage, ExtendedResponse, Options } from 'src/lib/log'; +import type { GroupingObject } from 'src/lib/metrics-log'; + +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; + +import { log } from '../../src/lib/log'; +import { metricsAPICall } from '../../src/lib/metrics-log'; + +describe('log', function () { + vi.mock('../../src/lib/metrics-log'); + + let req: ExtendedIncomingMessage; + let res: ExtendedResponse; + let group: GroupingObject; + let options: Options; + + beforeEach(() => { + req = { + method: 'GET', + headers: {}, + url: '/', + } as ExtendedIncomingMessage; + + res = { + statusCode: 200, + getHeader: vi.fn(), + setHeader: vi.fn(), + removeListener: vi.fn(), + once: vi.fn(), + } as unknown as ExtendedResponse; + + group = { apiKey: 'test-api-key' }; + options = { bufferLength: 1 }; + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should not log OPTIONS requests', function () { + req.method = 'OPTIONS'; + const logId = log('api-key', req, res, group, options); + expect(logId).toBeUndefined(); + expect(metricsAPICall).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/php/src/Metrics.php b/packages/php/src/Metrics.php index 59a5035f58..7f72806dc7 100644 --- a/packages/php/src/Metrics.php +++ b/packages/php/src/Metrics.php @@ -92,6 +92,10 @@ public function __construct(public string $api_key, public string $group_handler */ public function track(Request $request, Response &$response): void { + if ($request->getMethod() === 'OPTIONS') { + return; + } + if (empty($this->base_log_url)) { $this->base_log_url = $this->getProjectBaseUrl(); } diff --git a/packages/php/tests/MetricsTest.php b/packages/php/tests/MetricsTest.php index 8aaf4bb902..61dab32c5b 100644 --- a/packages/php/tests/MetricsTest.php +++ b/packages/php/tests/MetricsTest.php @@ -227,6 +227,29 @@ public function testTrackHandlesApiServerUnavailability(bool $development_mode): } } + /** + * @group track + */ + public function testTrackIgnoresOptionsMethod(): void + { + $request = new Request([], [], [], [], [], ['REQUEST_METHOD' => 'OPTIONS']); + $response = $this->getMockJsonResponse(); + + $handlers = $this->getMockHandlers( + new \GuzzleHttp\Psr7\Response(200, [], 'OK'), + new \GuzzleHttp\Psr7\Response(200, [], json_encode(['baseUrl' => $this->base_log_url])) + ); + + $this->metrics = new Metrics($this->readme_api_key, $this->group_handler, [ + 'development_mode' => false, + 'client' => new Client(['handler' => $handlers->metrics]), + 'client_readme' => new Client(['handler' => $handlers->readme]) + ]); + + $this->metrics->track($request, $response); + $this->assertEmpty($this->api_calls, 'No API calls should be made for OPTIONS requests.'); + } + /** * @group getProjectBaseUrl * @dataProvider providerDevelopmentModeToggle diff --git a/packages/python/readme_metrics/django.py b/packages/python/readme_metrics/django.py index 8a42609e9f..2878a7034d 100644 --- a/packages/python/readme_metrics/django.py +++ b/packages/python/readme_metrics/django.py @@ -16,6 +16,9 @@ def __init__(self, get_response, config=None): self.metrics_core = Metrics(self.config) def __call__(self, request): + if request.method == "OPTIONS": + return self.get_response(request) + try: request.rm_start_dt = datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ") request.rm_start_ts = int(time.time() * 1000) diff --git a/packages/python/readme_metrics/flask_readme.py b/packages/python/readme_metrics/flask_readme.py index 4cb6471421..78da6c23b1 100644 --- a/packages/python/readme_metrics/flask_readme.py +++ b/packages/python/readme_metrics/flask_readme.py @@ -27,6 +27,9 @@ def init_app(self, app: Flask): app.after_request(self.after_request) def before_request(self): + if request.method == "OPTIONS": + return + try: request.rm_start_dt = datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ") request.rm_start_ts = int(time.time() * 1000) diff --git a/packages/python/readme_metrics/tests/django_test.py b/packages/python/readme_metrics/tests/django_test.py index 9869286c4f..603f56ab89 100644 --- a/packages/python/readme_metrics/tests/django_test.py +++ b/packages/python/readme_metrics/tests/django_test.py @@ -63,3 +63,11 @@ def test_missing_content_length(self): request.headers = {} middleware(request) assert getattr(request, "rm_content_length") == "0" + + def test_options_request(self): + middleware = MetricsMiddleware(Mock(), config=mock_config) + middleware.metrics_core = Mock() + request = Mock() + request.method = "OPTIONS" + middleware(request) + assert not middleware.metrics_core.process.called diff --git a/packages/python/readme_metrics/tests/flask_test.py b/packages/python/readme_metrics/tests/flask_test.py index 67ab84084b..accd802608 100644 --- a/packages/python/readme_metrics/tests/flask_test.py +++ b/packages/python/readme_metrics/tests/flask_test.py @@ -69,3 +69,15 @@ def test_after_request(self): assert call_args[0][0] == request assert isinstance(call_args[0][1], ResponseInfoWrapper) assert call_args[0][1].headers.get("X-Header") == "X Value!" + + def test_before_request_options(self): + app = Flask(__name__) + extension = ReadMeMetrics(config=mock_config, app=app) + + with app.test_request_context("/", method="OPTIONS"): + extension.before_request() + + assert not hasattr(request, "rm_start_dt") + assert not hasattr(request, "rm_start_ts") + assert not hasattr(request, "rm_content_length") + assert not hasattr(request, "rm_body")