chore: adding late event timer metrics #304
Conversation
kotharironak
requested review from
a team,
avinashkolluru,
findingrish,
laxmanchekka,
ravisingal and
satish-mittal
| tenant -> | ||
| PlatformMetricsRegistry.registerTimer( | ||
| DELAY_IN_SPAN_PROCESSED_TIME_METRIC, Map.of("tenantId", tenant))) | ||
| .record(spanProcessedTime - spanStartTime, TimeUnit.MILLISECONDS); |
There was a problem hiding this comment.
Do you guys think, I should wrap this in Math.absolute?
|
Checking why the build is failing here but passes locally. |
Codecov Report
@@ Coverage Diff @@
## main #304 +/- ##
============================================
+ Coverage 79.70% 79.74% +0.03%
- Complexity 1297 1299 +2
============================================
Files 118 118
Lines 5159 5168 +9
Branches 467 467
============================================
+ Hits 4112 4121 +9
Misses 837 837
Partials 210 210
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| tenant -> | ||
| PlatformMetricsRegistry.registerTimer( | ||
| DELAY_IN_SPAN_PROCESSED_TIME_METRIC, Map.of("tenantId", tenant), true)) | ||
| .record(spanProcessedTime - spanStartTime, TimeUnit.MILLISECONDS); |
There was a problem hiding this comment.
corner case, but clock skews could result in spanStartTime being more than spanProcessedTime. Should we first check if difference is not negative?
There was a problem hiding this comment.
Same question I had - #304 (comment)
Let me do that.
There was a problem hiding this comment.
@satish-mittal I have wrapped the difference around math.abs as if there is a very large skew we would like to know, right?
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| // register and update timer per tenant | ||
| // its uses absolute value to take care if any clock skewness too. |
There was a problem hiding this comment.
| // its uses absolute value to take care if any clock skewness too. | |
| // it uses absolute value to take care of any clock skewness too. |
There was a problem hiding this comment.
Done via UI to handle in a single commit.
| private final JaegerResourceNormalizer resourceNormalizer = new JaegerResourceNormalizer(); | ||
| private final TenantIdHandler tenantIdHandler; | ||
|
|
||
| // measure the span's start time and its processing time |
There was a problem hiding this comment.
| // measure the span's start time and its processing time | |
| // measure the difference between span's start time and its processing time |
There was a problem hiding this comment.
Done via UI to handle in a single commit.
| private final TenantIdHandler tenantIdHandler; | ||
|
|
||
| // measure the span's start time and its processing time | ||
| private static final String DELAY_IN_SPAN_PROCESSED_TIME_METRIC = "span.processed.delay.time"; |
There was a problem hiding this comment.
| private static final String DELAY_IN_SPAN_PROCESSED_TIME_METRIC = "span.processed.delay.time"; | |
| private static final String SPAN_PROCESSING_DELAY_TIME_METRIC = "span.processing.delay.time"; |
There was a problem hiding this comment.
Done via UI to handle in a single commit.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| private final TenantIdHandler tenantIdHandler; | ||
|
|
||
| // measure the difference between span's start time and its processing time | ||
| private static final String SPAN_PROCESSING_DELAY_TIME_METRIC = "span.processing.delay.time"; |
There was a problem hiding this comment.
Name: span.arrival.delay??
|
Closing this one as we are going with - #306 |
Description
There are cases when we would like to if the event arrived late or not. As part of this PR, it added the metrics to measure that.
So, the metric
span.processed.delay.timemeasure the different of current time to span start_time.Testing
Added basic unit test
Checklist: