feat: measure object upload bytes #280
Merged
Conversation
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
ivanyu
requested changes
Jun 14, 2023
storage/s3/src/main/java/io/aiven/kafka/tieredstorage/storage/s3/S3Storage.java
Outdated
Show resolved
Hide resolved
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
2 times, most recently
from
e49ca4a to
2d28abc
Compare
ivanyu
reviewed
Jun 14, 2023
storage/s3/src/main/java/io/aiven/kafka/tieredstorage/storage/s3/S3Storage.java
Outdated
Show resolved
Hide resolved
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
3 times, most recently
from
374ce3f to
5cf07cc
Compare
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
from
5cf07cc to
268fa38
Compare
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
2 times, most recently
from
10d035f to
4c8a116
Compare
jeqo
changed the title
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
from
4c8a116 to
0ae6fb3
Compare
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
2 times, most recently
from
6ee4f15 to
f41a1d7
Compare
core/src/main/java/io/aiven/kafka/tieredstorage/RemoteStorageManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/aiven/kafka/tieredstorage/metrics/Metrics.java
Outdated
Show resolved
Hide resolved
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
from
f41a1d7 to
21dea2b
Compare
ivanyu
reviewed
Jul 11, 2023
| @@ -238,17 +238,20 @@ private void uploadSegmentLog(final RemoteLogSegmentMetadata remoteLogSegmentMet | |||
| throws IOException, StorageBackendException { | |||
| final String fileKey = objectKey.key(remoteLogSegmentMetadata, ObjectKey.Suffix.LOG); | |||
| try (final var sis = transformFinisher.toInputStream()) { | |||
| uploader.upload(sis, fileKey); | |||
| final var bytes = uploader.upload(sis, fileKey); | |||
| metrics.recordObjectUpload(ObjectKey.Suffix.LOG, bytes); | |||
There was a problem hiding this comment.
We need a way to get the aggregated number of bytes. It seems to me, one of:
- Add a separate aggregated sensor.
- Make the suffix a tag instead of a part of the sensor name (so we can sum by this tag).
- Don't bother with separating by tag, the log files will anyway be 99%.
There was a problem hiding this comment.
We can follow a similar approach as with topic vs topic-partition, and have an aggregated metric on INFO level, and by object type in DEBUG, wdyt?
There was a problem hiding this comment.
Sure, giving a try now. Pushed latest changes.
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
from
21dea2b to
ae541e9
Compare
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
from
ae541e9 to
569a1e1
Compare
jeqo
force-pushed
the
jeqo/storage-upload-size
branch
from
569a1e1 to
75b4b43
Compare
ivanyu
approved these changes
Jul 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes:
uploadis expanded to return the number of bytes uploaded.