-
Notifications
You must be signed in to change notification settings - Fork 3
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 custom timeInTopic metric to timeseries-aggregator #172
Add custom timeInTopic metric to timeseries-aggregator #172
Conversation
Codecov Report
@@ Coverage Diff @@
## master #172 +/- ##
=======================================
Coverage ? 0%
=======================================
Files ? 30
Lines ? 36
Branches ? 6
=======================================
Hits ? 0
Misses ? 36
Partials ? 0
Continue to review full report at Codecov.
|
@@ -52,6 +53,8 @@ class WindowedMetric private(var windowedMetricsMap: mutable.TreeMap[TimeWindow, | |||
* @param incomingMetricData - incoming metric data | |||
*/ | |||
def compute(incomingMetricData: MetricData): Unit = { | |||
timeInTopicMetricPointHistogram.update(incomingMetricData.getTimestamp() - System.currentTimeMillis()) |
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.
Is incomingMetricData.getTimestamp() is in micros?
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.
You know, I'm not sure, let me double check
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.
It looks like the MetricDataTransformer object converts the span's microseconds to seconds
https://github.com/ExpediaDotCom/haystack-trends/blob/master/span-timeseries-transformer/src/main/scala/com/expedia/www/haystack/trends/transformer/MetricDataTransformer.scala#L41
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.
I'll update my PR with that info
This PR adds a new metric that measures the amount of time between a given span's creation timestamp and the point at which it is consumed as part of trends creation.
Our goal is to be able to demonstrate the amount of time between the time an app submits a trace and the time it takes to generate trends off of it is underneath a certain upper bound.