Skip to content

{Core} Add SessionId for telemetry #15076

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.

Sign up for GitHub

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

Merged
houk-ms merged 3 commits into from
Sep 15, 2020
Merged

{Core} Add SessionId for telemetry #15076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9aaf5ae
add session id for Telemetry
houk-ms Sep 4, 2020
c0cfb6a
add psutil dependency
houk-ms Sep 8, 2020
e3ea8ea
fix code style
houk-ms Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion src/azure-cli-core/azure/cli/core/telemetry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def _get_base_properties(self):
'Reserved.ChannelUsed': 'AI',
'Reserved.EventId': str(uuid.uuid4()),
'Reserved.SequenceNumber': 1,
'Reserved.SessionId': str(uuid.uuid4()),
'Reserved.SessionId': _get_session_id(),
'Reserved.TimeSinceSessionStart': 0,

'Reserved.DataModel.Source': 'DataModelAPI',
Expand Down Expand Up @@ -407,6 +407,33 @@ def _get_installation_id():
return _get_profile().get_installation_id()


@decorators.suppress_all_exceptions(fallback_return="")
def _get_session_id():
# As a workaround to get the terminal info as SessionId, this function may not be accurate.

def get_hash_result(content):
import hashlib

hasher = hashlib.sha256()
hasher.update(content.encode('utf-8'))
return hasher.hexdigest()

# Usually, more than one layer of sub-process will be started when excuting a CLI command. While, the create time
# of these sub-processes will be very close, usually in several milliseconds. We use 1 second as the threshold here.
# When the difference of create time between current process and its parent process is larger than the threshold,
# the parent process will be viewed as the terminal process.
import psutil
time_threshold = 1
process = psutil.Process()
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)
Copy link
Member

@yungezz yungezz Sep 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you mentioned will distinguish by terminal type right?

Copy link
Contributor Author

@houk-ms houk-ms Sep 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@arrownj arrownj Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test below scenario:

  1. 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

  1. open a terminal or tty and run following command

bash script.sh

check whether the sessionId returned by the 2 commands are the same.

Copy link
Contributor Author

@houk-ms houk-ms Sep 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@arrownj arrownj Sep 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, you convince me. 👍

return get_hash_result(content)
process = parent_process
return ""


@decorators.call_once
@decorators.suppress_all_exceptions(fallback_return=None)
def _get_profile():
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/requirements.py3.Darwin.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ oauthlib==3.0.1
paramiko==2.6.0
pbr==5.3.1
portalocker==1.4.0
psutil==5.7.2
pycparser==2.19
PyJWT==1.7.1
PyNaCl==1.3.0
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/requirements.py3.Linux.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ oauthlib==3.0.1
paramiko==2.6.0
pbr==5.3.1
portalocker==1.4.0
psutil==5.7.2
pycparser==2.19
PyJWT==1.7.1
PyNaCl==1.3.0
Expand Down
1 change: 1 addition & 0 deletions src/azure-cli/requirements.py3.windows.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ oauthlib==3.0.1
paramiko==2.6.0
pbr==5.3.1
portalocker==1.2.1
psutil==5.7.2
pycparser==2.19
PyJWT==1.7.1
PyNaCl==1.3.0
Expand Down
4 changes: 4 additions & 0 deletions src/azure-cli/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@
'jsondiff==1.2.0'
]

# dependencies for specific OSes
if not sys.platform.startswith('cygwin'):
DEPENDENCIES.append('psutil~=5.7')

TESTS_REQUIRE = [
'mock~=4.0'
]
Expand Down