add-disk-buffering-debug-mode #664
Conversation
...rc/main/java/io/opentelemetry/android/features/diskbuffering/DiskBufferingConfiguration.java
Outdated
Show resolved
Hide resolved
| @@ -22,6 +22,7 @@ public final class DiskBufferingConfiguration { | |||
| private final long maxFileAgeForWriteMillis; | |||
| private final long minFileAgeForReadMillis; | |||
| private final long maxFileAgeForReadMillis; | |||
| private final boolean enableDebugMode; | |||
There was a problem hiding this comment.
I think a debug mode should be part of the OtelRumConfig or the lowest level of configuration (if there's one) so the whole SDK respects the debug mode flag, and not only for the disk buffering, this would be useful across the SDK and its configurations
There was a problem hiding this comment.
I think there's a case to be made for both. Top-level debug mode could imply disk buffering debug mode, but not the other way around. In any case, that should be a separate PR IMO. Feel free to log an issue to track.
There was a problem hiding this comment.
All good, having an SDK debug mode for all options feels too much IMO, a top-level debug mode that logs everything everywhere and respects eg a severity log level for filtering would be cleaner and easier to enable/disable.
If we introduce a debug flag per feature eg diskbuffering, that just adds more complexity everywhere, I'd rather achieve this with TAG filtering if the global debug mode is enabled.
There was a problem hiding this comment.
I unresolved the comment so that I can copy-paste the comment URL otherwise the anchor link does not work, but it's not that you have to address this in this PR.
There was a problem hiding this comment.
Thanks. I'm not entirely sure I made my point, so let's continue chatting about it at SIG sometime.
…ring/DiskBufferingConfiguration.java Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
| Level.INFO, | ||
| "Disk buffering has been " | ||
| + (enabled ? "enabled." : "disabled.") | ||
| + (enableDebugMode ? " Debug mode is active." : "")); |
There was a problem hiding this comment.
I don't think we need to log the enabled setting here -- that's a separate concern below. I recommend changing the log message to be simpler and only reflect the new value of enableDebugMode.
| /** Enables or disables disk buffering. */ | ||
| public Builder setEnabled(boolean enabled) { | ||
| this.enabled = enabled; | ||
| if (enableDebugMode) { | ||
| Logger.getLogger(DiskBufferingConfiguration.class.getName()) | ||
| .log(Level.INFO, "Debug log message here"); |
There was a problem hiding this comment.
I think this message is probably not what you intended?
|
Hey @Annosha just checking in to see if you're still able to work on some revisions/improvements to this PR. Thanks again! |
Description
Fixes #597 Add setting to enable disk buffering debug mode
This PR introduces a configurable debug mode in the
DiskBufferingConfigurationclass within OpenTelemetry Android, allowing users to enable verbose logging for troubleshooting disk buffering behavior.Changes Made
setDebugModeMethod:A new method,
setDebugMode, has been added to theDiskBufferingConfiguration.Builderclass. This method accepts a boolean parameter to enable or disable debug mode.When enabled, a log message indicates that debug mode for disk buffering is active, providing enhanced logging details for easier troubleshooting.
setEnabledMethod:The
setEnabledmethod now includes a check forenableDebugMode. If debug mode is active, an additional log message provides further information on the state of disk buffering.This conditional logging gives users more visibility into the disk buffering configuration when debug mode is enabled.