-
Notifications
You must be signed in to change notification settings - Fork 196
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
[LOPS-2313] Provide a trace id when calling remote commands #2589
Conversation
LGTM when the tests pass. |
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.
Update to use our TraceId class in the Request class. Also, we should only show the trace id in --debug mode. Users typically do not care about this value; it's for OCE / Plat engineers diagnosing problems.
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.
Only 1 small comment but LGTM :)
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.
We might want to go back to prepending the TRACE_ID to the beginning of the command line. Let's hold this PR until we decide what's easiest to handle on the back end.
|
||
$ssh_data = $this->sendCommandViaSsh($command_line); | ||
// Separate the command line and environment variables | ||
list($command_line, $env_vars) = $this->getCommandLine($command_args, $env_vars); |
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.
Don't need to return $env_vars, since they are passed in.
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.
This still isn't resolved, and getCommandLine does not include $env_vars at the beginning of the line
Description:
Screen.Recording.2024-05-29.at.10.48.02.AM.mov