Add basic tracing functionality to tests - #525
Conversation
|
I like the idea a lot! Can we attach arbitrary metadata to the spans? |
Yes, in the form of baggage. As mentioned in the PR description, a good idea would be to allow passing the |
Signed-off-by: Colin James <contificate@gmail.com>
Signed-off-by: Colin James <contificate@gmail.com>
6147562 to
b8f425d
Compare
|
|
||
| tracing_endpoint = pytestconfig.getoption("--tracing-endpoint") | ||
| has_tracing = tracing_endpoint is not None | ||
| observers = [] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think you can have several observers per host with different endpoint for example.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
| 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) |
There was a problem hiding this comment.
I think these could be added as methods of the Host object.
| "--tracing-endpoint", | ||
| action="store", | ||
| default=None, | ||
| help="Specify distributed tracing endpoint." |
There was a problem hiding this comment.
Maybe add examples of endpoints here for users?
| 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'}) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Since there is interest in this PR and we would like to have tracing support for the live migration benchmarking project, we agreed with Colin to reorganize the code into a proper solution. I will open my own PR when I'm done, which will surely introduce an Observer class in /lib as mentioned in one of my comments.
5 participants
Opening as a draft to receive commentary.
This simple change introduces a
--tracing-endpointoption to the test framework that, if provided, will augment the host setup fixture to create an observer on each host. The effect of this is that toolstack operations will be traced, with their spans submitted to the provided endpoint (either Zipkin, Jaeger, or compatible span aggregator).A simple test is provided that uses this to perform an operation and then query the endpoint to fetch the root span of the operation (with some metadata attached).
The overall idea is that the benchmarking/performance teams can modify extant tests (or write new tests) to retrofit tracing (in a non-intrusive, opt-in, way).
I'm seeking some feedback about the direction of this change as there's more to consider:
Hostobject'sxemethod could be modified to allow for environmental variables. Then, as done in the provided sample test (albeit manually), we could supplyBAGGAGEto attach metadata to the spans of specific toolstack operations (relying on the metadata to fetch and process the related spans later).Any feedback will be much appreciated. This change is very simple and could do with being made a lot more featureful and robust, but I'm not overly familiar with the testing framework or its tests, so I've kept it small and isolated for now.