Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access log can exclude defined regular expression patterns #6753

Merged
merged 9 commits into from
Jan 24, 2022

Conversation

timyates
Copy link
Member

This change allows the user to specify a list of regular expression patterns that are compiled
and matched against the current request URI.

If the request URI matches one of them, then the request does not appear in the access log.

This change allows the user to specify a list of regular expression patterns that are compiled
and matched against the current request URI.

If the request URI matches one of them, then the request does not appear in the access log.
@timyates timyates added this to the 3.3.0 milestone Jan 13, 2022
@timyates timyates self-assigned this Jan 13, 2022
@timyates timyates linked an issue Jan 13, 2022 that may be closed by this pull request
4 tasks
timyates added a commit that referenced this pull request Jan 17, 2022
This change allows configuration of a list of regular-expression patterns for tracing.

If the current path matches one of these patterns, then tracing is not performed on that
request.

Follows the same config naming strategy as #6753
timyates added a commit to micronaut-projects/micronaut-tracing that referenced this pull request Jan 18, 2022
This change allows configuration of a list of regular-expression patterns for tracing.

If the current path matches one of these patterns, then tracing is not performed on that
request.

Follows the same config naming strategy as the exclusion of access logs entries in

micronaut-projects/micronaut-core#6753
} else {
protocol = request.protocolVersion().text();
accessLog.exclude();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@graemerocher Would it be safe here to remove the handler from the pipeline? Setting an excluded flag and checking it is extra overhead

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea!

                ctx.pipeline().remove(this);

Seems to work in the test... Checking, and if there's no issues I can find I'll push

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Load tested it with a filtered url and a non-filtered url

ab -k -n 100000 -r -c 5 http://localhost:8080/path/one & ab -k -r -n 100000 -c 5 http://localhost:8080/something

And we didn't get any errors. I'm not a Netty-guru (yet) though, so @graemerocher may know if this is a dangerous thing to be doing

Anyway, pushed the change 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so that doesn't work with keep-alive connections (all logs get dropped after the first excluded one removes the handler from the pipeline)

Reverted back to the field

@yawkat yawkat self-requested a review January 20, 2022 15:53
@@ -55,6 +57,7 @@

private final Logger logger;
private final AccessLogFormatParser accessLogFormatParser;
private final Predicate<String> uriInclusion;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nullable

@yawkat
Copy link
Member

yawkat commented Jan 21, 2022

@jameskleeh can you merge this before monday so i can fix up #6785 in time for 3.3?

@jameskleeh
Copy link
Contributor

@yawkat Yeah I wasn't sure if you wanted to make the edits on this PR or not. If you're good with it I'll merge

@yawkat
Copy link
Member

yawkat commented Jan 24, 2022

@jameskleeh i don't have any changes to make to this PR. I've put them all in #6785, because they're unrelated to the exclusions feature.

@jameskleeh jameskleeh merged commit 35b03d2 into 3.3.x Jan 24, 2022
@jameskleeh jameskleeh deleted the access-log-exclusions branch January 24, 2022 15:59
burtbeckwith pushed a commit to micronaut-projects/micronaut-tracing that referenced this pull request Jan 25, 2022
This change allows configuration of a list of regular-expression patterns for tracing.

If the current path matches one of these patterns, then tracing is not performed on that
request.

Follows the same config naming strategy as the exclusion of access logs entries in

micronaut-projects/micronaut-core#6753
yawkat pushed a commit that referenced this pull request Jun 24, 2022
* Access log can exclude defined regular expression patterns

This change allows the user to specify a list of regular expression patterns that are compiled
and matched against the current request URI.

If the request URI matches one of them, then the request does not appear in the access log.

* Reduce blast radius of my catch-all controller

* Update javadoc

* Use null instead of calling a truthy predicate

* Give the test a proper name

* Allow for access logs to be out of order

* Pipeline manipulation instead of exclude field for AccessLogger

* Undo whitespace changes

* Pipeline manipulation fails with keep-alive.  Revert to exclude field

This reverts commit 3e97cdb.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable access logger for management endpoints
4 participants