wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 612 forks source link

[Epilogue] Improve opt-in logging support #6916

Open jwbonner opened 1 month ago

jwbonner commented 1 month ago

Is your feature request related to a problem? Please describe. Epilogue currently default to an opt-out model of logging, where every field and method in a class tagged with @Logged is automatically recorded. For complex robot projects, this can easily produce a massive amount of poorly organized data (this could easily be thousands of fields on a decently sized project). It is also very different from the opt-in model that most teams are used to with Monologue. While Epilogue has an option to enable opt-in logging on a per-class basis, there is no easy method to avoid opt-out logging globally without tagging every individual class using the "strategy" attribute.

Describe the solution you'd like Placing the @Logged annotation on a class should do opt-out logging by default, which is the current behavior. Additionally, users should be able to use the @Logged annotation on a field or method in a class not marked @Logged to enable opt-in logging of those individual fields/methods. This seems like the cleanest solution to the problem, since it's natural for users interested in a limited subset of fields to only tag those fields. I believe this also eliminates the need for the "strategy" attribute on the class-level annotation.

Describe alternatives you've considered Another solution would be a global option (configured via Epilogue.configure or similar) to change the default behavior from opt-out to opt-in. My understanding is that this option is significantly more difficult to implement.

SamCarlberg commented 1 month ago

users should be able to use the @Logged annotation on a field or method in a class not marked @Logged to enable opt-in logging of those individual fields/methods. This seems like the cleanest solution to the problem, since it's natural for users interested in a limited subset of fields to only tag those fields. I believe this also eliminates the need for the "strategy" attribute on the class-level annotation.

This sounds like a practical solution. Currently, anything marked with @Logged that's not in a class with the @Logged annotation will be ignored so this wouldn't affect the current behavior.

Another solution would be a global option (configured via Epilogue.configure or similar) to change the default behavior from opt-out to opt-in. My understanding is that this option is significantly more difficult to implement.

Correct. The configuration class is runtime-only. There is no way to detect that configuration at compile time without some AST walking hackery in the annotation processor. The alternative would be some configuration in the build.gradle file for the annotation processor to detect, which also isn't great.