adding time based filter for aggregation views #181
Conversation
Codecov Report
@@ Coverage Diff @@
## main #181 +/- ##
============================================
+ Coverage 78.51% 78.68% +0.16%
+ Complexity 856 10 -846
============================================
Files 80 80
Lines 3608 3626 +18
Branches 407 413 +6
============================================
+ Hits 2833 2853 +20
+ Misses 599 598 -1
+ Partials 176 175 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| long requestEndTime = getRequestEndTime(request.getFilter()); | ||
| Duration requestDuration = Duration.ofMillis(requestEndTime - requestStartTime); | ||
|
|
||
| if (minRequestDuration != Duration.ZERO |
There was a problem hiding this comment.
Why do we need to check for Duration Zero explicitly. Single compare check as below should be good enough. We will anyways have order defined in query service config, and handler will higher minDuration will be defined first.
There was a problem hiding this comment.
We can do that. I was keeping the default handler first in the list.
With this, we have to keep the handler order from highest to lowest minRequestDuration. Logically, this is easy to maintain, addressing the comments.
e.g if there are three handlers as described in test cases, we will have to list them in the following order.
- backend-traces-from-bare-span-event-view-5min-aggr-handler
- minRequestDuration : 12 hrs
- backend-traces-from-bare-span-event-view-30secs-aggr-handler
- minRequestDuration : 3 hrs
- backend-traces-from-bare-span-event-view-aggr-handler
- minRequestDuration : 0 hrs
| Duration requestDuration = Duration.ofMillis(requestEndTime - requestStartTime); | ||
|
|
||
| // check: requestDuration < minRequestDuration | ||
| if (!(requestDuration.compareTo(minRequestDuration) < 0)) { |
There was a problem hiding this comment.
| if (!(requestDuration.compareTo(minRequestDuration) < 0)) { | |
| if (requestDuration.compareTo(minRequestDuration) >= 0) { |
There was a problem hiding this comment.
As we are selecting a handler based on minimum cost, here, would like to reduce the cost of the hander that can server has met.
In this requestDuration.compareTo(minRequestDuration) >= 0, I don't want to change anything.
There was a problem hiding this comment.
Oh! okey. I will change the boolean condition.
There was a problem hiding this comment.
Done, misunderstood with the ordering of handler.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Description
As part of Postgres implementation, it adds the support of choosing a request handler based on query request time range.
In Postgres, there was a requirement to have multiple levels of aggregations for time series metrics data - 30 secs, 5 mins, and so on. This requires different table selections based on the query request time range. So, as part of Postgres implementation, it adds the support of choosing a request handler based on the query request time range.
Testing
Added the required unit tests
Checklist: