search

cloudflare / foundations

Cloudflare's Rust service foundations library.
https://blog.cloudflare.com/introducing-foundations-our-open-source-rust-service-foundation-library
BSD 3-Clause "New" or "Revised" License
1.25k stars 51 forks source link

ZTC-1478: track the number of times a logger has been replaced with a child logger #47

Closed bcottrell8738 closed 3 months ago

bcottrell8738 commented 3 months ago

Every time we call the add_fields! macro, it adds one to the depth of the nested structure of Arcs inside the logger object. If the structure gets too deeply nested, it causes a stack overflow on drop.

Fixing this to make it keep the structure flat rather than nested would be difficult; so, this commit makes it keep track of how many times it has replaced a logger with a child logger, and it stops you (with a panic) before you reach a dangerous level of nesting.

The panic string is intended to be informative enough so that you get a hint as to where to go look in your code.

bcottrell8738 commented 3 months ago

Nice, this looks way better than if I had done it. My only comment is around documenting this new behaviour, maybe the documentation for add_fields and set_verbosity should warn the user about the potential panic lurking inside?

How does that look?

jgpaiva commented 3 months ago

Nice, this looks way better than if I had done it. My only comment is around documenting this new behaviour, maybe the documentation for add_fields and set_verbosity should warn the user about the potential panic lurking inside?

How does that look?

LGTM!