From 5ead1d1afa04f38c887a7edfeb19cc2bd90aaeb8 Mon Sep 17 00:00:00 2001 From: Steven Woods Date: Thu, 1 Feb 2024 15:54:48 +0000 Subject: [PATCH 1/6] Store trace_log_dir in XS_EXPORTER_BUGTOOL_ENDPOINT of the observer.conf This can then be used by observer.py, whereas the value 'bugtool' was useless Signed-off-by: Steven Woods --- ocaml/libs/tracing/tracing.ml | 2 ++ ocaml/libs/tracing/tracing.mli | 2 ++ ocaml/xapi/xapi_observer.ml | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ocaml/libs/tracing/tracing.ml b/ocaml/libs/tracing/tracing.ml index e128c510522..05700596116 100644 --- a/ocaml/libs/tracing/tracing.ml +++ b/ocaml/libs/tracing/tracing.ml @@ -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/xapi/xapi_observer.ml b/ocaml/xapi/xapi_observer.ml index 3c5992290cd..5f73beeb073 100644 --- a/ocaml/xapi/xapi_observer.ml +++ b/ocaml/xapi/xapi_observer.ml @@ -235,7 +235,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 | [] -> From 9e0d3da8ae059020e283c5bc02c05e9abec855d5 Mon Sep 17 00:00:00 2001 From: Steven Woods Date: Thu, 8 Feb 2024 14:22:35 +0000 Subject: [PATCH 2/6] Set traceparent trace flag to 01 This enables the sampled flag (https://www.w3.org/TR/trace-context/#sampled-flag). This indicates that we may have recorded trace data. In the future if we use sampling, this flag can be set with more granularity. Signed-off-by: Steven Woods --- ocaml/libs/tracing/tracing.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/libs/tracing/tracing.ml b/ocaml/libs/tracing/tracing.ml index 05700596116..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 From 10d38c6ca23f67465a8a9ff37aea126fb2fdb1f8 Mon Sep 17 00:00:00 2001 From: Steven Woods Date: Mon, 5 Feb 2024 11:41:18 +0000 Subject: [PATCH 3/6] Add service.name attribute as a default observer attribute. This is necessary for the python observers to set their local endpoint value. It also allows filtering traces by component Signed-off-by: Steven Woods --- ocaml/tests/test_observer.ml | 9 ++++++++- ocaml/xapi/xapi_observer.ml | 8 ++++---- 2 files changed, 12 insertions(+), 5 deletions(-) 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 5f73beeb073..355cb8dfd0c 100644 --- a/ocaml/xapi/xapi_observer.ml +++ b/ocaml/xapi/xapi_observer.ml @@ -429,7 +429,7 @@ let assert_valid_attributes attributes = ) attributes -let default_attributes ~__context ~host ~name_label = +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 @@ -439,12 +439,13 @@ let default_attributes ~__context ~host ~name_label = ; ("xs.host.name", host_label) ; ("xs.host.uuid", host_uuid) ; ("xs.observer.name", name_label) + ; ("service.name", to_string component) ] 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 +604,12 @@ 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 From 7a0d3e40706d1ff568b539fa3b62459e427fd644 Mon Sep 17 00:00:00 2001 From: Steven Woods Date: Thu, 15 Feb 2024 17:46:49 +0000 Subject: [PATCH 4/6] Add the default_attributes to Dom0ObserverConfig Observers The default_attributes are not added to the DB and so these are missing in the observer.conf. Dom0ObserverConfig currently lacks a single source of truth outside of the DB (and default_attributes is not given to the DB) and so attributes outside of the DB are lost whenever a different setter function is used. This should be reworked in future so that observer.conf is read from/updated instead of regenerated each time. Signed-off-by: Steven Woods --- ocaml/xapi/xapi_observer.ml | 44 ++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/ocaml/xapi/xapi_observer.ml b/ocaml/xapi/xapi_observer.ml index 355cb8dfd0c..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 @@ -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,19 +447,6 @@ let assert_valid_attributes attributes = ) attributes -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) - ] - let register_component ~__context ~self ~host ~component = let name_label = Db.Observer.get_name_label ~__context ~self in let attributes = @@ -609,7 +614,10 @@ let set_attributes ~__context ~self ~value = (fun c -> let module Forwarder = (val get_forwarder c : ObserverInterface) in Forwarder.set_attributes ~__context ~uuid - ~attributes:(default_attributes ~__context ~host ~name_label ~component:c @ value) + ~attributes: + (default_attributes ~__context ~host ~name_label ~component:c + @ value + ) ) (Db.Observer.get_components ~__context ~self |> List.map of_string From 0a0d55db0bec49c601de27eeed7bc5a52eb75af0 Mon Sep 17 00:00:00 2001 From: Steven Woods Date: Fri, 23 Feb 2024 10:29:35 +0000 Subject: [PATCH 5/6] Create a new python3 directory for python3-only scripts This is intended as the start of a new directory structure. Splitting the python3-only scripts into a new directory gives a simple way to exclude them from python2 tests and coverage. Signed-off-by: Steven Woods --- .codecov.yml | 41 +++++++++++++++++++++-- .github/workflows/main.yml | 15 ++++++++- Makefile | 1 + pyproject.toml | 1 + python3/Makefile | 9 +++++ python3/README.md | 12 +++++++ {scripts => python3/packages}/observer.py | 2 -- scripts/Makefile | 1 - 8 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 python3/Makefile create mode 100644 python3/README.md rename {scripts => python3/packages}/observer.py (93%) 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..7b660722b20 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -56,7 +56,8 @@ jobs: - 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/pyproject.toml b/pyproject.toml index b65a36bb062..d05d028fbad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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/scripts/observer.py b/python3/packages/observer.py similarity index 93% rename from scripts/observer.py rename to python3/packages/observer.py index 96013361866..a2538304e19 100644 --- a/scripts/observer.py +++ b/python3/packages/observer.py @@ -1,5 +1,3 @@ -#!/usr/bin/python3 - if __name__ == '__main__': # run a program passed as parameter, with its original arguments import runpy 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 From 8596e9852a5b51c02784d1d1a87bf2056a13e93d Mon Sep 17 00:00:00 2001 From: Steven Woods Date: Tue, 20 Feb 2024 15:59:31 +0000 Subject: [PATCH 6/6] CP-46151: Productise the observer.py. Instead of being a noop, it will now automatically instrument the python code given to it and create spans accordingly. The trace created will link to the xapi code that called it. It will export to all endpoints that it is given, which it reads from the observer.conf files created by xapi. Signed-off-by: Steven Woods Co-authored-by: Bernhard Kaindl Co-authored-by: Marcus Granado <590521+mg12@users.noreply.github.com> --- .github/workflows/main.yml | 2 +- pyproject.toml | 2 +- python3/packages/__init__.py | 0 python3/packages/observer.py | 425 ++++++++++++++++++++++++++++++++- python3/tests/__init__.py | 0 python3/tests/test_observer.py | 133 +++++++++++ 6 files changed, 550 insertions(+), 12 deletions(-) create mode 100644 python3/packages/__init__.py create mode 100644 python3/tests/__init__.py create mode 100644 python3/tests/test_observer.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7b660722b20..6662dc218ea 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -51,7 +51,7 @@ 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 diff --git a/pyproject.toml b/pyproject.toml index d05d028fbad..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 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 index a2538304e19..1eee0f2a7d6 100644 --- a/python3/packages/observer.py +++ b/python3/packages/observer.py @@ -1,13 +1,418 @@ -if __name__ == '__main__': - # run a program passed as parameter, with its original arguments - import runpy - import sys +# 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. +# - # shift all the argvs one left - sys.argv = sys.argv[1:] - argv0=sys.argv[0] +""" +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. - def run(file): - runpy.run_path(file, run_name='__main__') +If there are no *observer.conf files or something fails, this script runs the +passed script without any instrumentation. +""" - run(argv0) +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)