Skip to content

Tweak monitoring to reduce false positives - #154

Open
tunetheweb wants to merge 1 commit into
mainfrom
tunetheweb-patch-1
Open

tunetheweb wants to merge 1 commit into
mainfrom
tunetheweb-patch-1

Conversation

@tunetheweb

Copy link
Copy Markdown
Member

No description provided.

Comment thread terraform/monitoring.tf
Comment on lines +159 to +160
-httpRequest.userAgent =~ "(?i)(curl|bot|crawler|spider|slurp|archiver|scraper|research|baiduspider|googlebot|facebookexternalhit|meta-externalagent|mj12bot|petalbot|ccbot|censysinspect|worker|python|http-|-http)"
-protoPayload.userAgent =~ "(?i)(curl|bot|crawler|spider|slurp|archiver|scraper|research|baiduspider|googlebot|facebookexternalhit|meta-externalagent|mj12bot|petalbot|ccbot|censysinspect|worker|python|http-|-http)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This hides real errors on /v1/technologies and /v1/cwv-distribution. I've reviewed user-agent list many times but it's still growing. I now think it's easier to fix server errors. Fixed in #156

Comment thread terraform/monitoring.tf
(resource.type = "cloud_run_revision" AND resource.labels.service_name = "report-api-prod")
OR logName = "projects/httparchive/logs/requests"
severity >= WARNING
httpRequest.requestUrl =~ "https:\/\/cdn\.httparchive\.org"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is mostly redundant.
The only IP-based hit happened before I added enforce_valid_host rule. Since then, IP requests get a 403 at the load balancer - denied_by_security_policy is already excluded.

Maybe add include_host = true to CDN cache key additionally, to avoid them hitting cache. Or should we move this policy to edge?

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.

2 participants