T2viz UI metrics #312
T2viz UI metrics #312
Conversation
Signed-off-by: Yulong Ruan <[email protected]>
Signed-off-by: Yulong Ruan <[email protected]>
ruanyl
requested review from
lezzago,
joshuali925,
anirudha,
SuZhou-Joe,
xluo-aws,
gaobinlong,
Hailong-am,
wanglam and
raintygao
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #312 +/- ##
==========================================
+ Coverage 88.59% 88.65% +0.06%
==========================================
Files 60 61 +1
Lines 1710 1728 +18
Branches 419 426 +7
==========================================
+ Hits 1515 1532 +17
- Misses 193 194 +1
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Yulong Ruan <[email protected]>
Signed-off-by: Yulong Ruan <[email protected]>
| @@ -111,14 +127,23 @@ export const Text2Viz = () => { | |||
| }); | |||
| } else { | |||
| setEditorInput(JSON.stringify(result, undefined, 4)); | |||
|
|
|||
| // Report metric when visualization generated successfully | |||
| if (usageCollection) { | |||
There was a problem hiding this comment.
Could use METRIC_TYPE.COUNT to either record a 1 for success or 0 for failure. We could calculate average to get success rate.
There was a problem hiding this comment.
Currently, it doesn't record the failures. If we do need to record the failures, I'd rather use another event type, like:
usageCollection.reportUiStats(
VIS_NLQ_APP_ID,
usageCollection.METRIC_TYPE.LOADED,
`generate_failed-${uuidv4()}`
);What do you think?
There was a problem hiding this comment.
I mean we could use the same metric, and emit 0 when fails, 1 when succeeds. Like
try {
const response = await fetch();
emitMetric("fetch_success", COUNT, 1);
} catch( err: any) {
emitMetric("fetch_success", COUNT, 0);
}
We always ensure that we emit one time for a fetch, it's either 1 or 0.
| @@ -410,6 +444,13 @@ export const Text2Viz = () => { | |||
| paddingSize="none" | |||
| scrollable={false} | |||
| > | |||
| {usageCollection ? ( | |||
There was a problem hiding this comment.
IIUC, usageCollection is always available, the feature flag only controls whether it can send metrics to server.
There was a problem hiding this comment.
usageCollection is declared as an optional plugin of dashboards-assistant plugin. Theoretically, it can be undefined.
| } | ||
|
|
||
| export const FeedbackThumbs = ({ usageCollection, appName, className }: Props) => { | ||
| const [feedback, setFeedback] = useState<'thumbs_up' | 'thumbs_down' | undefined>(); |
There was a problem hiding this comment.
If visualization is regenerated, will feedback be reset?
There was a problem hiding this comment.
Yes, it will reset because the FeedbackThumbs component will be re-rendered.
Signed-off-by: Yulong Ruan <[email protected]>
Description
[Describe what this change achieves]
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.