Skip to content

Conversation

@timgent
Copy link
Owner

@timgent timgent commented Oct 30, 2020

No description provided.

MetricsCalculator.calculateMetrics(describedDataset, metricDescriptors)
(describedDataset, metricValues)
}
val allPreviousMetricsFut: Future[Map[Instant, Map[SingleDsDescription, Map[SimpleMetricDescriptor, MetricValue]]]] =
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should load only historic metrics that we actually need


for {
_ <- storedMetricsFut
allPreviousMetrics: Map[Instant, Map[SingleDsDescription, Map[SimpleMetricDescriptor, MetricValue]]] <- allPreviousMetricsFut
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per above we should only load historic metrics we actually need

@timgent timgent force-pushed the metric-anomaly-check branch from 69802f8 to ba46206 Compare October 30, 2020 16:05
@kingbrown9
Copy link

That would be a great feature. What's left for this to get merged?

@timgent
Copy link
Owner Author

timgent commented May 10, 2022

@kingbrown9 in the current form it could be merged with not too much more work - would just need to fully review the code.

The reason it stalled was because there's an issue which is trickier to solve. When you get an anomalous result should future anomaly checks take that into account or not? There's no way to know without human intervention to say if it was legitimate or not. This was part of the reason for starting to work on a UI (which is still in a very early, basic state, though did work in a simple form with the API, though it didn't cover this use case yet).

Thoughts appreciated though if it's useful even in it's current form as probably not too much work to get this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants