storage: add log.Safe to "impossible" panic

[nvanbenschoten]

See #28889.

Release note: None

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[tschottdorf]

What will this error leak? If there are any parts of the key in there, that wouldn't be acceptable. And even if that isn't the case, we're placing blind trust in that not changing out from under us -- I think we need to be more prudent here and extract only the information we can affort to leak from production clusters. For example, grab the key range, extract the sizes of the key, longest common prefix, and stuff like that. Or even better, go into tree.Delete()s impl and let it return Safe (or at least typed) errors (when we know that it's not leaking input).

I know this is a pain, but we've got to do it.

[nvanbenschoten]

The error wouldn't currently have leaked sensitive information because it had to have been either an ErrInvertedRange or an ErrEmptyRange, but I agree with your point about this changing in the future. I reworked this to scope the log.Safe call more closely. An added benefit of this is that we now get this extra introspection into any panics by users of the interval package. PTAL.

[nvanbenschoten]

The error wouldn't currently have leaked sensitive information because it had to have been either an ErrInvertedRange or an ErrEmptyRange, but I agree with your point about this changing in the future. I reworked this to scope the log.Safe call more closely. An added benefit of this is that we now get this extra introspection into any panics by users of the interval package. PTAL.

[tschottdorf]

Thanks!