diff --git a/.codecov.yml b/.codecov.yml index 8380434a2a5..9be7955160d 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -158,6 +158,31 @@ coverage: # threshold: 20% + python3: + + # + # The python3 limit applies to: + # ----------------------------- + # + # - python3/** + # - excluding: **/test_*.py + # + paths: ["python3/**", "!**/test_*.py"] + + # + # For python3/** (excluding tests): + # + # For python3, coverage should not be reduced compared to its base: + # + target: auto + + # + # Exception: the threshold value given is allowed + # + # Allows for not covering 20% if the changed lines of the PR: + # + threshold: 20% + # Checks each Python version separately: python-3.11: flags: ["python3.11"] @@ -175,18 +200,26 @@ coverage: # Python modules and scripts below scripts/ (excluding tests) # scripts: + paths: ["scripts/**", "!**/test_*.py"] target: 48% threshold: 2% - paths: ["scripts/**", "!**/test_*.py"] # - # Python modules and scripts below ocaml/ + # Python modules and scripts below ocaml/ (excluding tests) # ocaml: paths: ["ocaml/**", "!**/test_*.py"] target: 51% threshold: 3% + # + # Python modules and scripts below python3/ (excluding tests) + # + python3: + paths: ["python3/**", "!**/test_*.py"] + target: 48% + threshold: 2% + # # Test files # @@ -239,6 +272,10 @@ component_management: - "ocaml/xapi-storage-script/**" - "!**/test_*.py" + - component_id: python3 + name: python3 + paths: ["python3/**", "!**/test_*.py"] + - component_id: test_cases name: test_cases paths: ["**/test_*.py"] diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cadf84c35c4..6662dc218ea 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -51,12 +51,13 @@ jobs: - name: Install dependencies only needed for python 3 if: ${{ matrix.python-version != '2.7' }} - run: pip install pandas pytype toml + run: pip install pandas pytype toml wrapt - name: Install common dependencies for Python ${{matrix.python-version}} run: pip install future mock pytest-coverage pytest-mock - - name: Run Pytest and get code coverage for Codecov + - name: Run Pytest for python 2 and get code coverage for Codecov + if: ${{ matrix.python-version == '2.7' }} run: > pytest --cov=scripts --cov=ocaml/xcp-rrdd @@ -67,6 +68,18 @@ jobs: env: PYTHONDEVMODE: yes + - name: Run Pytest for python 3 and get code coverage for Codecov + if: ${{ matrix.python-version != '2.7' }} + run: > + pytest + --cov=scripts --cov=ocaml/xcp-rrdd --cov=python3/ + scripts/ ocaml/xcp-rrdd python3/ -vv -rA + --junitxml=.git/pytest${{matrix.python-version}}.xml + --cov-report term-missing + --cov-report xml:.git/coverage${{matrix.python-version}}.xml + env: + PYTHONDEVMODE: yes + - name: Upload Python ${{matrix.python-version}} coverage report to Codecov uses: codecov/codecov-action@v3 with: diff --git a/Makefile b/Makefile index bcfc5b9eb78..921e65923de 100644 --- a/Makefile +++ b/Makefile @@ -147,6 +147,7 @@ install: build doc sdk doc-json mkdir -p $(DESTDIR)/etc/bash_completion.d # ocaml/xapi make -C scripts install + make -C python3 install cp -f _build/install/default/bin/xapi $(DESTDIR)$(OPTDIR)/bin/xapi scripts/install.sh 755 ocaml/quicktest/quicktest $(DESTDIR)$(OPTDIR)/debug cp -f _build/install/default/bin/quicktestbin $(DESTDIR)$(OPTDIR)/debug/quicktestbin diff --git a/ocaml/libs/tracing/tracing.ml b/ocaml/libs/tracing/tracing.ml index e128c510522..87326fb65cf 100644 --- a/ocaml/libs/tracing/tracing.ml +++ b/ocaml/libs/tracing/tracing.ml @@ -143,7 +143,7 @@ end module SpanContext = struct type t = {trace_id: string; span_id: string} [@@deriving rpcty] - let to_traceparent t = Printf.sprintf "00-%s-%s-00" t.trace_id t.span_id + let to_traceparent t = Printf.sprintf "00-%s-%s-01" t.trace_id t.span_id let of_traceparent traceparent = let elements = String.split_on_char '-' traceparent in @@ -722,6 +722,8 @@ module Export = struct let set_trace_log_dir dir = trace_log_dir := dir + let get_trace_log_dir () = !trace_log_dir + let set_max_file_size size = max_file_size := size let set_compress_tracing_files enabled = compress_tracing_files := enabled diff --git a/ocaml/libs/tracing/tracing.mli b/ocaml/libs/tracing/tracing.mli index 8b5940fb3cf..ee30b29f041 100644 --- a/ocaml/libs/tracing/tracing.mli +++ b/ocaml/libs/tracing/tracing.mli @@ -153,6 +153,8 @@ module Export : sig val set_trace_log_dir : string -> unit + val get_trace_log_dir : unit -> string + val set_compress_tracing_files : bool -> unit end diff --git a/ocaml/tests/test_observer.ml b/ocaml/tests/test_observer.ml index 4da9b324388..69c2f754c93 100644 --- a/ocaml/tests/test_observer.ml +++ b/ocaml/tests/test_observer.ml @@ -82,7 +82,13 @@ module TracerProvider = struct () with _ -> Alcotest.failf "Missing mandatory attribute: %s" x ) - ["xs.pool.uuid"; "xs.host.name"; "xs.host.uuid"; "xs.observer.name"] + [ + "xs.pool.uuid" + ; "xs.host.name" + ; "xs.host.uuid" + ; "xs.observer.name" + ; "service.name" + ] let check_endpoints ~name ~endpoints = let provider = find_provider_exn ~name in @@ -287,6 +293,7 @@ let verify_json_fields_and_values ~json = ; ("xs.observer.name", `String "test-observer") ; ("xs.host.uuid", `String _) ; ("xs.host.name", `String _) + ; ("service.name", `String _) ] ) ; ("annotations", `List _) diff --git a/ocaml/xapi/xapi_observer.ml b/ocaml/xapi/xapi_observer.ml index 3c5992290cd..eff01624156 100644 --- a/ocaml/xapi/xapi_observer.ml +++ b/ocaml/xapi/xapi_observer.ml @@ -220,6 +220,19 @@ module Xapi_cluster = struct end end +let default_attributes ~__context ~host ~name_label ~component = + let pool = Helpers.get_pool ~__context in + let host_label = Db.Host.get_name_label ~__context ~self:host in + let host_uuid = Db.Host.get_uuid ~__context ~self:host in + let pool_uuid = Db.Pool.get_uuid ~__context ~self:pool in + [ + ("xs.pool.uuid", pool_uuid) + ; ("xs.host.name", host_label) + ; ("xs.host.uuid", host_uuid) + ; ("xs.observer.name", name_label) + ; ("service.name", to_string component) + ] + module ObserverConfig = struct type t = { otel_service_name: string @@ -235,7 +248,7 @@ module ObserverConfig = struct let rec bugtool_endpoint endpoints = match endpoints with | x :: _ when x = Tracing.bugtool_name -> - Some x + Some (Tracing.Export.Destination.File.get_trace_log_dir ()) | _ :: t -> bugtool_endpoint t | [] -> @@ -247,12 +260,18 @@ module ObserverConfig = struct attrs let config_of_observer ~__context ~component ~observer = + (* In the future this should be updated so that the config is read + from and updated instead of being regenerated. *) let endpoints = Db.Observer.get_endpoints ~__context ~self:observer in + let host = Helpers.get_localhost ~__context in + let name_label = Db.Observer.get_name_label ~__context ~self:observer in { - otel_service_name= component + otel_service_name= to_string component ; otel_resource_attributes= attributes_to_W3CBaggage - (Db.Observer.get_attributes ~__context ~self:observer) + (Db.Observer.get_attributes ~__context ~self:observer + @ default_attributes ~__context ~host ~name_label ~component + ) ; xs_exporter_zipkin_endpoints= zipkin_endpoints endpoints ; xs_exporter_bugtool_endpoint= bugtool_endpoint endpoints } @@ -292,8 +311,7 @@ module Dom0ObserverConfig (ObserverComponent : OBSERVER_COMPONENT) : if Db.Observer.get_enabled ~__context ~self:observer then ( let observer_config = ObserverConfig.config_of_observer ~__context - ~component:(to_string ObserverComponent.component) - ~observer + ~component:ObserverComponent.component ~observer in Xapi_stdext_unix.Unixext.mkdir_rec dir_name 0o755 ; let file_name = observer_conf_path_of ~uuid in @@ -429,22 +447,10 @@ let assert_valid_attributes attributes = ) attributes -let default_attributes ~__context ~host ~name_label = - let pool = Helpers.get_pool ~__context in - let host_label = Db.Host.get_name_label ~__context ~self:host in - let host_uuid = Db.Host.get_uuid ~__context ~self:host in - let pool_uuid = Db.Pool.get_uuid ~__context ~self:pool in - [ - ("xs.pool.uuid", pool_uuid) - ; ("xs.host.name", host_label) - ; ("xs.host.uuid", host_uuid) - ; ("xs.observer.name", name_label) - ] - let register_component ~__context ~self ~host ~component = let name_label = Db.Observer.get_name_label ~__context ~self in let attributes = - default_attributes ~__context ~host ~name_label + default_attributes ~__context ~host ~name_label ~component @ Db.Observer.get_attributes ~__context ~self in let uuid = Db.Observer.get_uuid ~__context ~self in @@ -603,13 +609,15 @@ let set_attributes ~__context ~self ~value = let uuid = Db.Observer.get_uuid ~__context ~self in let host = Helpers.get_localhost ~__context in let name_label = Db.Observer.get_name_label ~__context ~self in - let default_attributes = default_attributes ~__context ~host ~name_label in let observation_fn () = List.iter (fun c -> let module Forwarder = (val get_forwarder c : ObserverInterface) in Forwarder.set_attributes ~__context ~uuid - ~attributes:(default_attributes @ value) + ~attributes: + (default_attributes ~__context ~host ~name_label ~component:c + @ value + ) ) (Db.Observer.get_components ~__context ~self |> List.map of_string diff --git a/pyproject.toml b/pyproject.toml index b65a36bb062..505f16e430c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,8 +39,8 @@ ensure_newline_before_comments = false # Note mypy has no config setting for PYTHONPATH, so you need to call it with: # PYTHONPATH="scripts/examples/python:.:scripts:scripts/plugins:scripts/examples" files = [ + "python3", "scripts/usb_reset.py", - "scripts/unit_tests", ] pretty = true error_summary = true @@ -113,6 +113,7 @@ inputs = [ "scripts/examples/python", "scripts/yum-plugins", "scripts/*.py", + "python3/packages/*.py", # To be added later, # when converted to Python3-compatible syntax: diff --git a/python3/Makefile b/python3/Makefile new file mode 100644 index 00000000000..480aee5dfeb --- /dev/null +++ b/python3/Makefile @@ -0,0 +1,9 @@ +include ../config.mk + +SITE3_DIR=$(shell python3 -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())") + +IDATA=install -m 644 + +install: + mkdir -p $(DESTDIR)$(SITE3_DIR) + $(IDATA) packages/observer.py $(DESTDIR)$(SITE3_DIR)/ diff --git a/python3/README.md b/python3/README.md new file mode 100644 index 00000000000..e2bf8dcc464 --- /dev/null +++ b/python3/README.md @@ -0,0 +1,12 @@ +# Python3 Scripts + +This directory is intended for scripts that only run on python3. As more scripts are +ported to python3 this directory should start to fill up. The intended structure of +the directory is as follows: + +- bin: This contains files to be installed in bin and are meant to be run by users +- libexec: This contains files to be installed in libexec and are meant to only be +run by xapi and other daemons. +- packages: This contains files to be installed in python's site-packages and are meant +to be modules and packages to be imported by other scripts or executed via python3 -m +- plugins: This contains files that are meant to be xapi plugins diff --git a/python3/packages/__init__.py b/python3/packages/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python3/packages/observer.py b/python3/packages/observer.py new file mode 100644 index 00000000000..1eee0f2a7d6 --- /dev/null +++ b/python3/packages/observer.py @@ -0,0 +1,418 @@ +# Copyright (C) Cloud Software Group. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published +# by the Free Software Foundation; version 2.1 only. with the special +# exception on linking described in file LICENSE. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# + +""" +Calls the passed script with its original arguments, instrumenting it to make a +trace of all function calls if at least one *observer.conf file exists in the +OBSERVER_CONFIG_DIR directory. + +If there are no *observer.conf files or something fails, this script runs the +passed script without any instrumentation. +""" + +import configparser +import functools +import inspect +import logging +import os +import runpy +import sys +import traceback +from datetime import datetime, timezone +from logging.handlers import SysLogHandler +from typing import List, Sequence + +# The opentelemetry library may generate exceptions we aren't expecting, this code +# must not fail or it will cause the pass-through script to fail when at worst +# this script should be a noop. As such, we sometimes need to catch broad exceptions: +# pylint: disable=broad-exception-caught, too-many-locals, too-many-statements +# wrapt.decorator adds the extra parameters so we shouldn't provide them: +# pylint: disable=no-value-for-parameter +# We only want to import opentelemetry libraries if instrumentation is enabled +# pylint: disable=import-outside-toplevel + +DEBUG_ENABLED = False +DEFAULT_MODULES = "LVHDSR,XenAPI,SR,SRCommand,util" +FORMAT = "observer.py: %(message)s" +handler = SysLogHandler(facility="local5", address="/dev/log") +logging.basicConfig(format=FORMAT, handlers=[handler]) +syslog = logging.getLogger(__name__) +if DEBUG_ENABLED: + syslog.setLevel(logging.DEBUG) +else: + syslog.setLevel(logging.INFO) +debug = syslog.debug + + +def _get_configs_list(config_dir): + try: + # There can be many observer config files in the configuration directory + return [ + f"{config_dir}/{f}" + for f in os.listdir(config_dir) + if os.path.isfile(os.path.join(config_dir, f)) + and f.endswith("observer.conf") + ] + except FileNotFoundError as err: + debug("configs exception: %s", err) + return [] + + +def read_config(config_path, header): + """Read a config file and return a dictionary of key-value pairs.""" + + parser = configparser.ConfigParser() + with open(config_path, encoding="utf-8") as config_file: + try: + parser.read_string(f"[{header}]\n{config_file.read()}") + except configparser.ParsingError as e: + debug("read_config(): invalid config file %s: %s", config_path, e) + return {} + + config = {k: v.strip("'") for k, v in dict(parser[header]).items()} + debug("%s: %s", config_path, config) + return config + + +def _span_noop(wrapped=None, span_name_prefix=""): + """Noop decorator. Overridden by _init_tracing() if there are configs.""" + if wrapped is None: + return functools.partial(_span_noop, span_name_prefix=span_name_prefix) + + return wrapped + + +def _patch_module_noop(_): + """Noop patch_module. Overridden by _init_tracing() if there are configs.""" + + +def _init_tracing(configs: List[str], config_dir: str): + """ + Initialise tracing with the given configuration files. + + If configs is empty, return the noop span and patch_module functions. + If configs are passed: + - Import the opentelemetry packages + - Read the configuration file + - Create a tracer + - Trace the script + - Return the span and patch_module functions for tracing the program + """ + if not configs: + return _span_noop, _patch_module_noop + + try: + from warnings import simplefilter + + # On 3.10-3.12, the import of wrapt might trigger warnings, filter them: + simplefilter(action="ignore", category=DeprecationWarning) + import wrapt # type: ignore[import-untyped] + from opentelemetry import context, trace + from opentelemetry.baggage.propagation import W3CBaggagePropagator + from opentelemetry.exporter.zipkin.json import ZipkinExporter + from opentelemetry.sdk.resources import Resource + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import BatchSpanProcessor, SpanExportResult + from opentelemetry.trace.propagation.tracecontext import ( + TraceContextTextMapPropagator, + ) + except ImportError as err: + syslog.error("missing opentelemetry dependencies: %s", err) + return _span_noop, _patch_module_noop + + try: + config_dict = read_config(f"{config_dir}/all.conf", header="default") + except FileNotFoundError: + config_dict = {} + module_names = config_dict.get("module_names", DEFAULT_MODULES).split(",") + debug("module_names: %s", module_names) + + # pylint: disable=too-few-public-methods + class FileZipkinExporter(ZipkinExporter): + """Class to export spans to a file in Zipkin format.""" + + def __init__(self, *args, **kwargs): + self.bugtool_filename_callback = kwargs.pop("filename_callback") + self.bugtool_filename = self.bugtool_filename_callback() + self.trace_log_dir = kwargs.pop("trace_log_dir") + debug("bugtool filename=%s", self.bugtool_filename) + self.bytes_written = 0 + super().__init__(*args, **kwargs) + + def export(self, spans: Sequence[trace.Span]) -> SpanExportResult: + """Export the given spans to the file endpoint.""" + + data = self.encoder.serialize(spans, self.local_node) + datastr = str(data) + debug("data.type=%s,data.len=%s", type(data), len(datastr)) + debug("data=%s", datastr) + os.makedirs(name=self.trace_log_dir, exist_ok=True) + + with open(self.bugtool_filename, "a", encoding="utf-8") as bugtool_file: + bugtool_file.write(f"{datastr}\n") # ndjson + self.bytes_written += len(data) + + # Create new file if it gets > 1MB + if self.bytes_written > 1024 * 1024: + self.bugtool_filename = self.bugtool_filename_callback() + self.bytes_written = 0 + + return SpanExportResult.SUCCESS + + def create_tracer_from_config(path): + """Create a tracer from a config file.""" + + otelvars = "opentelemetry-python.readthedocs.io/en/latest/sdk/environment_variables.html" + config = read_config(path, header=otelvars) + config_otel_resource_attrs = config.get("otel_resource_attributes", "") + + if config_otel_resource_attrs: + # OTEL requires some attributes e.g. service.name + # to be in the environment variable + os.environ["OTEL_RESOURCE_ATTRIBUTES"] = config_otel_resource_attrs + + trace_log_dir = config.get("xs_exporter_bugtool_endpoint", "") + + zipkin_endpoints = config.get("xs_exporter_zipkin_endpoints") + otel_exporter_zipkin_endpoints = ( + zipkin_endpoints.split(",") if zipkin_endpoints else [] + ) + otel_resource_attrs = dict( + item.split("=") + for item in config.get("otel_resource_attributes", "").split(",") + if "=" in item + ) + + service_name = config.get( + "otel_service_name", otel_resource_attrs.get("service.name", "unknown") + ) + host_uuid = otel_resource_attrs.get("xs.host.uuid", "unknown") + # Remove . to prevent users changing directories in the bugtool_filenamer + tracestate = os.getenv("TRACESTATE", "unknown").strip("'").replace(".", "") + + # rfc3339 + def bugtool_filenamer(): + """Return an rfc3339-compliant ndjson file name.""" + now = datetime.now(timezone.utc).isoformat() + return ( + f"{trace_log_dir}/{service_name}-{host_uuid}-{tracestate}-{now}.ndjson" + ) + + traceparent = os.getenv("TRACEPARENT", None) + propagator = TraceContextTextMapPropagator() + context_with_traceparent = propagator.extract({"traceparent": traceparent}) + + context.attach(context_with_traceparent) + + # Create a tracer provider with the given resource attributes + provider = TracerProvider( + resource=Resource.create( + W3CBaggagePropagator().extract({}, otel_resource_attrs) + ) + ) + + # Add a span processor for each endpoint defined in the config + if trace_log_dir: + processor_file_zipkin = BatchSpanProcessor( + FileZipkinExporter( + filename_callback=bugtool_filenamer, + trace_log_dir=trace_log_dir + ) + ) + provider.add_span_processor(processor_file_zipkin) + for zipkin_endpoint in otel_exporter_zipkin_endpoints: + processor_zipkin = BatchSpanProcessor( + ZipkinExporter(endpoint=zipkin_endpoint) + ) + provider.add_span_processor(processor_zipkin) + + trace.set_tracer_provider(provider) + return trace.get_tracer(__name__) + + tracers = list(map(create_tracer_from_config, configs)) + debug("tracers=%s", tracers) + + def span_of_tracers(wrapped=None, span_name_prefix=""): + """ + Public decorator that creates a trace around a function. + + If there are no tracers, the function is called without any tracing. + If there are tracers, the function is called with a trace around it. + + It creates a span with the given span name prefix and then clones + the returned span for each of the existing traces to produce a nested + trace for each of them. + + Args: + wrapped: The function to be traced. + span_name_prefix: The prefix to be added to the span name. + + Returns: + The decorated function or a partial function if wrapped is None. + + If wrapped is None, the decorator is being used with parameters and + a partial function is returned instead of the decorated function so + that the function is decorated properly on the second pass. + """ + if wrapped is None: # handle decorators with parameters + return functools.partial(span_of_tracers, span_name_prefix=span_name_prefix) + + @wrapt.decorator + def instrument_function(wrapped, _, args, kwargs): + """Decorator that creates a trace around a function.""" + if not tracers: + return wrapped(*args, **kwargs) + + module_name = wrapped.__module__ if hasattr(wrapped, "__module__") else "" + qual_name = wrapped.__qualname__ if hasattr(wrapped, "__qualname__") else "" + + if not module_name and not qual_name: + span_name = str(wrapped) + else: + prefix = f"{span_name_prefix}:" if span_name_prefix else "" + span_name = f"{prefix}{module_name}:{qual_name}" + + tracer = tracers[0] + with tracer.start_as_current_span(span_name) as aspan: + if inspect.isclass(wrapped): + # class or classmethod + aspan.set_attribute("xs.span.args.str", str(args)) + aspan.set_attribute("xs.span.kwargs.str", str(kwargs)) + else: + # function, staticmethod or instancemethod + bound_args = inspect.signature(wrapped).bind(*args, **kwargs) + bound_args.apply_defaults() + for k, v in bound_args.arguments.items(): + aspan.set_attribute(f"xs.span.arg.{k}", str(v)) + + # must be inside "aspan" to produce nested trace + result = wrapped(*args, **kwargs) + return result + + def autoinstrument_class(aclass): + """Auto-instrument a class.""" + + t = tracers[0] + module_name = f"{aclass.__module__}:{aclass.__qualname__}" + + with t.start_as_current_span(f"auto_instrumentation.add: {module_name}"): + for method_name, method in aclass.__dict__.items(): + if not callable(getattr(aclass, method_name)): + continue + + with t.start_as_current_span( + f"class.instrument:{module_name}.{method_name}={method}" + ): + # Avoid RecursionError: + # 'maximum recursion depth exceeded in comparison' + # in the XenAPI module (triggered by XMLRPC calls in it): + if method_name in ["__getattr__", "__call__", "__init__"]: + continue + try: + setattr(aclass, method_name, instrument_function(method)) + except Exception: + debug( + "setattr.instrument_function: Exception %s", + traceback.format_exc(), + ) + + + def autoinstrument_module(amodule): + """Autoinstrument the classes and functions in a module.""" + + # Instrument the methods of the classes in the module + for _, aclass in inspect.getmembers(amodule, inspect.isclass): + try: + autoinstrument_class(aclass) + except Exception: + debug("instrument_function: Exception %s", traceback.format_exc()) + + # Instrument the module-level functions of the module + for fname, afunction in inspect.getmembers(amodule, inspect.isfunction): + setattr(amodule, fname, instrument_function(afunction)) + + if inspect.ismodule(wrapped): + autoinstrument_module(wrapped) + + return instrument_function(wrapped) + + def _patch_module(module_name): + wrapt.importer.discover_post_import_hooks(module_name) + wrapt.importer.when_imported(module_name)( + lambda hook: span_of_tracers(wrapped=hook) + ) + + for m in module_names: + _patch_module(m) + + return span_of_tracers, _patch_module + + +observer_config_dir = os.getenv("OBSERVER_CONFIG_DIR", default=".").strip("'") +observer_configs = _get_configs_list(observer_config_dir) +debug("configs = %s", observer_configs) + +try: + # If there are configs, span and patch_module are now operational + # and can be used to trace the program. + # If there are no configs, or an exception is raised, span and patch_module + # are not overridden and will be the defined no-op functions. + span, patch_module = _init_tracing(observer_configs, observer_config_dir) +except Exception as exc: + syslog.error("Exception while setting up tracing, running script untraced: %s", exc) + span, patch_module = _span_noop, _patch_module_noop + + +def main(): + """ + Run the passed python script using the runpy module, passing the given arguments. + + The program will be automatically instrumented when the corresponding module + in the program is imported. + """ + + # When sys.argv has only argv[0], but no command to call, exit with an error message + if len(sys.argv) < 2: + print(__file__ + ": usage: command argument list", file=sys.stderr) + return 31 # EINVAL + + # Shift the arguments by one so that the program to run is first in sys.argv + sys.argv = sys.argv[1:] + argv0 = sys.argv[0] + + @span(span_name_prefix=argv0) + def run(file): + """Run the given python file calling its __main__ function.""" + + # Defensive error handling should hopefully only be needed in exceptional cases + # but in case things go wrong, this may be a starting point for logging it: + try: + runpy.run_path(file, run_name="__main__") + return 0 + except FileNotFoundError as e: + print( + f"{__file__}: {' '.join(sys.argv)}\n{e.filename}: No such file", + file=sys.stderr, + ) + return 2 + except Exception: + print( + f"{__file__}: {' '.join(sys.argv)}\n{traceback.format_exc()}", + file=sys.stderr) + return 139 # This is what the default SIGSEGV handler on Linux returns + + return run(argv0) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/python3/tests/__init__.py b/python3/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python3/tests/test_observer.py b/python3/tests/test_observer.py new file mode 100644 index 00000000000..53944d97ca9 --- /dev/null +++ b/python3/tests/test_observer.py @@ -0,0 +1,133 @@ +"""Test python3/packages/observer.py""" + +import os +import sys +import unittest + +from mock import MagicMock, mock_open, patch + +# Ensure observer is initialised as noop +with patch("os.listdir") as mock_listdir: + # Prevent it finding an observer.conf + mock_listdir.return_value = [] + from packages import observer + +# mock modules to avoid dependencies +sys.modules["opentelemetry"] = MagicMock() +sys.modules["opentelemetry.sdk.resources"] = MagicMock() +sys.modules["opentelemetry.sdk.trace"] = MagicMock() +sys.modules["opentelemetry.sdk.trace.export"] = MagicMock() +sys.modules["opentelemetry.exporter.zipkin.json"] = MagicMock() +sys.modules["opentelemetry.baggage.propagation"] = MagicMock() +sys.modules["opentelemetry.trace.propagation.tracecontext"] = MagicMock() +sys.modules["opentelemetry.context"] = MagicMock() +sys.modules["opentelemetry.trace"] = MagicMock() + +TEST_CONFIG = """ + XS_EXPORTER_BUGTOOL_ENDPOINT='/var/log/dt/test' + OTEL_SERVICE_NAME='test-observer' + OTEL_RESOURCE_ATTRIBUTES='service.name=sm' + """ +TEST_OBSERVER_CONF = "test-observer.conf" +OBSERVER_OPEN = "packages.observer.open" + + +# pylint: disable=missing-function-docstring,protected-access +class TestObserver(unittest.TestCase): + """Test python3/packages/observer.py""" + + def simple_method(self): + """A simple helper method for tests to wrap using observer.span""" + return 5 + + def init_tracing_and_run_simple_method(self, read_data): + """Run the init_tracing method with the given read_data""" + + with patch(OBSERVER_OPEN, mock_open(read_data=read_data)): + span, _ = observer._init_tracing([TEST_OBSERVER_CONF], ".") + + simple_method = span(self.simple_method) + self.assertEqual(simple_method(), 5) + + def test_span_with_parameters(self): + span, _ = observer._init_tracing([], ".") + + # This needs to use the decorator sugar so that wrapped is initially None + @span(span_name_prefix="test") + def simple_method(): + return 5 + + self.assertEqual(simple_method(), 5) + + def test_span_of_tracers_with_parameters(self): + with patch(OBSERVER_OPEN, mock_open(read_data="empty")): + span, _ = observer._init_tracing([TEST_OBSERVER_CONF], ".") + + # This needs to use the decorator sugar so that wrapped is initially None + @span(span_name_prefix="test") + def simple_method(): + return 5 + + self.assertEqual(simple_method(), 5) + + # Use the span method defined on import with no configs + def test_span_after_import(self): + simple_method = observer.span(self.simple_method) + + self.assertEqual(simple_method(), 5) + + # Check the span is a noop when configs is empty + def test_configs_empty(self): + span, _ = observer._init_tracing([], ".") + + simple_method = span(self.simple_method) + + self.assertEqual(simple_method(), 5) + + @patch("os.path.isfile") + @patch("os.listdir") + def test_get_configs_list(self, mock_ls, mock_isfile): + mock_ls.return_value = [ + "first-observer.conf", + "ignore.conf", + "second-observer.conf", + ] + mock_isfile.return_value = True + configs = observer._get_configs_list("test-dir") + + self.assertEqual( + configs, ["test-dir/first-observer.conf", "test-dir/second-observer.conf"] + ) + + def test_configs_directory_not_found(self): + configs = observer._get_configs_list("non-existent") + + self.assertEqual(configs, []) + + def test_configs_exist(self): + self.init_tracing_and_run_simple_method(read_data=TEST_CONFIG) + self.assertEqual(os.environ["OTEL_RESOURCE_ATTRIBUTES"], "service.name=sm") + + def test_all_conf_missing(self): + with patch(OBSERVER_OPEN) as mock_file: + mock_file.return_value.__enter__.side_effect = [ + FileNotFoundError, + mock_open(read_data="empty").return_value, + ] + span, _ = observer._init_tracing([TEST_OBSERVER_CONF], ".") + + simple_method = span(self.simple_method) + + self.assertEqual(simple_method(), 5) + + # A method to decorate with observer.span + def simple_method_with_args(self, a, b=3): + return a + b + + def test_tracing_with_arguments(self): + with patch(OBSERVER_OPEN, mock_open(read_data="empty")): + span, _ = observer._init_tracing([TEST_OBSERVER_CONF], ".") + + simple_method_with_args = span(self.simple_method_with_args) + + self.assertEqual(simple_method_with_args(5), 8) diff --git a/scripts/Makefile b/scripts/Makefile index 8f07e91efe7..17c8e383fac 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -177,7 +177,6 @@ endif sed -i 's/#!\/usr\/bin\/python/#!\/usr\/bin\/python3/' $(DESTDIR)$(SITE3_DIR)/XenAPIPlugin.py $(IDATA) examples/python/XenAPI/XenAPI.py $(DESTDIR)$(SITE3_DIR)/ $(IDATA) examples/python/inventory.py $(DESTDIR)$(SITE3_DIR)/ - $(IDATA) observer.py $(DESTDIR)$(SITE3_DIR)/ $(IPROG) examples/python/echo.py $(DESTDIR)$(PLUGINDIR)/echo $(IPROG) examples/python/shell.py $(DESTDIR)$(LIBEXECDIR)/shell.py # poweron diff --git a/scripts/observer.py b/scripts/observer.py deleted file mode 100644 index 96013361866..00000000000 --- a/scripts/observer.py +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/python3 - -if __name__ == '__main__': - # run a program passed as parameter, with its original arguments - import runpy - import sys - - # shift all the argvs one left - sys.argv = sys.argv[1:] - argv0=sys.argv[0] - - def run(file): - runpy.run_path(file, run_name='__main__') - - run(argv0)