-
Notifications
You must be signed in to change notification settings - Fork 33
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
Migrate DiskManager to Kotlin #334
Merged
breedx-splk
merged 7 commits into
open-telemetry:main
from
breedx-splk:diskmanager_to_kotlin
May 2, 2024
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
667e6e0
Rename .java to .kt
breedx-splk b1747a4
migrate DiskManager and test to kotlin
breedx-splk e0fcf7f
wire up the new constructor
breedx-splk e417f3e
Rename .java to .kt
breedx-splk 5415042
migrate SimpleTemporaryFileProvider
breedx-splk f4cbc14
move to companion
breedx-splk 79c04b9
spotless
breedx-splk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
122 changes: 0 additions & 122 deletions
122
...ent/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.java
This file was deleted.
Oops, something went wrong.
114 changes: 114 additions & 0 deletions
114
...agent/src/main/java/io/opentelemetry/android/internal/features/persistence/DiskManager.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.android.internal.features.persistence | ||
|
||
import android.util.Log | ||
import io.opentelemetry.android.common.RumConstants | ||
import io.opentelemetry.android.features.diskbuffering.DiskBufferingConfiguration | ||
import io.opentelemetry.android.internal.services.CacheStorageService | ||
import io.opentelemetry.android.internal.services.PreferencesService | ||
import java.io.File | ||
import java.io.IOException | ||
|
||
internal class DiskManager( | ||
private val cacheStorageService: CacheStorageService, | ||
private val preferencesService: PreferencesService, | ||
private val diskBufferingConfiguration: DiskBufferingConfiguration, | ||
) { | ||
@get:Throws(IOException::class) | ||
val signalsBufferDir: File | ||
get() { | ||
val dir = File(cacheStorageService.cacheDir, "opentelemetry/signals") | ||
ensureExistingOrThrow(dir) | ||
return dir | ||
} | ||
|
||
@get:Throws(IOException::class) | ||
val temporaryDir: File | ||
get() { | ||
val dir = File(cacheStorageService.cacheDir, "opentelemetry/temp") | ||
ensureExistingOrThrow(dir) | ||
deleteFiles(dir) | ||
return dir | ||
} | ||
|
||
val maxFolderSize: Int | ||
/** | ||
* It checks for the available cache space in disk, then it attempts to divide by 3 in order to | ||
* get each signal's folder max size. The resulting value is subtracted the max file size value | ||
* in order to account for temp files used during the reading process. | ||
* | ||
* @return If the calculated size is < the max file size value, it returns 0. The calculated | ||
* size is stored in the preferences and returned otherwise. | ||
*/ | ||
get() { | ||
val storedSize = preferencesService.retrieveInt(MAX_FOLDER_SIZE_KEY, -1) | ||
if (storedSize > 0) { | ||
Log.d( | ||
RumConstants.OTEL_RUM_LOG_TAG, | ||
String.format("Returning max folder size from preferences: %s", storedSize), | ||
) | ||
return storedSize | ||
} | ||
val requestedSize = diskBufferingConfiguration.maxCacheSize | ||
val availableCacheSize = | ||
cacheStorageService.ensureCacheSpaceAvailable(requestedSize.toLong()).toInt() | ||
// Divides the available cache size by 3 (for each signal's folder) and then subtracts the | ||
// size of a single file to be aware of a temp file used when reading data back from the | ||
// disk. | ||
val maxCacheFileSize = maxCacheFileSize | ||
val calculatedSize = availableCacheSize / 3 - maxCacheFileSize | ||
if (calculatedSize < maxCacheFileSize) { | ||
Log.w( | ||
RumConstants.OTEL_RUM_LOG_TAG, | ||
String.format( | ||
"Insufficient folder cache size: %s, it must be at least: %s", | ||
calculatedSize, | ||
maxCacheFileSize, | ||
), | ||
) | ||
return 0 | ||
} | ||
preferencesService.store(MAX_FOLDER_SIZE_KEY, calculatedSize) | ||
Log.d( | ||
RumConstants.OTEL_RUM_LOG_TAG, | ||
String.format( | ||
"Requested cache size: %s, available cache size: %s, folder size: %s", | ||
requestedSize, | ||
availableCacheSize, | ||
calculatedSize, | ||
), | ||
) | ||
return calculatedSize | ||
} | ||
|
||
val maxCacheFileSize: Int | ||
get() = diskBufferingConfiguration.maxCacheFileSize | ||
|
||
companion object { | ||
private const val MAX_FOLDER_SIZE_KEY = "max_signal_folder_size" | ||
|
||
private fun deleteFiles(dir: File) { | ||
val files = dir.listFiles() | ||
if (files != null) { | ||
for (file in files) { | ||
if (file.isDirectory()) { | ||
deleteFiles(file) | ||
} | ||
file.delete() | ||
} | ||
} | ||
} | ||
|
||
private fun ensureExistingOrThrow(dir: File) { | ||
if (!dir.exists()) { | ||
if (!dir.mkdirs()) { | ||
throw IOException("Could not create dir $dir") | ||
} | ||
} | ||
} | ||
} | ||
} |
27 changes: 0 additions & 27 deletions
27
...a/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.java
This file was deleted.
Oops, something went wrong.
17 changes: 17 additions & 0 deletions
17
...ava/io/opentelemetry/android/internal/features/persistence/SimpleTemporaryFileProvider.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.android.internal.features.persistence | ||
|
||
import io.opentelemetry.contrib.disk.buffering.internal.files.TemporaryFileProvider | ||
import java.io.File | ||
import java.util.UUID | ||
|
||
internal class SimpleTemporaryFileProvider(private val tempDir: File) : TemporaryFileProvider { | ||
/** Creates a unique file instance using the provided prefix and the current time in millis. */ | ||
override fun createTemporaryFile(prefix: String): File { | ||
return File(tempDir, prefix + "_" + UUID.randomUUID() + ".tmp") | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fairly idiomatic these days....but I'm happy to be talked out of this style/syntax if it is important to anyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair enough, considering that Kotlin allows you to access getter methods as if they were class fields later on, it seems like the question of defining them like this vs regular getters boils down to personal preferences really. I've seen custom accessors widely used for cases where the
get()
orset()
body is quite small, like this one, although it does seem a bit strange to me to see them as big as theget()
inmaxFolderSize
for example.Regarding my personal preferences, I find it easier to read and quickly set apart in my mind what parts in a class hold business logic vs what others only store things in memory by using regular methods and fields, although it's probably a matter of getting used to different styles, so it's not the end of the world for me to use either approach.