-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
logging: Add support for additional logger filters other than hostname #6082
logging: Add support for additional logger filters other than hostname #6082
Conversation
Like I said in the issue, I'm not totally convinced this is the right way to do this. But I can't really think of anything better. I think I think there should be a I'm not sure if that's the best name though, I'd like @mholt's opinion. |
9643509
to
de7a8d4
Compare
caddytest/integration/caddyfile_adapt/log_filter_with_header.txt
Outdated
Show resolved
Hide resolved
de7a8d4
to
6074d06
Compare
@francislavoie, all the feedback items above should be resolved, except for the potential merge conflict with multiple hostnames. |
@francislavoie @armadi1809 Let me know if you want to discuss anything to wrap up this PR. 👍 |
If I remember correctly this needs to wait for #6088 to be merged as it will require some changes after that's done right? @francislavoie |
I'm merging it now (once CI passes) 👍 you can rebase your branch and make the necessary adjustments. |
90b3214
to
4f04d4c
Compare
Needs another rebase, there's a conflict with another recent change of mine. Sorry! |
4f04d4c
to
10cf134
Compare
@francislavoie, rebase done. Any thoughts on the open discussion above? |
caddytest/integration/caddyfile_adapt/log_filter_with_header.txt
Outdated
Show resolved
Hide resolved
10cf134
to
c7bd12b
Compare
caddytest/integration/caddyfile_adapt/log_filter_with_header.txt
Outdated
Show resolved
Hide resolved
c7bd12b
to
aea0e75
Compare
I did a commit to do a final bit of cleanup and refactoring. I think this is good to go now. |
Awesome, thank you both!! |
Just wanted to say THANK YOU for this awesome new feature. I have just tested it with Caddy v2.8.0-rc and my configuration looked like this (maybe this helps for other users, viewers of this MR or for the caddy docs website). example.com {
log example.com
log health_check_log {
output file /var/log/caddy/access-health-checks.json {
roll_size 50m
roll_local_time
roll_keep_for 30d
}
format filter {
wrap json
fields {
request>tls>version tls_version TLSv
request>tls>cipher_suite tls_cipher
}
}
no_hostname
}
@monitoring_ohdear_uptime {
# Two possible user-agent headers:
# Mozilla/5.0 (compatible; OhDear/1.1; +https://ohdear.app/checker; brokenLinks)
# OhDear.app (+https://ohdear.app/docs/checks/uptime)
header_regexp user-agent `^.*\+https:\/\/ohdear\.app.*$`
header_regexp X-External-Monitoring ^redacted_secret$
}
# Before Caddy v.2.8.0 these requests were skipped; now commented
#log_skip @monitoring_ohdear_uptime
# Now with Caddy v2.8.0-rc.1 these requests are written into a dedicated file 👍
log_name @monitoring_ohdear_uptime health_check_log
... |
closes #5978
@francislavoie This took some time but here is my initial implementation of this. Would like to get your feedback on this and your thoughts on what other tests should go in here? Thanks.