-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix glean metrics not showing after fix for Categorical probes #2817
Conversation
let isCategoricalProbe = | ||
$store.probe.details && $store.probe.details.kind === 'categorical'; |
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.
would this only work with Glean metrics? how about categorical histograms?
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.
My understanding is we want it to be true iff it's a Legacy probe and it's categorical, no?
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.
Legacy probes have details
whereas Glean metrics don't. Categorical histograms have to have isCategoricalProbe === true
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.
ah yea thanks for the clarification. could you remind me why we don't need to check for categorical glean probes here - distribution comparison is not available for glean probes yet?
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 didn't think Glean had categorical probes. I did a quick look up at Glean dictionary and didn't find any. Oh, are they under a different name?
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 think Glean boolean probes have categorical view in Glam, but will need to check if we even display these.
Line 75 in f0228a3
boolean: 'categorical', |
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.
hm, if that's the case GLAM doesn't support boolean
probes
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.
Thanks for fixing this!
This fixes the bug introduced in #2779, which searches for a field that's not present into Glean metrics' metadata.