Skip to content

Add basic tracing functionality to tests #525

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

Draft
contificate wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Add basic tracing functionality to tests #525

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29e96df
Add tracing setup and teardown
contificate May 11, 2026
b8f425d
Add tracing test
contificate May 12, 2026
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
31 changes: 31 additions & 0 deletions conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ def pytest_addoption(parser: pytest.Parser) -> None:
default="2GiB",
help="Maximum amount of data written to a volume"
)
parser.addoption(
"--tracing-endpoint",
action="store",
default=None,
help="Specify distributed tracing endpoint."

@LuKP17 LuKP17 Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe add examples of endpoints here for users?

)

def pytest_configure(config: pytest.Config) -> None:
global_config.ignore_ssh_banner = config.getoption('--ignore-ssh-banner')
Expand Down Expand Up @@ -192,6 +198,19 @@ def pytest_runtest_makereport(

# END make test results visible from fixtures

def setup_tracing(host: Host, endpoint: str):
logging.info(f'Enabling tracing on {host}, endpoint = {endpoint}')
host.ssh('printf "observer-endpoint-http-enabled=true\nobserver-experimental-components=\\"\\"\n" > /etc/xapi.conf.d/observer.conf')
host.restart_toolstack(verify=True)
observer_uuid = host.xe('observer-create', {'name-label': 'xcpng-test', 'endpoints': endpoint, 'enabled': 'true'})

@LuKP17 LuKP17 Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we don't provide the host's uuid in host-uuids param, we will end up with multiple pool-wide observers if the function is called multiple times, which is not what was intended here I think.

host.restart_toolstack(verify=True)
return observer_uuid

def teardown_tracing(host, observer):
logging.info(f'Disabling tracing on {host}')
host.xe('observer-destroy', {'uuid': observer})
host.ssh('rm -f /etc/xapi.conf.d/observer.conf')
host.restart_toolstack(verify=True)

@stormi stormi Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think these could be added as methods of the Host object.


# fixtures

Expand Down Expand Up @@ -259,8 +278,20 @@ def cleanup_hosts() -> None:

if not host_list:
pytest.fail("This test requires at least one --hosts parameter")

tracing_endpoint = pytestconfig.getoption("--tracing-endpoint")
has_tracing = tracing_endpoint is not None
observers = []

@stormi stormi Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's just one observer per host, right? So rather than maintaining a list of observers, I think each host should have its optional observer as an attribute, and maybe the setup and teardown parts should be handled directly in setup_host and cleanup_hosts.

@gthvn1 gthvn1 Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can have several observers per host with different endpoint for example.

@stormi stormi Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can, but that's not what's in the current implementation, and I suppose that if/when we need more than one observer per host we can have a list or dict of observers attached to the host.

@LuKP17 LuKP17 Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

From my understanding of the relationship between observers and hosts, it's the other way around: observers contain an optional list of hosts uuids. If this list is empty then the observer is pool-wide, we provide hosts uuids if we need to filter which hosts in the pool are affected by this observer (most importantly it dictates which ones are not).
So for migration tests with a pool of 2 hosts, there's no need to specify hosts when creating an observer, we will get the source and dest hosts traces by default (pool-wide observer). But for some tests we would maybe like only one host to be affected by the observer, then add this host's uuid in the observer's host list.
If an observer is a standalone XAPI object, maybe we can rethink how to reflect this in xcp-ng-tests (like introducing an Observer class).

if has_tracing:
for h in host_list:
observers.append(setup_tracing(h, tracing_endpoint))

yield host_list

if has_tracing:
for h, o in zip(host_list, observers):
teardown_tracing(h, o)

cleanup_hosts()

@pytest.fixture(scope='session')
Expand Down