Skip to content

Commit

Permalink
opentelemetry-instrumentation-system-metrics: don't report open file …
Browse files Browse the repository at this point in the history
…descriptors on windows (#2946)

* opentelemetry-instrumentation-system-metrics: don't report files descriptors on windows

* Run manually added tox env tests on windows too

* Add jobs for botocore and system-metrics on windows

* Skip open file descriptor test on windows

---------

Co-authored-by: Tammy Baylis <[email protected]>
  • Loading branch information
xrmx and tammy-baylis-swi authored Nov 5, 2024
1 parent d6a59e4 commit 8cfbca2
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 5 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/generate_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
tox_ini_path = Path(__file__).parent.parent.parent.joinpath("tox.ini")
workflows_directory_path = Path(__file__).parent

generate_test_workflow(tox_ini_path, workflows_directory_path, "ubuntu-latest")
generate_test_workflow(
tox_ini_path, workflows_directory_path, "ubuntu-latest", "windows-latest"
)
generate_lint_workflow(tox_ini_path, workflows_directory_path)
generate_misc_workflow(tox_ini_path, workflows_directory_path)
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ def get_test_job_datas(tox_envs: list, operating_systems: list) -> list:
"py312": "3.12",
}

# we enable windows testing only for packages with windows specific code paths
per_tox_env_os_enablement = {
"windows-latest": {
"py312-test-instrumentation-botocore",
"py312-test-instrumentation-system-metrics",
},
}

test_job_datas = []

for operating_system in operating_systems:
Expand All @@ -71,6 +79,14 @@ def get_test_job_datas(tox_envs: list, operating_systems: list) -> list:
]
tox_env = tox_test_env_match.string

# if we have an entry for the os add only jobs for tox env manually configured
packages_manually_enabled = per_tox_env_os_enablement.get(
operating_system
)
if packages_manually_enabled:
if tox_env not in packages_manually_enabled:
continue

test_requirements = groups["test_requirements"]

if test_requirements is None:
Expand Down
42 changes: 42 additions & 0 deletions .github/workflows/test_1.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3903,3 +3903,45 @@ jobs:

- name: Run tests
run: tox -e pypy3-test-processor-baggage -- -ra

py312-test-instrumentation-botocore_windows-latest:
name: instrumentation-botocore 3.12 Windows
runs-on: windows-latest
steps:
- name: Checkout repo @ SHA - ${{ github.sha }}
uses: actions/checkout@v4

- name: Set up Python 3.12
uses: actions/setup-python@v5
with:
python-version: "3.12"

- name: Install tox
run: pip install tox

- name: Configure git to support long filenames
run: git config --system core.longpaths true

- name: Run tests
run: tox -e py312-test-instrumentation-botocore -- -ra

py312-test-instrumentation-system-metrics_windows-latest:
name: instrumentation-system-metrics 3.12 Windows
runs-on: windows-latest
steps:
- name: Checkout repo @ SHA - ${{ github.sha }}
uses: actions/checkout@v4

- name: Set up Python 3.12
uses: actions/setup-python@v5
with:
python-version: "3.12"

- name: Install tox
run: pip install tox

- name: Configure git to support long filenames
run: git config --system core.longpaths true

- name: Run tests
run: tox -e py312-test-instrumentation-system-metrics -- -ra
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2940](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2940))
- `opentelemetry-instrumentation-dbapi` sqlcommenter key values created from PostgreSQL, MySQL systems
([#2897](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2897))
- `opentelemetry-instrumentation-system-metrics`: don't report open file descriptors on Windows
([#2946](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2946))

### Breaking changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,10 @@ def _instrument(self, **kwargs):
unit="switches",
)

if "process.open_file_descriptor.count" in self._config:
if (
sys.platform != "win32"
and "process.open_file_descriptor.count" in self._config
):
self._meter.create_observable_up_down_counter(
name="process.open_file_descriptor.count",
callbacks=[self._get_open_file_descriptors],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

# pylint: disable=protected-access

import sys
from collections import namedtuple
from platform import python_implementation
from unittest import mock, skipIf
Expand Down Expand Up @@ -118,21 +119,30 @@ def test_system_metrics_instrument(self):
f"process.runtime.{self.implementation}.thread_count",
f"process.runtime.{self.implementation}.context_switches",
f"process.runtime.{self.implementation}.cpu.utilization",
"process.open_file_descriptor.count",
]

on_windows = sys.platform == "win32"
if self.implementation == "pypy":
self.assertEqual(len(metric_names), 21)
self.assertEqual(len(metric_names), 20 if on_windows else 21)
else:
self.assertEqual(len(metric_names), 22)
self.assertEqual(len(metric_names), 21 if on_windows else 22)
observer_names.append(
f"process.runtime.{self.implementation}.gc_count",
)
if not on_windows:
observer_names.append(
"process.open_file_descriptor.count",
)

for observer in metric_names:
self.assertIn(observer, observer_names)
observer_names.remove(observer)

if on_windows:
self.assertNotIn(
"process.open_file_descriptor.count", observer_names
)

def test_runtime_metrics_instrument(self):
runtime_config = {
"process.runtime.memory": ["rss", "vms"],
Expand Down Expand Up @@ -844,6 +854,7 @@ def test_runtime_cpu_percent(self, mock_process_cpu_percent):
f"process.runtime.{self.implementation}.cpu.utilization", expected
)

@skipIf(sys.platform == "win32", "No file descriptors on Windows")
@mock.patch("psutil.Process.num_fds")
def test_open_file_descriptor_count(self, mock_process_num_fds):
mock_process_num_fds.configure_mock(**{"return_value": 3})
Expand Down

0 comments on commit 8cfbca2

Please sign in to comment.