{Core} Add SessionId for telemetry#15076
Conversation
houk-ms
requested review from
Juliehzl,
arrownj,
fengzhou-msft,
haroldrandom,
jiasli,
jsntcy and
mmyyrroonn
as code owners
|
Core |
| while process and process.ppid() and process.pid != process.ppid(): | ||
| parent_process = process.parent() | ||
| if parent_process and process.create_time() - parent_process.create_time() > time_threshold: | ||
| content = '{}{}{}'.format(_get_installation_id(), parent_process.create_time(), parent_process.pid) |
There was a problem hiding this comment.
I thought you mentioned will distinguish by terminal type right?
There was a problem hiding this comment.
It turns out to be an invalid method. For there may be the cases where a shell was called in another shell, e.g, powershell will call cmd to execute CLI command. So we can't recognize the target terminal processs only by the name.
I think the method I proposed here is better for practice. It can deal with all kinds of OSes and their different terminals.
There was a problem hiding this comment.
Please test below scenario:
- write a script which has following content, let's give it a name
script.sh
#!/bin/bash
az vm list -g resource_group # It's better that the time cost of this command is over 1 second.
az vm list -g resource_group
- open a terminal or tty and run following command
bash script.sh
check whether the sessionId returned by the 2 commands are the same.
There was a problem hiding this comment.
For automation script, the sessionIDs of the commands in script will be the same except for the first command. It only occurs on None-windwos OSes and only in automation scripts. It will be recorded as known issue before we can find a better method to get sessionID.
|
Hello, not sure if this is the right place to report this, but it seems this change introduced a regression issue which once more breaks building azure-cli with pip in cygwin, due to the re-introduction of Please let me know if you would like this reported in a new issue. |
|
@pspag Sorry for the inconvenience. We removed the psutil dependency for Cygwin in setup.py, but it only takes effect when using source installation. We will schedule to fix the issue as soon as we can. As a workaround to use the latest CLI in cygwin, try to
|
|
@houk-ms many thanks for your reply. No problem at all, since I came across this I just wanted to report it as I imagine it will be also affecting others. In the meantime I use 2.11.1 where this feature is not included. |
6 participants
Description
SessionId is used to help us identify whether the users are running their CLI commands in the same terminal. It can help us analysis the user behavior, by which we may be able to cluster the commands and improve CLI error rate measurement. It may also help us separate interactive users from automation scripts.
This PR
Note
The package psutil was remove from CLI dependencies in #11665 because of cygwin incompatibility. In this PR, we reintroduce the package and provide a workaround to avoid installing it in cygwin.
Known Issue
For special systems which do not support psutil like Cygwin, the sessionID can not be calculated. But it is set to empty, so that we can filter them easily.
For automation script, the sessionIDs of the commands in script will be the same except for the first command. It only occurs on None-windwos OSes and only in automation scripts.