-
Notifications
You must be signed in to change notification settings - Fork 186
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
Publish is_drift metric to Prom #1263
Conversation
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.
Two initial questions before final review.
return | ||
|
||
data = current_pred["data"] | ||
if "is_drift" in data: |
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.
Question for @adriangonz and @joshsgoldstein. What is the motivation for this PR?
Asking as wondering if we are definitely only concerned about is_drift
here? Often p_val
and distance
are of equal (or greater) interest. As is threshold
, and distance_threshold
(dependent on detector).
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.
@ascillitoe we are enabling scv2 alerting in deploy
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.
Initially we will just expose is_drift
- as that's good enough to set up alerting. I agree that exposing the other metrics will eventually become important though, but that's probably outside the scope of this PR.
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.
LGTM.
(except for the failing tests?)
Co-authored-by: Josh <josh4goldstein@gmail.com> Co-authored-by: Adrian Gonzalez-Martin <agm@seldon.io>
added drift metrics to be scraped by prometheus