Emit missing ingest/merge/time, ingest/merge/cpu and ingest/persists/cpu metrics
#18866
Conversation
This file contains hidden or 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
- Adds the missing streaming appendator metrics for ingest/merge/time, ingest/merge/cpu and ingest/persists/cpu
Init ServiceEmitter as non-static to reuse individually for each test Add a helper in the base class for asserting segment generation metrics
abhishekrb19
changed the title
ingest/merge/time, ingest/merge/cpu and ingest/persists/cpu metrics for streaming tasksingest/merge/time, ingest/merge/cpu and ingest/persists/cpu metrics
jtuglu1
approved these changes
Dec 23, 2025
Contributor
jtuglu1
left a comment
jtuglu1
left a comment
There was a problem hiding this comment.
LGTM – left minor comments.
| throw e; | ||
| } | ||
| finally { | ||
| metrics.incrementNumPersists(); |
Contributor
There was a problem hiding this comment.
nit: can we move the increments below the time-sensitive metric emissions? Given we are reporting at nano-scale, chance of interrupt and/or delay due to atomic load+store can skew the nano count by X-XXns.
| identifier, | ||
| segment.getSize(), | ||
| indexes.size(), | ||
| (mergeFinishTime - startTime) / 1000000, |
Contributor
There was a problem hiding this comment.
👍 We should probably make sure this is not being done elsewhere. For future readers, System.nanoTime() is actually more expensive than System.getCurrentTimeMillis() so if you need milli-level precision, just use the latter from the start. Milli itself is also expensive: https://pzemtsov.github.io/2017/07/23/the-slow-currenttimemillis.html
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The metrics
ingest/merge/time,ingest/merge/cpuandingest/persists/cpumetrics have been documented but were previously reported as zero because they were not set in the streaming and batch appendators (it probably regressed in a refactor).These metrics are now set by the appendators and will be reported during the persist and merge phases for realtime and batch ingestion. Updated unit tests to verify that these values are emitted (non-zero values).
Also, cleaned up
KafkaIndexTaskTestandKinesisIndexTaskTestby initializing aStubServiceEmitteras a non-static member in the base classSeekableStreamIndexTaskTestBaseso it can be used by each unit test as needed.Release note
Correctly report
ingest/merge/time,ingest/merge/cpuandingest/persists/cpumetrics for streaming and batch ingestion tasks, which were previously always misreported as zero values.This PR has:
(https://github.com/apache/druid/blob/master/dev/license.md)