Skip to content

Commit

Permalink
feat(dotnet, node, php, python): exclude OPTIONS requests from being …
Browse files Browse the repository at this point in the history
…logged (#1070)

| 🚥 Resolves #58  |
| :------------------- |

## 🧰 Changes

Add a middleware checks for skipping OPTIONS requests
  • Loading branch information
AndriiAndreiev authored Sep 3, 2024
1 parent b073f7d commit 642c293
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 0 deletions.
6 changes: 6 additions & 0 deletions packages/dotnet/ReadMe/Metrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions packages/node/src/lib/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
46 changes: 46 additions & 0 deletions packages/node/test/lib/log.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
4 changes: 4 additions & 0 deletions packages/php/src/Metrics.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
23 changes: 23 additions & 0 deletions packages/php/tests/MetricsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/python/readme_metrics/django.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions packages/python/readme_metrics/flask_readme.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions packages/python/readme_metrics/tests/django_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 12 additions & 0 deletions packages/python/readme_metrics/tests/flask_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

0 comments on commit 642c293

Please sign in to comment.