CASSANDRASC-117 Record existing and additional metrics with dropwizard #111
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
sarankk
changed the title
sarankk
force-pushed
the
additional-metrics
branch
8 times, most recently
from
1ac085b
to
06d6c16
Compare
yifan-c
reviewed
Apr 10, 2024
src/main/java/org/apache/cassandra/sidecar/metrics/instance/InstanceHealthMetrics.java
Outdated
Show resolved
Hide resolved
yifan-c
reviewed
Apr 10, 2024
src/main/java/org/apache/cassandra/sidecar/metrics/instance/InstanceRestoreMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/RestoreMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/RestoreMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/RestoreMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/SchemaMetrics.java
Outdated
Show resolved
Hide resolved
src/test/java/org/apache/cassandra/sidecar/concurrent/ExecutorPoolsTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/concurrent/ExecutorPools.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/ResourceMetrics.java
Outdated
Show resolved
Hide resolved
sarankk
force-pushed
the
additional-metrics
branch
3 times, most recently
from
e9beb62
to
bb32b9e
Compare
frankgh
reviewed
Apr 11, 2024
There was a problem hiding this comment.
I've left a few comments, but I think we need to address the SSTable Component metrics, because it seems they are not really SSTable Component per se, but rather based on the extension. So either the metric will need to be extension based metric, or we need to correctly determine the SSTable Component
| workerExecutor.executeBlocking(promise -> { | ||
| long startTime = System.nanoTime(); | ||
| handler.handle(id); | ||
| recordTimeTaken(System.nanoTime() - startTime); |
There was a problem hiding this comment.
unrelated to the PR, but do we need to complete the promise here?
There was a problem hiding this comment.
It is wrong w/o fulfilling the promise. I have a feedback commit (to be pushed) to address it.
src/main/java/org/apache/cassandra/sidecar/metrics/instance/InstanceRestoreMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/instance/InstanceRestoreMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/instance/SSTableImportMetrics.java
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/concurrent/TaskExecutorPool.java
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/utils/SSTableImporter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/instance/SSTableImportMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/routes/sstableuploads/SSTableUploadHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/instance/UploadSSTableMetrics.java
Outdated
Show resolved
Hide resolved
|
Thanks @frankgh for the review. Addressed them. Modified to using SSTable component name instead of extension while tagging metrics |
sarankk
force-pushed
the
additional-metrics
branch
from
99a1824
to
4518dc8
Compare
sarankk
force-pushed
the
additional-metrics
branch
from
0afa958
to
7ec2b57
Compare
…ics.java Co-authored-by: Francisco Guerrero <frank.guerrero@gmail.com>
…stanceRestoreMetrics.java Co-authored-by: Francisco Guerrero <frank.guerrero@gmail.com>
…utorPool.java Co-authored-by: Francisco Guerrero <frank.guerrero@gmail.com>
sarankk
force-pushed
the
additional-metrics
branch
from
8b43a37
to
ff1acd4
Compare
sarankk
force-pushed
the
additional-metrics
branch
from
ff1acd4
to
5123b43
Compare
yifan-c
reviewed
Apr 16, 2024
src/main/java/org/apache/cassandra/sidecar/metrics/instance/SSTableImportMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/routes/sstableuploads/SSTableImportHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/instance/UploadSSTableMetrics.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/cassandra/sidecar/metrics/instance/StreamSSTableMetrics.java
Outdated
Show resolved
Hide resolved
sarankk
force-pushed
the
additional-metrics
branch
3 times, most recently
from
da3e0ed
to
d9f4855
Compare
sarankk
force-pushed
the
additional-metrics
branch
from
d9f4855
to
8bbc0c2
Compare
|
Thanks @yifan-c for the feedback, addressed it. |
yifan-c
approved these changes
Apr 16, 2024
frankgh
approved these changes
Apr 17, 2024
sarankk
force-pushed
the
additional-metrics
branch
from
22fa4da
to
b18e07a
Compare
sarankk
force-pushed
the
additional-metrics
branch
from
b18e07a
to
43b911e
Compare
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.
No description provided.