-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add drop reason dimension to ingest/events/thrownAway metric #18855
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
Add drop reason dimension to ingest/events/thrownAway metric #18855
Conversation
0a7d1ad to
76bbd91
Compare
76bbd91 to
36e0c72
Compare
c9c7e0d to
0ed4046
Compare
kfaraz
left a comment
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.
Looks good, left some minor suggestions.
...service/src/main/java/org/apache/druid/indexing/common/stats/TaskRealtimeMetricsMonitor.java
Outdated
Show resolved
Hide resolved
indexing-service/src/main/java/org/apache/druid/indexing/common/task/RowFilter.java
Outdated
Show resolved
Hide resolved
indexing-service/src/main/java/org/apache/druid/indexing/common/task/RowFilter.java
Outdated
Show resolved
Hide resolved
indexing-service/src/main/java/org/apache/druid/indexing/common/task/RowFilter.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/ThrownAwayReason.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/ThrownAwayReason.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/ThrownAwayReason.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/ThrownAwayReason.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/ThrownAwayReason.java
Outdated
Show resolved
Hide resolved
...ce/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunner.java
Outdated
Show resolved
Hide resolved
...ce/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunner.java
Outdated
Show resolved
Hide resolved
...ce/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunner.java
Outdated
Show resolved
Hide resolved
...ce/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunner.java
Outdated
Show resolved
Hide resolved
...g-service/src/test/java/org/apache/druid/indexing/common/TaskRealtimeMetricsMonitorTest.java
Outdated
Show resolved
Hide resolved
9013d02 to
5ca36a7
Compare
9d8b12a to
1704dda
Compare
3a435d8 to
c8f8981
Compare
kfaraz
left a comment
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.
Left some final suggestions.
...rvice/src/main/java/org/apache/druid/indexing/common/stats/DropwizardRowIngestionMeters.java
Outdated
Show resolved
Hide resolved
...service/src/main/java/org/apache/druid/indexing/common/stats/TaskRealtimeMetricsMonitor.java
Outdated
Show resolved
Hide resolved
indexing-service/src/main/java/org/apache/druid/indexing/common/task/InputRowFilter.java
Outdated
Show resolved
Hide resolved
...ce/src/main/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunner.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/RowIngestionMetersTotals.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/RowIngestionMetersTotals.java
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/RowIngestionMetersTotals.java
Outdated
Show resolved
Hide resolved
abhishekrb19
left a comment
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.
LGTM, left a few minor comments - thanks!
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
processing/src/main/java/org/apache/druid/segment/incremental/InputRowFilterResult.java
Outdated
Show resolved
Hide resolved
78aaeae to
9a2d432
Compare
e2a2eea to
e489214
Compare
e489214 to
62440f3
Compare
Description
I've found it hard to build observability on logically-dropped events (either due to transform, null, or temporal conditions).
This adds:
reasondimension to theingest/events/thrownAwaymetric for aggregating on thrown away reason.thrownAwayByReasonmap to row statistics task API response payload, so future consumers can make use of it should they need to.Release note
Add reason dimension to ingest/events/thrownAway metric. This allows for increased observability on why certain events are being logically excluded from ingest.
This PR has: