Skip to content

CP-45235: Support for xe-cli to transmit traceparent #5633

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
merged 1 commit into from
Jun 6, 2024
Merged

CP-45235: Support for xe-cli to transmit traceparent #5633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions ocaml/xapi-cli-server/xapi_cli.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ let with_session ~local rpc u p session f =
(fun () -> f session)
(fun () -> do_logout ())

let do_rpcs _req s username password minimal cmd session args =
let do_rpcs _req s username password minimal cmd session args tracing =
let cmdname = get_cmdname cmd in
let cspec =
try Hashtbl.find cmdtable cmdname
Expand All @@ -137,7 +137,8 @@ let do_rpcs _req s username password minimal cmd session args =
try
let generic_rpc = get_rpc () in
(* NB the request we've received is for the /cli. We need an XMLRPC request for the API *)
Tracing.with_tracing ~name:("xe " ^ cmdname) @@ fun tracing ->
Tracing.with_tracing ~parent:tracing ~name:("xe " ^ cmdname)
@@ fun tracing ->
let req = Xmlrpc_client.xmlrpc ~version:"1.1" ~tracing "/" in
let rpc = generic_rpc req s in
if do_forward then
Expand Down Expand Up @@ -188,6 +189,14 @@ let uninteresting_cmd_postfixes = ["help"; "-get"; "-list"]

let exec_command req cmd s session args =
let params = get_params cmd in
let tracing =
Option.bind
Http.Request.(req.traceparent)
Tracing.SpanContext.of_traceparent
|> Option.map (fun span_context ->
Tracing.Tracer.span_of_span_context span_context (get_cmdname cmd)
)
in
let minimal =
if List.mem_assoc "minimal" params then
bool_of_string (List.assoc "minimal" params)
Expand Down Expand Up @@ -248,7 +257,7 @@ let exec_command req cmd s session args =
params
)
) ;
do_rpcs req s u p minimal cmd session args
do_rpcs req s u p minimal cmd session args tracing

let get_line str i =
try
Expand Down
17 changes: 14 additions & 3 deletions ocaml/xe-cli/newcli.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ let xapipasswordfile = ref ""

let xapiport = ref None

let traceparent = ref None

let get_xapiport ssl =
match !xapiport with None -> if ssl then 443 else 80 | Some p -> p

Expand Down Expand Up @@ -66,7 +68,7 @@ exception Usage
let usage () =
error
"Usage: %s <cmd> [-s server] [-p port] ([-u username] [-pw password] or \
[-pwf <password file>]) <other arguments>\n"
[-pwf <password file>]) [--traceparent traceparent] <other arguments>\n"
Sys.argv.(0) ;
error
"\n\
Expand Down Expand Up @@ -208,6 +210,8 @@ let parse_args =
| "debugonfail" -> (
xedebugonfail := try bool_of_string v with _ -> false
)
| "traceparent" ->
traceparent := Some v
| _ ->
raise Not_found
) ;
Expand All @@ -234,6 +238,8 @@ let parse_args =
Some ("debugonfail", "true", xs)
| "-h" :: h :: xs ->
Some ("server", h, xs)
| "--traceparent" :: h :: xs ->
Some ("traceparent", h, xs)
| _ ->
None
in
Expand Down Expand Up @@ -286,6 +292,10 @@ let parse_args =
List.rev !l
in
let extras_rest = process_args extras in
(*if traceparent is set as env var update it after we process the extras.*)
Option.iter
(fun tp -> traceparent := Some tp)
Copy link
Member

Choose a reason for hiding this comment

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

The cli argument must take precedence over the environment variable, which usually takes preference over the contents of the configuration file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what it does: it sets the values from configuration files, then overwrites them with the values from environment variables and then again with values from the cli argument. process_args calls set_keyword to achieve this.

This line is separate because it reads and process TRACEPARENT env var after it processes XE_EXTRA_ARGS.

(Sys.getenv_opt Tracing.EnvHelpers.traceparent_key) ;
let help = ref false in
let args' = List.filter (fun s -> s <> "-help" && s <> "--help") args in
if List.length args' < List.length args then help := true ;
Expand All @@ -300,7 +310,7 @@ let parse_args =
debug_channel := Some tmpch
)
in
args_rest @ extras_rest @ rcs_rest @ !reserve_args
(args_rest @ extras_rest @ rcs_rest @ !reserve_args, !traceparent)

let exit_status = ref 1

Expand Down Expand Up @@ -790,7 +800,7 @@ let main () =
Printf.printf "ThinCLI protocol: %d.%d\n" major minor ;
exit 0
) ;
let args = parse_args args in
let args, traceparent = parse_args args in
(* All the named args are taken as permitted filename to be uploaded *)
let permitted_filenames = get_permit_filenames args in
if List.length args < 1 then
Expand All @@ -803,6 +813,7 @@ let main () =
in
let args = String.concat "\n" args in
Printf.fprintf oc "User-agent: xe-cli/Unix/%d.%d\r\n" major minor ;
Option.iter (Printf.fprintf oc "traceparent: %s\r\n") traceparent ;
Printf.fprintf oc "content-length: %d\r\n\r\n" (String.length args) ;
Printf.fprintf oc "%s" args ;
flush_all () ;
Expand Down