Add support for selectors in TimeSeriesMetricsService #79691
Conversation
|
👍 LGTM |
| if (capabilities.isDimension()) { | ||
| dimensions.add(fieldName); | ||
| if (capabilities.getMetricType() != null) { |
There was a problem hiding this comment.
A field can't be a metric and a dimension at the same time, right?
elasticmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
|
|
||
| @TestLogging(value = "org.elasticsearch.search.tsdb:debug", reason = "test") | ||
| @TestLogging(value = "org.elasticsearch.timeseries.support:debug", reason = "test") |
There was a problem hiding this comment.
👍. Not sure if we actually need to keep it though. But it helped me at first, that's for sure.
| RE_EQUAL { | ||
| @Override | ||
| protected Predicate<String> matcher(String expression) { | ||
| return Pattern.compile(expression, 0).asMatchPredicate(); |
| * @param metrics metrics selectors | ||
| * @param dimensions dimension selectors | ||
| * @param time time | ||
| * @param callback callback |
There was a problem hiding this comment.
Could you nuke the empty @param annotations?
| /** | ||
| * Get the latest value for all time series in many ranges. | ||
| * Return the latest metrics before time within staleness period | ||
| * @param metrics metrics selectors |
There was a problem hiding this comment.
Could you mention that these are ORed together.
| } | ||
| DocumentField metricField = hit.field(metric); | ||
| if (metricField == null) { | ||
| // TODO skip in query? |
There was a problem hiding this comment.
This has to stay forever I think. You are filtering out documents that don't have any of the fields, but we still have to skip those that have only one.
| FilterAggregationBuilder[] filters = new FilterAggregationBuilder[(int) (last - first)]; | ||
| for (int i = 0; i < filters.length; i++) { | ||
| metricAgg.subAggregation( | ||
| new FilterAggregationBuilder( |
There was a problem hiding this comment.
filters is pretty much built for this.
| } | ||
| return metricAgg; |
There was a problem hiding this comment.
This whole thing is going to be really fun to run. I get it, I think, but wow. I guess this is what we need.
There was a problem hiding this comment.
We might be able to improve things by having top_metrics have a better behavior when the field is missing. But that might be a thing for another time.
Probably wants a PR description. |
Adds basic support for selectors in TimeSeriesMetricsService
Relates to #74660