Skip to content

Conversation

@gthvn1
Copy link
Contributor

@gthvn1 gthvn1 commented Oct 8, 2025

No description provided.

@gthvn1 gthvn1 force-pushed the gtn-fix-perfmon-typo branch from 1bb4376 to f9e5617 Compare October 8, 2025 15:49
@psafont
Copy link
Member

psafont commented Oct 9, 2025

@gthvn1 Tests are failing because perfmon doesn't have any tests. This means that currently perfmon can't be modified without spending the effort to add them

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 9, 2025

Oh. I understood that python3/tests/test_extauth_hook_AD.py and nbd_client_manager:nbd_client_manager.py failed. So it was not related to my modification. And I see the coverage issue but I thought it was only a warning.
Ok so I guess that need to have a closer look to understand how to add a test ;)

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 9, 2025

In fact I see in the logs:

...
python3/tests/test_perfmon.py::TestGetFsUsage::test_get_percent_fs_usage PASSED [ 40%]
python3/tests/test_perfmon.py::TestGetFsUsage::test_get_percent_log_fs_usage PASSED [ 41%]
python3/tests/test_perfmon.py::TestGetFsUsage::test_get_percent_log_fs_usage_same_file_system PASSED [ 42%]
python3/tests/test_perfmon.py::TestGetMemUsage::test_get_percent_mem_usage PASSED [ 43%]
python3/tests/test_perfmon.py::TestGetMemUsage::test_get_percent_mem_usage_exception PASSED [ 43%]
python3/tests/test_perfmon.py::TestGetPercentSRUsage::test_get_percent_sr_usage_correct_input PASSED [ 44%]
python3/tests/test_perfmon.py::TestGetPercentSRUsage::test_get_percent_sr_usage_exception_handling PASSED [ 45%]
...

So it looks like perfmon has some tests.
But when I run tests locally (I just run pytest tests) I have an issue with tests/observer/it_traces.py.

>           assert span.__name__ == "span_of_tracers"
E           AssertionError: assert '_span_noop' == 'span_of_tracers'
E
E             - span_of_tracers
E             + _span_noop

tests/observer/it_traces.py:73: AssertionError
---------------------------------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------------------------------
Hello, I am a print() in traced_script.py.
----------------------------------------------------------------------------------------------- Captured log call ------------------------------------------------------------------------------------------------
DEBUG    __main__:observer.py:400 configs = ['/home/gthouvenin/git/xen-api/python3/tests/observer/observer.conf']
ERROR    __main__:observer.py:147 missing opentelemetry dependencies: No module named 'opentelemetry'
============================================================================================ short test summary info =============================================================================================
FAILED tests/observer/it_traces.py::it_creates_a_tracer - AssertionError: assert '_span_noop' == 'span_of_tracers'
========================================================================================= 1 failed, 109 passed in 0.32s ==========================================================================================

@lindig
Copy link
Contributor

lindig commented Oct 14, 2025

@snwoods is there anything related to tracing that could be outdated above?

@snwoods
Copy link
Contributor

snwoods commented Oct 14, 2025

The issue with the tests/observer/it_traces.py test is that you don't have the opentelemetry python module installed. The output shows missing opentelemetry dependencies which happens when there is an import error. It then falls back to the _span_noop function decorator, which is to say tracing is disabled. But the test expects span_of_tracers decorator function

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 15, 2025

after installing opentelemetry-api and opentelemetry-sdk I still have some issues:

>       assert msg[4].startswith("tracers=[<opentelemetry.sdk.trace.Tracer object at")
E       AssertionError: assert False
E        +  where False = <built-in method startswith of str object at 0x7f4128b5bad0>('tracers=[<opentelemetry.sdk.trace.Tracer object at')
E        +    where <built-in method startswith of str object at 0x7f4128b5bad0> = 'Overriding of current TracerProvider is not allowed'.startswith

tests/observer/it_traces.py:104: AssertionError
----------------------------------------------------------------------------------------------- Captured log call ------------------------------------------------------------------------------------------------
DEBUG    __main__:observer.py:400 configs = ['/home/gthouvenin/git/xen-api/python3/tests/observer/observer.conf']
DEBUG    __main__:observer.py:95 /home/gthouvenin/git/xen-api/python3/tests/observer/all.conf: {'module_names': 'XenAPI,tests.observer.traced_script'}
DEBUG    __main__:observer.py:155 module_names: ['XenAPI', 'tests.observer.traced_script']
DEBUG    __main__:observer.py:95 /home/gthouvenin/git/xen-api/python3/tests/observer/observer.conf: {'otel_resource_attributes': '"key1=value1,key2=value2"'}
WARNING  opentelemetry.trace:__init__.py:537 Overriding of current TracerProvider is not allowed
DEBUG    __main__:observer.py:267 tracers=[<opentelemetry.sdk.trace.Tracer object at 0x7f4126131d90>]
============================================================================================ short test summary info =============================================================================================
FAILED tests/observer/it_traces.py::it_creates_a_tracer - AssertionError: assert False
========================================================================================= 1 failed, 109 passed in 0.40s ==========================================================================================

Actually, my point was that on my laptop, all tests related to perfmon pass. I can try, but I think that if I just keep the comment in my PR, the CI tests will fail as well.

@gthvn1 gthvn1 force-pushed the gtn-fix-perfmon-typo branch 2 times, most recently from 34d8557 to b15396f Compare October 31, 2025 10:08
@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 31, 2025

I just tested pushing only with comments, and CI passed. It is funny to see that fixing an actual error in the code is what causes the CI fail 😅 ... I will take a deeper look and try to figure out how to fix this.

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 31, 2025

I think the issue is that I added a line that isn't tested. So, if I understand correctly, it is the coverage of the newly added code that is checked. In my case, 100% of what I added is not tested. I will see how I can write a test for this line.

@gthvn1 gthvn1 force-pushed the gtn-fix-perfmon-typo branch from b15396f to 7aa4d84 Compare October 31, 2025 11:17
Signed-off-by: Guillaume <guillaume.thouvenin@vates.tech>
@gthvn1 gthvn1 force-pushed the gtn-fix-perfmon-typo branch from 7aa4d84 to 35b216f Compare October 31, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants