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

Log warnings for unretrievable URLs in descriptor #29

Open
danlamanna opened this issue Jan 26, 2023 · 1 comment
Open

Log warnings for unretrievable URLs in descriptor #29

danlamanna opened this issue Jan 26, 2023 · 1 comment

Comments

@danlamanna
Copy link
Contributor

These lines silently ignore URLs that can't be retrieved:

resp, err := http.Get(entry.Url().String())
if err != nil {
continue
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
continue
}

@scosman
Copy link
Owner

scosman commented Jan 30, 2023

P2: if anyone want to tackle this would take to master, but not a top issue. Would need to be behind a config flag ZS_LOG_UPSTREAM_ERRORS which still would default off.

Why I don't think this is on by default:

  • Logs could be filled with non-exceptional errors. Ignoring bad URLs is the right thing, some users could send thousands, polluting the log with errors that don't need to be addressed
  • the URLs can contain access keys (examples: S3 access keys as query string param, I used with a system with 256 bit keys in URL). We shouldn't assume the logging platform is setup for "secrets", and getting this should be opt in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants