Track TensorBoard usage in integrated terminals #15125
Conversation
joyceerhl
requested review from
DonJayamanne,
DavidKutu,
IanMatthewHuff and
rchiodo
Codecov Report
@@ Coverage Diff @@
## main #15125 +/- ##
======================================
Coverage 65% 65%
======================================
Files 560 560
Lines 26362 26363 +1
Branches 3747 3747
======================================
+ Hits 17174 17175 +1
Misses 8494 8494
Partials 694 694
|
| // every 5 min or so | ||
| const matches = window.terminals.filter((terminal) => terminal.name === 'tensorboard'); | ||
| if (matches.length > 0) { | ||
| sendTelemetryEvent(EventName.TENSORBOARD_DETECTED_IN_INTEGRATED_TERMINAL); |
There was a problem hiding this comment.
I'm not fully sure what the PM requirements are on this. If we just care about seeing this once per VS Code session then we could just send this once and turn off the Watcher after that (just to avoid doing something every 5 min and to not log the event repeatedly). But this is just a smidge of work happening every five so seems ok to me as is.
There was a problem hiding this comment.
Good catch, checked with Jeffrey and we only care about seeing this once per VSCode session. Added clearInterval after sending telemetry (in addition to clearInterval on extension dispose).
| // until the user kills it, the terminal with the updated name should | ||
| // stick around for long enough that we only need to run this check | ||
| // every 5 min or so | ||
| const matches = window.terminals.filter((terminal) => terminal.name === 'tensorboard'); |
There was a problem hiding this comment.
Noob question: is the terminal name always going to be tensorboard, and not C://path/to/tensorboard, or tb or any other alias?
There was a problem hiding this comment.
It's possible that users alias the tensorboard command but might also result in false positives. For running with path/to/tensorboard, the terminal name is also tensorboard (at least on my machine).
There was a problem hiding this comment.
No tensorboard.exe on Windows then 👍
Per @jmew, collect telemetry on how often TensorBoard is started from an integrated terminal in VS Code. It's possible (but IMO unlikely) that a terminal would have that name and not be running tensorboard.
A more accurate but also more complicated and expensive alternative would be to poll the process tree under each terminal pty and check if any of the terminals' associated processes' children are running tensorboard.
[ ] Has sufficient logging.[ ] The wiki is updated with any design decisions/details.