CP-46377: Add env vars TRACEPARENT & OBSERVER_CONFIG_DIR to exec_xmlrpc calls
#5345
Conversation
GabrielBuica
force-pushed
the
private/dbuica/CP-46377
branch
2 times, most recently
from
f539910 to
5ba8c33
Compare
GabrielBuica
force-pushed
the
private/dbuica/CP-46377
branch
from
6a5d418 to
bdce7b5
Compare
GabrielBuica
force-pushed
the
private/dbuica/CP-46377
branch
from
b9e3d63 to
beb72e0
Compare
| raise Api_errors.(Server_error (invalid_value, ["component"; component])) | ||
|
|
||
| let observed_components_of components = | ||
| match components with [] -> startup_components () | components -> components |
There was a problem hiding this comment.
This looks dubious to me. How can an empty argument observe anything? I think the handling of the empty list is unexpected for clients.
There was a problem hiding this comment.
As far as I understand it, it is because of how things are stored in the database. When you create an observer and you don't specify the components, the expectation is that all components are enabled. Querying the database for this observer returns empty. I don't know the details of why was implemented like this, but I agree with it being annoying for clients.
There was a problem hiding this comment.
The problem with the current design is that you can't have no components being observed because the empty list maps to all components. I think @mg12 should review this. It would be cleaner to use an option:
let observed_components_of = function
| None -> startup_components ()
| Some comps -> comps
Now one could calls observed_components of (Some []) and get back the empty list. Alternatively the behaviour of the empty list needs to be documented.
There was a problem hiding this comment.
@lindig I'll talk to @GabrielBuica about this. I think we should handle this specific change in a future PR, as this part is in the code for a long time and it's not related to the traceparent change of this PR.
GabrielBuica
force-pushed
the
private/dbuica/CP-46377
branch
2 times, most recently
from
e2a5cb4 to
92aa095
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5345 +/- ##
=======================================
Coverage 45.38% 45.38%
=======================================
Files 18 18
Lines 2937 2937
=======================================
Hits 1333 1333
Misses 1604 1604
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Moves `module Component` in a separate file `xapi_observer_components.ml`. Moves `module EnvRecord` in a seperate file `xapi_aux/env_record.ml` Basic funcionality from `Xapi_observer` cannot be used from `sm_exec.ml` because cycle dependencies. This commit does some housekeeping and splits `xapi_observer.ml` into `xapi_observer.ml` and `xapi_observer_components.ml` while also moving `module EnvRecord` under `xapi_aux`. `xapi_observer_components.ml` also contains the functions that act on components independent of the observers. Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Adds a `xapi_observer_component.ml` function `is_component_enabled` -> `component:t` --> `bool`. Returns `true` if the given `component` is enabled in any of the observers. A helper function is needed to check if `SMapi` component is enabled in any of the observers. Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
…_exec` calls Adds `Tracing.SpanContext.trace_id_of_span_context` as an env variable. Adds a new entry `observer_config_dir` in `xapi_globs.ml`. Sets this new entry as an env variable. Currently there is no path for SMAPI `observer.py` calls to access their respective `TRACEPARENT`, nor to access the configuration files for observers. This commit sets: - `TRACEPARENT` as an environment variable if any smapi observer component is enabled; - `OBSERVER_CONFIG_DIR` as an environment variable if any smapi observer component is enabled. This contains the path to the enabled observers configuration files. Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Defines `default_path_env_pair` as the default value used by optional parameter `?env`. External clients of `Forkhelpers` need to include the value `default_path_env_pair` when passing an argument to `?env`. Now this value is defined inside the module for client use. Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Moves `config_root` under `observer_confi_dir` in `xapi_globs.ml`. Adds `observer_conf_path` function that constructs the path based on `uuid`. Removes `~config_root` parameter. There were various ways to define paths is `Dom0ObserverConfig` which increase the probabilty of introducing bugs in the future. This consolidates them into `observer_conf_path_of`. Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Splits functionality `env_vars_of_observer` into `traceparent_of_dbg` under `debuginfo.ml` and `env_vars_of_component` under `xapi_observer_components.ml. Doing the processing of traceparent and of the necessary observer env vars under their respective modules increases the readablity of `sm_exec.ml`. Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
GabrielBuica
force-pushed
the
private/dbuica/CP-46377
branch
from
449111a to
b5ce70d
Compare
|
Passed bvt after addressing comments and rebasing. |
pytype_reporter extracted 49 problem reports from pytype output.You can check the results of the job here |
6 participants
Sets up the necessary environment variables,
TRACEPARENTandOBSERVER_CONFIG_DIRfor SMAPIobserver.pyconfiguration.