Add trace-per-test option #34
Conversation
|
@xmo-odoo I do like this idea, but would we really need a full new copy of the plugin? Could this be done conditionally? |
There's no copy of the plugin? I did implement that initially locally as a test, but then realised the trace-per-test version was really just a subset of the trace-per-run version, so Before this change, I'm not sure adding an |
|
I'll buy that! Just looks like we're missing one line of coverage for the |
Yep, I'll take a gander as how things are tested. |
xmo-odoo
force-pushed
the
trace-per-test
branch
2 times, most recently
from
494e1a9 to
3ab38a0
Compare
|
I added a test for the trace-per-test mode, however it doesn't look like pytest-cov sees the code run by Also not sure the test is very valuable. I wanted to check that the |
xmo-odoo
force-pushed
the
trace-per-test
branch
2 times, most recently
from
7c5fd04 to
0a06f05
Compare
Trace-per-run is not always great e.g. it's confusing to have a sequence of tests which are normally independent, and then the layout changes when xdist is involved as you get concurrent spans (one for each worker). This also causes issues in the more bare-bone / simplistic frontends / final collectors (e.g. OpenTelemetry Desktop) as they might not have the most complex folding and filtering features. OTD for instance has little to no ability to manipulate spans, but it is possible to select individual traces and see just that trace's spans (in fact that's the only mode). As a result, generating a separate trace per test per run allows easier classification, observation, and manipulation of the trace. I would also assume in the long term it allows comparing the traces of the same test in order to get insight into their evolution, something which is more difficult with trace-per-run. Finally an other issue with trace-per-run, mostly in long suites, is that the trace remains incomplete (tools complain of missing parent spans) until the entire run has completed, making observation during run more difficult.
xmo-odoo
force-pushed
the
trace-per-test
branch
from
0a06f05 to
f9ae1ee
Compare
|
Finally got around to looking into the coverage issue (sorry) I can confirm that coverage does not see the So in order to get coverage, I parametrized one of the span tests to run in both trace-per-suite and trace-per-test modes. I think technically all the tests could be parametrized thus (or near enough) but every other tests specifically checks for the |
Mostly for tests: it can be really difficult to correlate issues as there are 3 different processes involved (the test runner, the odoo being tested, and dummy-central (as github)) and the intermixing of logs between the test driver and the odoo being tested is not *amazing*. - `pytest-opentelemetry`'s `--export-traces` is the signal for running tests with tracing enabled, that way just having `pytest-opentelemetry` installed does not do anything untowards. - Until chrisguidry/pytest-opentelemetry#34 is merged, should probably use the linked branch as the default / base mode of having a single trace for the entire test suite is not great when there are 460 tests, especially as local clients (opentelemetry-desktop-viewer, venator) mostly work on traces and aren't very good at zooming on *spans* at least currently. - Additionally, the conftest plugin adds hooks for propagating through the xmlrpc client (communications with odoo) and enables the requests instrumentor from the opentelemetry python contribs. - The dummy `saas_worker` was moved into the filesystem, that makes it easier to review and update. - A second such module was added for the otel instrumentation *of the odoo under test*, that instruments psycopg2, requests, wsgi, and the server side of xmlrpc. - The git ops were instrumented directly in runbot_merge, as I've tried to keep `saas_tracing` relatively generic, in case it could be moved to community or used by internal eventually. Some typing was added to the conftest hooks and fixtures, and they were migrated from tmpdir (py.path) to tmp_path (pathlib) for consistency, even though technically the `mkdir` of pathlib is an annoying downgrade. Fixes #835
Trace-per-run is not always great e.g. it's confusing to have a sequence of tests which are normally independent, and then the layout changes when xdist is involved as you get concurrent spans (one for each worker).
This also causes issues in the more bare-bone / simplistic frontends / final collectors (e.g. OpenTelemetry Desktop) as they might not have the most complex folding and filtering features. OTD for instance has little to no ability to manipulate spans, but it is possible to select individual traces and see just that trace's spans (in fact that's the only mode).
As a result, generating a separate trace per test per run allows easier classification, observation, and manipulation of the trace. I would also assume in the long term it allows comparing the traces of the same test in order to get insight into their evolution, something which is more difficult with trace-per-run.
Finally an other issue with trace-per-run, mostly in long suites, is that the trace remains incomplete (tools complain of missing parent spans) until the entire run has completed, making observation during run more difficult.
Fixes #33