From 6d1031530b6922ff0be9cb71da979f3bd7258afb Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:10:13 -0500 Subject: [PATCH 01/18] adding mypy to parametric --- docs/edit/format.md | 7 ++-- format.sh | 15 ++++++- pyproject.toml | 7 ++++ requirements.txt | 1 + tests/parametric/conftest.py | 11 ++---- tests/parametric/test_library_tracestats.py | 7 ++-- tests/parametric/test_tracer.py | 10 +++-- tests/parametric/test_tracer_flare.py | 2 +- utils/parametric/_library_client.py | 44 +++++++++++++-------- utils/parametric/spec/trace.py | 2 +- 10 files changed, 66 insertions(+), 40 deletions(-) diff --git a/docs/edit/format.md b/docs/edit/format.md index 9b03ce8261..985668fc10 100644 --- a/docs/edit/format.md +++ b/docs/edit/format.md @@ -1,10 +1,9 @@ -System tests code is in python, and is linted/formated using [black](https://black.readthedocs.io/en/stable/) and [pylint](https://pylint.readthedocs.io/en/latest/). +System tests code is in python, and is linted/formated using [mypy](https://mypy.readthedocs.io/en/stable/), [black](https://black.readthedocs.io/en/stable/) and [pylint](https://pylint.readthedocs.io/en/latest/). -Ensure you meet the other pre-reqs in [README.md](../../README.md#requirements) -Then, run the linter with: +You'll only need [python3.12](https://www.python.org/downloads/) (you may have it by default) to run them, with a unique command : ```bash ./format.sh ``` -Any library app code (that is, code written in Java, Golang, .NET, etc in weblog or parametric apps) will need to be formatted using that library's formatter. +There is a good chance that all your files are correctly formated :tada:. For some use-cases where issues can't be automatically fixed, you'll jave the explanation in the output. And you can run only checks, without modifying any file using the `-c` option. diff --git a/format.sh b/format.sh index 649cc5d4a1..4341afd120 100755 --- a/format.sh +++ b/format.sh @@ -38,11 +38,22 @@ source venv/bin/activate echo "Checking Python files..." if [ "$COMMAND" == "fix" ]; then - black . + black --quiet . else black --check --diff . fi -pylint utils # pylint does not have a fix mode + +echo "Running mypy type checks..." +if ! mypy --config pyproject.toml --install-types --non-interactive; then + echo "Mypy type checks failed. Please fix the errors above. 💥 💔 💥" + exit 1 +fi + +echo "Running pylint checks..." +if ! pylint utils; then + echo "Pylint checks failed. Please fix the errors above. 💥 💔 💥" + exit 1 +fi echo "Checking trailing whitespaces..." INCLUDE_PATTERN='.*\.(md|yml|yaml|sh|cs|Dockerfile|java|sql|ts|js|php)$' diff --git a/pyproject.toml b/pyproject.toml index dbbb5f8cd4..1db41d3635 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,6 +102,13 @@ allow_no_jira_ticket_for_bugs = [ "tests/parametric/test_config_consistency.py::Test_Config_TraceLogDirectory", ] +[tool.mypy] +files = ["utils/parametric", "tests/parametric"] +ignore_missing_imports = true +disable_error_code = ["no-redef"] +exclude = '^(?!utils/parametric|tests/parametric).*$' +follow_imports = "skip" + [tool.pylint] init-hook='import sys; sys.path.append(".")' max-line-length = 120 diff --git a/requirements.txt b/requirements.txt index 39f982111c..f3e61080f9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ pylint==3.0.4 python-dateutil==2.8.2 msgpack==1.0.4 watchdog==3.0.0 +mypy==1.13.0 aiohttp==3.9.0 yarl==1.9.4 diff --git a/tests/parametric/conftest.py b/tests/parametric/conftest.py index ff2aef83d4..4e99280f00 100644 --- a/tests/parametric/conftest.py +++ b/tests/parametric/conftest.py @@ -46,10 +46,7 @@ class AgentRequest(TypedDict): class AgentRequestV06Stats(AgentRequest): - method: str - url: str - headers: Dict[str, str] - body: V06StatsPayload + body: V06StatsPayload # type: ignore def pytest_configure(config): @@ -107,7 +104,7 @@ def __init__(self, base_url: str, pytest_request: None): self._base_url = base_url self._session = requests.Session() self._pytest_request = pytest_request - self.log_path = f"{context.scenario.host_log_folder}/outputs/{pytest_request.cls.__name__}/{pytest_request.node.name}/agent_api.log" + self.log_path = f"{context.scenario.host_log_folder}/outputs/{pytest_request.cls.__name__}/{pytest_request.node.name}/agent_api.log" # type: ignore os.makedirs(os.path.dirname(self.log_path), exist_ok=True) def _url(self, path: str) -> str: @@ -460,7 +457,7 @@ def wait_for_rc_capabilities(self, capabilities: List[int] = [], wait_loops: int time.sleep(0.01) raise AssertionError("No RemoteConfig capabilities found, got capabilites %r" % capabilities_seen) - def wait_for_tracer_flare(self, case_id: str = None, clear: bool = False, wait_loops: int = 100): + def wait_for_tracer_flare(self, case_id: Optional[str] = None, clear: bool = False, wait_loops: int = 100): """Wait for the tracer-flare to be received by the test agent.""" for i in range(wait_loops): try: @@ -479,7 +476,7 @@ def wait_for_tracer_flare(self, case_id: str = None, clear: bool = False, wait_l @pytest.fixture(scope="session") -def docker() -> str: +def docker() -> Optional[str]: """Fixture to ensure docker is ready to use on the system.""" # Redirect output to /dev/null since we just care if we get a successful response code. r = subprocess.run( diff --git a/tests/parametric/test_library_tracestats.py b/tests/parametric/test_library_tracestats.py index 313793f628..06a53cae76 100644 --- a/tests/parametric/test_library_tracestats.py +++ b/tests/parametric/test_library_tracestats.py @@ -19,10 +19,9 @@ def _human_stats(stats: V06StatsAggr) -> str: """Return human-readable stats for debugging stat aggregations.""" - copy = stats.copy() - del copy["ErrorSummary"] - del copy["OkSummary"] - return str(copy) + # Create a copy excluding 'ErrorSummary' and 'OkSummary' since TypedDicts don't allow delete + filtered_copy = {k: v for k, v in stats.items() if k not in {"ErrorSummary", "OkSummary"}} + return str(filtered_copy) def enable_tracestats(sample_rate: Optional[float] = None) -> Any: diff --git a/tests/parametric/test_tracer.py b/tests/parametric/test_tracer.py index 805cf92750..06b8fb2fc6 100644 --- a/tests/parametric/test_tracer.py +++ b/tests/parametric/test_tracer.py @@ -26,7 +26,7 @@ def test_tracer_span_top_level_attributes(self, test_agent: _TestAgentAPI, test_ "operation", service="my-webserver", resource="/endpoint", typestr="web" ) as parent: parent.set_metric("number", 10) - with test_library.start_span("operation.child", parent_id=parent.span_id) as child: + with test_library.start_span("operation.child", parent_id=str(parent.span_id)) as child: child.set_meta("key", "val") traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -60,7 +60,7 @@ def test_tracer_repository_url_environment_variable( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=parent.span_id): + with test_library.start_span("operation.child", parent_id=str(parent.span_id)): pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -86,7 +86,7 @@ def test_tracer_commit_sha_environment_variable( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=parent.span_id): + with test_library.start_span("operation.child", parent_id=str(parent.span_id)): pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -146,7 +146,7 @@ def test_tracer_repository_url_strip_credentials( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=parent.span_id): + with test_library.start_span("operation.child", parent_id=str(parent.span_id)): pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -175,6 +175,7 @@ def test_tracer_service_name_environment_variable( traces = test_agent.wait_for_num_traces(1, sort_by_start=False) trace = find_trace(traces, root.trace_id) span = find_root_span(trace) + assert span is not None, "Root span not found" assert span["name"] == "operation" assert span["service"] == library_env["DD_SERVICE"] @@ -195,5 +196,6 @@ def test_tracer_env_environment_variable( trace = find_trace(traces, root.trace_id) span = find_root_span(trace) + assert span is not None, "Root span not found" assert span["name"] == "operation" assert span["meta"]["env"] == library_env["DD_ENV"] diff --git a/tests/parametric/test_tracer_flare.py b/tests/parametric/test_tracer_flare.py index 23f9a794d7..d697bb18be 100644 --- a/tests/parametric/test_tracer_flare.py +++ b/tests/parametric/test_tracer_flare.py @@ -69,7 +69,7 @@ def _java_tracer_flare_filenames() -> Set: } -def _set_log_level(test_agent, log_level: str) -> int: +def _set_log_level(test_agent, log_level: str) -> str: """Helper to create the appropriate "flare-log-level" config in RC for a given log-level.""" cfg_id = uuid4().hex test_agent.set_remote_config( diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 5fbdff47fe..0516e32cb7 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -2,7 +2,7 @@ import contextlib import time import urllib.parse -from typing import Generator, List, Optional, Tuple, TypedDict, Union, Dict +from typing import Any, Generator, List, Optional, Tuple, TypedDict, Union, Dict from docker.models.containers import Container import pytest @@ -120,7 +120,7 @@ def trace_start_span( # TODO: Update the cpp parametric app to accept null values for unset parameters service = service or "" resource = resource or "" - parent_id = parent_id or 0 + parent_id = parent_id or "" typestr = typestr or "" resp = self._session.post( @@ -189,7 +189,11 @@ def span_set_error(self, span_id: int, typestr: str, message: str, stack: str) - ) def span_add_link( - self, span_id: int, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None + self, + span_id: int, + parent_id: int, + attributes: Optional[dict[Any, Any]] = None, + http_headers: Optional[List[Tuple[str, str]]] = None, ): # Avoid using http_headers when creating a span link in the parametric apps # Alternative endpoints will be provided to set these values. This will be documented in a future PR. @@ -205,13 +209,13 @@ def span_add_link( def span_get_baggage(self, span_id: int, key: str) -> str: resp = self._session.get(self._url("/trace/span/get_baggage"), json={"span_id": span_id, "key": key,},) - resp = resp.json() - return resp["baggage"] + data = resp.json() + return data["baggage"] def span_get_all_baggage(self, span_id: int) -> dict: resp = self._session.get(self._url("/trace/span/get_all_baggage"), json={"span_id": span_id}) - resp = resp.json() - return resp["baggage"] + data = resp.json() + return data["baggage"] def trace_inject_headers(self, span_id): resp = self._session.post(self._url("/trace/span/inject_headers"), json={"span_id": span_id},) @@ -237,7 +241,7 @@ def otel_trace_start_span( parent_id: int, links: List[Link], http_headers: List[Tuple[str, str]], - attributes: dict = None, + attributes: Optional[dict[Any, Any]] = None, ) -> StartSpanResponse: resp = self._session.post( self._url("/trace/otel/start_span"), @@ -304,8 +308,8 @@ def otel_set_baggage(self, span_id: int, key: str, value: str) -> None: resp = self._session.post( self._url("/trace/otel/otel_set_baggage"), json={"span_id": span_id, "key": key, "value": value} ) - resp = resp.json() - return resp["value"] + data = resp.json() + return data["value"] def get_tracer_config(self) -> Dict[str, Optional[str]]: resp = self._session.get(self._url("/trace/config")).json() @@ -369,7 +373,12 @@ def remove_all_baggage(self): def set_error(self, typestr: str = "", message: str = "", stack: str = ""): self._client.span_set_error(self.span_id, typestr, message, stack) - def add_link(self, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None): + def add_link( + self, + parent_id: int, + attributes: Optional[dict[Any, Any]] = None, + http_headers: Optional[List[Tuple[str, str]]] = None, + ): self._client.span_add_link(self.span_id, parent_id, attributes, http_headers) def finish(self): @@ -397,7 +406,7 @@ def set_status(self, code: StatusCode, description): self._client.otel_set_status(self.span_id, code, description) def add_event(self, name: str, timestamp: Optional[int] = None, attributes: Optional[dict] = None): - self._client.otel_add_event(self.span_id, name, timestamp, attributes) + self._client.otel_add_event(self.span_id, name, timestamp, attributes) # type: ignore def record_exception(self, message: str, attributes: Optional[dict] = None): self._client.otel_record_exception(self.span_id, message, attributes) @@ -463,7 +472,7 @@ def otel_start_span( span_kind: SpanKind = SpanKind.UNSPECIFIED, parent_id: int = 0, links: Optional[List[Link]] = None, - attributes: dict = None, + attributes: Optional[dict[Any, Any]] = None, http_headers: Optional[List[Tuple[str, str]]] = None, ) -> Generator[_TestOtelSpan, None, None]: resp = self._client.otel_trace_start_span( @@ -478,10 +487,11 @@ def otel_start_span( span = _TestOtelSpan(self._client, resp["span_id"], resp["trace_id"]) yield span - return { - "span_id": resp["span_id"], - "trace_id": resp["trace_id"], - } + # this method does not return anything + # return { + # "span_id": resp["span_id"], + # "trace_id": resp["trace_id"], + # } def flush(self) -> bool: return self._client.trace_flush() diff --git a/utils/parametric/spec/trace.py b/utils/parametric/spec/trace.py index fbcf9dfa7f..4d5bb498ea 100644 --- a/utils/parametric/spec/trace.py +++ b/utils/parametric/spec/trace.py @@ -257,4 +257,4 @@ def id_to_int(value: Union[str, int]) -> int: # as stringified integers (ids will be stringified to workaround percision issues in some languages) return int(value) except ValueError: - return int(value, 16) + return int(value, 16) # type: ignore From 66389ff695c47e0b035162efe947d0eb943d1fbc Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:17:42 -0500 Subject: [PATCH 02/18] mypy for parametric --- utils/parametric/_library_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 0516e32cb7..f11fab421d 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -14,7 +14,7 @@ from utils.parametric.spec.otel_trace import OtelSpanContext from utils.tools import logger - +# testing a push def _fail(message): """ Used to mak a test as failed """ logger.error(message) From 3c2a6c23f90f4d081b348aecad965dabed448464 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:18:49 -0500 Subject: [PATCH 03/18] i think it works --- utils/parametric/_library_client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index f11fab421d..e3ea6771f8 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -14,7 +14,6 @@ from utils.parametric.spec.otel_trace import OtelSpanContext from utils.tools import logger -# testing a push def _fail(message): """ Used to mak a test as failed """ logger.error(message) From b73396e29444fc137d886c408566758aff9a0f62 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:20:47 -0500 Subject: [PATCH 04/18] Update format.md --- docs/edit/format.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/edit/format.md b/docs/edit/format.md index 985668fc10..32a2995b92 100644 --- a/docs/edit/format.md +++ b/docs/edit/format.md @@ -1,9 +1,10 @@ System tests code is in python, and is linted/formated using [mypy](https://mypy.readthedocs.io/en/stable/), [black](https://black.readthedocs.io/en/stable/) and [pylint](https://pylint.readthedocs.io/en/latest/). -You'll only need [python3.12](https://www.python.org/downloads/) (you may have it by default) to run them, with a unique command : +Ensure you meet the other pre-reqs in [README.md](../../README.md#requirements) +Then, run the linter with: ```bash ./format.sh ``` -There is a good chance that all your files are correctly formated :tada:. For some use-cases where issues can't be automatically fixed, you'll jave the explanation in the output. And you can run only checks, without modifying any file using the `-c` option. +Any library app code (that is, code written in Java, Golang, .NET, etc in weblog or parametric apps) will need to be formatted using that library's formatter. From 5059c0fdaf0fad2bf592f99b96ac3061cf303762 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:23:43 -0500 Subject: [PATCH 05/18] linting errors --- utils/parametric/_library_client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index e3ea6771f8..0516e32cb7 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -14,6 +14,7 @@ from utils.parametric.spec.otel_trace import OtelSpanContext from utils.tools import logger + def _fail(message): """ Used to mak a test as failed """ logger.error(message) From e698eea3e70b6489892ea28a146a735862d0feb2 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:37:30 -0500 Subject: [PATCH 06/18] errors --- tests/parametric/test_telemetry.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/parametric/test_telemetry.py b/tests/parametric/test_telemetry.py index 531b6534c0..991c4a9145 100644 --- a/tests/parametric/test_telemetry.py +++ b/tests/parametric/test_telemetry.py @@ -129,7 +129,7 @@ class Test_Consistent_Configs: "DD_TRACE_RATE_LIMIT": 10, "DD_TRACE_HEADER_TAGS": "User-Agent:my-user-agent,Content-Type.", "DD_TRACE_ENABLED": "true", - "DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP": "\d{3}-\d{2}-\d{4}", + "DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP": r"\d{3}-\d{2}-\d{4}", "DD_TRACE_LOG_DIRECTORY": "/some/temporary/directory", "DD_TRACE_CLIENT_IP_HEADER": "random-header-name", "DD_TRACE_HTTP_CLIENT_ERROR_STATUSES": "200-250", @@ -157,7 +157,9 @@ def test_library_settings(self, library_env, test_agent, test_library): configuration_by_name.get("DD_TRACE_HEADER_TAGS").get("value") == "User-Agent:my-user-agent,Content-Type." ) assert configuration_by_name.get("DD_TRACE_ENABLED").get("value") == True - assert configuration_by_name.get("DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP").get("value") == "\d{3}-\d{2}-\d{4}" + assert ( + configuration_by_name.get("DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP").get("value") == r"\d{3}-\d{2}-\d{4}" + ) assert configuration_by_name.get("DD_TRACE_CLIENT_IP_HEADER").get("value") == "random-header-name" @pytest.mark.parametrize( From f97299e8bf6ea79b6189c519c3cc84cd53fe98da Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 18 Nov 2024 15:44:45 -0500 Subject: [PATCH 07/18] testing cache idea --- format.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/format.sh b/format.sh index 4341afd120..dc2d7bed40 100755 --- a/format.sh +++ b/format.sh @@ -43,9 +43,13 @@ else black --check --diff . fi -echo "Running mypy type checks..." -if ! mypy --config pyproject.toml --install-types --non-interactive; then +# Create a temporary cache directory +TEMP_CACHE_DIR=$(mktemp -d) + +if ! mypy --config pyproject.toml --install-types --non-interactive --cache-dir="$TEMP_CACHE_DIR"; then echo "Mypy type checks failed. Please fix the errors above. 💥 💔 💥" + # Clean up the temporary cache directory + rm -rf "$TEMP_CACHE_DIR" exit 1 fi From 7b69cc9bd856cced2002c378f8e2333047b16d2d Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 11:15:58 -0500 Subject: [PATCH 08/18] resolving comments --- tests/parametric/test_tracer.py | 8 ++++---- utils/parametric/_library_client.py | 10 ++-------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/parametric/test_tracer.py b/tests/parametric/test_tracer.py index 06b8fb2fc6..8d50383b88 100644 --- a/tests/parametric/test_tracer.py +++ b/tests/parametric/test_tracer.py @@ -26,7 +26,7 @@ def test_tracer_span_top_level_attributes(self, test_agent: _TestAgentAPI, test_ "operation", service="my-webserver", resource="/endpoint", typestr="web" ) as parent: parent.set_metric("number", 10) - with test_library.start_span("operation.child", parent_id=str(parent.span_id)) as child: + with test_library.start_span("operation.child", parent_id=parent.span_id) as child: child.set_meta("key", "val") traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -60,7 +60,7 @@ def test_tracer_repository_url_environment_variable( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=str(parent.span_id)): + with test_library.start_span("operation.child", parent_id=parent.span_id): pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -86,7 +86,7 @@ def test_tracer_commit_sha_environment_variable( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=str(parent.span_id)): + with test_library.start_span("operation.child", parent_id=parent.span_id): pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -146,7 +146,7 @@ def test_tracer_repository_url_strip_credentials( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=str(parent.span_id)): + with test_library.start_span("operation.child", parent_id=parent.span_id): pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 0516e32cb7..27f32525ca 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -449,12 +449,12 @@ def start_span( name: str, service: Optional[str] = None, resource: Optional[str] = None, - parent_id: Optional[str] = None, + parent_id: Optional[int] = None, typestr: Optional[str] = None, tags: Optional[List[Tuple[str, str]]] = None, ) -> Generator[_TestSpan, None, None]: resp = self._client.trace_start_span( - name=name, service=service, resource=resource, parent_id=parent_id, typestr=typestr, tags=tags, + name=name, service=service, resource=resource, parent_id=str(parent_id), typestr=typestr, tags=tags, ) span = _TestSpan(self._client, resp["span_id"], resp["trace_id"]) yield span @@ -487,12 +487,6 @@ def otel_start_span( span = _TestOtelSpan(self._client, resp["span_id"], resp["trace_id"]) yield span - # this method does not return anything - # return { - # "span_id": resp["span_id"], - # "trace_id": resp["trace_id"], - # } - def flush(self) -> bool: return self._client.trace_flush() From 5addc213ef4a01659e792a742d9eb0b0314f9cec Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 11:18:53 -0500 Subject: [PATCH 09/18] undo cache in format --- format.sh | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/format.sh b/format.sh index dc2d7bed40..4341afd120 100755 --- a/format.sh +++ b/format.sh @@ -43,13 +43,9 @@ else black --check --diff . fi -# Create a temporary cache directory -TEMP_CACHE_DIR=$(mktemp -d) - -if ! mypy --config pyproject.toml --install-types --non-interactive --cache-dir="$TEMP_CACHE_DIR"; then +echo "Running mypy type checks..." +if ! mypy --config pyproject.toml --install-types --non-interactive; then echo "Mypy type checks failed. Please fix the errors above. 💥 💔 💥" - # Clean up the temporary cache directory - rm -rf "$TEMP_CACHE_DIR" exit 1 fi From 0a98908d9b4a238931aa18d77e94b5886ced126e Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 16:37:45 -0500 Subject: [PATCH 10/18] Update requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index f3e61080f9..c494e411b7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ pylint==3.0.4 python-dateutil==2.8.2 msgpack==1.0.4 watchdog==3.0.0 -mypy==1.13.0 +mypy==1.0.0 aiohttp==3.9.0 yarl==1.9.4 From d44363dd4ba83b7867ed012f097c5702fdb88bb9 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 16:44:07 -0500 Subject: [PATCH 11/18] Update requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c494e411b7..261cea9b82 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ pylint==3.0.4 python-dateutil==2.8.2 msgpack==1.0.4 watchdog==3.0.0 -mypy==1.0.0 +mypy==1.11.0 aiohttp==3.9.0 yarl==1.9.4 From 763951268de97f9f5a64eac4db9bede9cd043999 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 16:46:01 -0500 Subject: [PATCH 12/18] Update requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 261cea9b82..2a443bcc73 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ pylint==3.0.4 python-dateutil==2.8.2 msgpack==1.0.4 watchdog==3.0.0 -mypy==1.11.0 +mypy==1.5.0 aiohttp==3.9.0 yarl==1.9.4 From bc826ce733e4e1873a4818ac575596cc75fe2fdd Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 16:52:02 -0500 Subject: [PATCH 13/18] mypy version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 2a443bcc73..c494e411b7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ pylint==3.0.4 python-dateutil==2.8.2 msgpack==1.0.4 watchdog==3.0.0 -mypy==1.5.0 +mypy==1.0.0 aiohttp==3.9.0 yarl==1.9.4 From 58d4c42b0ade64866b0c430c9c3e46fe40015985 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 17:02:00 -0500 Subject: [PATCH 14/18] trying new version of mypy --- format.sh | 2 +- utils/parametric/spec/remoteconfig.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/format.sh b/format.sh index 4341afd120..b0d756727a 100755 --- a/format.sh +++ b/format.sh @@ -44,7 +44,7 @@ else fi echo "Running mypy type checks..." -if ! mypy --config pyproject.toml --install-types --non-interactive; then +if ! mypy --config pyproject.toml; then echo "Mypy type checks failed. Please fix the errors above. 💥 💔 💥" exit 1 fi diff --git a/utils/parametric/spec/remoteconfig.py b/utils/parametric/spec/remoteconfig.py index b0e7dc7930..59b8a60bd6 100644 --- a/utils/parametric/spec/remoteconfig.py +++ b/utils/parametric/spec/remoteconfig.py @@ -1,4 +1,4 @@ -from typing import Literal +from typing import Any, Literal from typing import Tuple from utils.dd_constants import Capabilities @@ -13,5 +13,5 @@ APPLY_STATUS = Literal[0, 1, 2, 3] -def human_readable_capabilities(caps: int) -> Tuple[str]: +def human_readable_capabilities(caps: int) -> Tuple[Any, ...]: return tuple(c.name for c in Capabilities if caps >> c & 1) From d1a896e6fec94732a3c614e919d315b70d8d279e Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 19 Nov 2024 17:04:46 -0500 Subject: [PATCH 15/18] library stubs errors --- tests/parametric/conftest.py | 2 +- tests/parametric/test_headers_baggage.py | 2 +- utils/parametric/_library_client.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/parametric/conftest.py b/tests/parametric/conftest.py index 4e99280f00..0e8fbda0c0 100644 --- a/tests/parametric/conftest.py +++ b/tests/parametric/conftest.py @@ -11,7 +11,7 @@ from typing import Dict, Generator, List, TextIO, TypedDict, Optional, Any import urllib.parse -import requests +import requests # type: ignore import pytest from utils.parametric.spec import remoteconfig diff --git a/tests/parametric/test_headers_baggage.py b/tests/parametric/test_headers_baggage.py index ea3df19074..96cd57c9d4 100644 --- a/tests/parametric/test_headers_baggage.py +++ b/tests/parametric/test_headers_baggage.py @@ -1,6 +1,6 @@ from operator import le from py import test -from requests import head +from requests import head # type: ignore from utils.parametric.spec.trace import SAMPLING_PRIORITY_KEY, ORIGIN from utils.parametric.spec.trace import span_has_no_parent from utils.parametric.headers import make_single_request_and_get_inject_headers diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 27f32525ca..546b7d4dfd 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -7,7 +7,7 @@ from docker.models.containers import Container import pytest from _pytest.outcomes import Failed -import requests +import requests # type: ignore from utils import context from utils.dd_constants import SpanKind, StatusCode From 40807861fb578db2880bc07a6347d715e4500d88 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 20 Nov 2024 11:28:29 -0500 Subject: [PATCH 16/18] library client reformat --- utils/parametric/_library_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 17e056e598..e8a014fdad 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -356,7 +356,7 @@ def remove_all_baggage(self): def set_error(self, typestr: str = "", message: str = "", stack: str = ""): self._client.span_set_error(self.span_id, typestr, message, stack) - + def add_link(self, parent_id: int, attributes: Optional[dict[Any, Any]] = None): self._client.span_add_link(self.span_id, parent_id, attributes) From 68e8dc8f98fecbe34bbd98286580a227d80ed793 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 20 Nov 2024 12:05:06 -0500 Subject: [PATCH 17/18] library client errors --- pyproject.toml | 2 +- tests/parametric/test_tracer.py | 8 ++-- utils/parametric/_library_client.py | 57 +++++++++++++++++++---------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1db41d3635..9b1f5f28de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,7 +106,7 @@ allow_no_jira_ticket_for_bugs = [ files = ["utils/parametric", "tests/parametric"] ignore_missing_imports = true disable_error_code = ["no-redef"] -exclude = '^(?!utils/parametric|tests/parametric).*$' +exclude = 'utils/parametric/_library_client\.py|^(?!utils/parametric|tests/parametric).*$' follow_imports = "skip" [tool.pylint] diff --git a/tests/parametric/test_tracer.py b/tests/parametric/test_tracer.py index 8d50383b88..8405b50b5c 100644 --- a/tests/parametric/test_tracer.py +++ b/tests/parametric/test_tracer.py @@ -26,7 +26,7 @@ def test_tracer_span_top_level_attributes(self, test_agent: _TestAgentAPI, test_ "operation", service="my-webserver", resource="/endpoint", typestr="web" ) as parent: parent.set_metric("number", 10) - with test_library.start_span("operation.child", parent_id=parent.span_id) as child: + with test_library.start_span("operation.child", parent_id=parent.span_id) as child: # type: ignore child.set_meta("key", "val") traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -60,7 +60,7 @@ def test_tracer_repository_url_environment_variable( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=parent.span_id): + with test_library.start_span("operation.child", parent_id=parent.span_id): # type: ignore pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -86,7 +86,7 @@ def test_tracer_commit_sha_environment_variable( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=parent.span_id): + with test_library.start_span("operation.child", parent_id=parent.span_id): # type: ignore pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) @@ -146,7 +146,7 @@ def test_tracer_repository_url_strip_credentials( """ with test_library: with test_library.start_span("operation") as parent: - with test_library.start_span("operation.child", parent_id=parent.span_id): + with test_library.start_span("operation.child", parent_id=parent.span_id): # type: ignore pass traces = test_agent.wait_for_num_traces(1, sort_by_start=False) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index e8a014fdad..5fbdff47fe 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -2,12 +2,12 @@ import contextlib import time import urllib.parse -from typing import Any, Generator, List, Optional, Tuple, TypedDict, Union, Dict +from typing import Generator, List, Optional, Tuple, TypedDict, Union, Dict from docker.models.containers import Container import pytest from _pytest.outcomes import Failed -import requests # type: ignore +import requests from utils import context from utils.dd_constants import SpanKind, StatusCode @@ -32,8 +32,9 @@ class SpanResponse(TypedDict): class Link(TypedDict): - parent_id: int + parent_id: int # 0 to extract from headers attributes: dict + http_headers: List[Tuple[str, str]] class APMLibraryClient: @@ -119,7 +120,7 @@ def trace_start_span( # TODO: Update the cpp parametric app to accept null values for unset parameters service = service or "" resource = resource or "" - parent_id = parent_id or "" + parent_id = parent_id or 0 typestr = typestr or "" resp = self._session.post( @@ -187,21 +188,30 @@ def span_set_error(self, span_id: int, typestr: str, message: str, stack: str) - json={"span_id": span_id, "type": typestr, "message": message, "stack": stack}, ) - def span_add_link(self, span_id: int, parent_id: int, attributes: Optional[dict[Any, Any]] = None): + def span_add_link( + self, span_id: int, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None + ): + # Avoid using http_headers when creating a span link in the parametric apps + # Alternative endpoints will be provided to set these values. This will be documented in a future PR. self._session.post( self._url("/trace/span/add_link"), - json={"span_id": span_id, "parent_id": parent_id, "attributes": attributes or {},}, + json={ + "span_id": span_id, + "parent_id": parent_id, + "attributes": attributes or {}, + "http_headers": http_headers or [], + }, ) def span_get_baggage(self, span_id: int, key: str) -> str: resp = self._session.get(self._url("/trace/span/get_baggage"), json={"span_id": span_id, "key": key,},) - data = resp.json() - return data["baggage"] + resp = resp.json() + return resp["baggage"] def span_get_all_baggage(self, span_id: int) -> dict: resp = self._session.get(self._url("/trace/span/get_all_baggage"), json={"span_id": span_id}) - data = resp.json() - return data["baggage"] + resp = resp.json() + return resp["baggage"] def trace_inject_headers(self, span_id): resp = self._session.post(self._url("/trace/span/inject_headers"), json={"span_id": span_id},) @@ -226,7 +236,8 @@ def otel_trace_start_span( span_kind: SpanKind, parent_id: int, links: List[Link], - attributes: Optional[dict[Any, Any]] = None, + http_headers: List[Tuple[str, str]], + attributes: dict = None, ) -> StartSpanResponse: resp = self._session.post( self._url("/trace/otel/start_span"), @@ -236,6 +247,7 @@ def otel_trace_start_span( "span_kind": span_kind.value, "parent_id": parent_id, "links": links, + "http_headers": http_headers, "attributes": attributes or {}, }, ).json() @@ -292,8 +304,8 @@ def otel_set_baggage(self, span_id: int, key: str, value: str) -> None: resp = self._session.post( self._url("/trace/otel/otel_set_baggage"), json={"span_id": span_id, "key": key, "value": value} ) - data = resp.json() - return data["value"] + resp = resp.json() + return resp["value"] def get_tracer_config(self) -> Dict[str, Optional[str]]: resp = self._session.get(self._url("/trace/config")).json() @@ -357,8 +369,8 @@ def remove_all_baggage(self): def set_error(self, typestr: str = "", message: str = "", stack: str = ""): self._client.span_set_error(self.span_id, typestr, message, stack) - def add_link(self, parent_id: int, attributes: Optional[dict[Any, Any]] = None): - self._client.span_add_link(self.span_id, parent_id, attributes) + def add_link(self, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None): + self._client.span_add_link(self.span_id, parent_id, attributes, http_headers) def finish(self): self._client.finish_span(self.span_id) @@ -385,7 +397,7 @@ def set_status(self, code: StatusCode, description): self._client.otel_set_status(self.span_id, code, description) def add_event(self, name: str, timestamp: Optional[int] = None, attributes: Optional[dict] = None): - self._client.otel_add_event(self.span_id, name, timestamp, attributes) # type: ignore + self._client.otel_add_event(self.span_id, name, timestamp, attributes) def record_exception(self, message: str, attributes: Optional[dict] = None): self._client.otel_record_exception(self.span_id, message, attributes) @@ -428,12 +440,12 @@ def start_span( name: str, service: Optional[str] = None, resource: Optional[str] = None, - parent_id: Optional[int] = None, + parent_id: Optional[str] = None, typestr: Optional[str] = None, tags: Optional[List[Tuple[str, str]]] = None, ) -> Generator[_TestSpan, None, None]: resp = self._client.trace_start_span( - name=name, service=service, resource=resource, parent_id=str(parent_id), typestr=typestr, tags=tags, + name=name, service=service, resource=resource, parent_id=parent_id, typestr=typestr, tags=tags, ) span = _TestSpan(self._client, resp["span_id"], resp["trace_id"]) yield span @@ -451,7 +463,8 @@ def otel_start_span( span_kind: SpanKind = SpanKind.UNSPECIFIED, parent_id: int = 0, links: Optional[List[Link]] = None, - attributes: Optional[dict[Any, Any]] = None, + attributes: dict = None, + http_headers: Optional[List[Tuple[str, str]]] = None, ) -> Generator[_TestOtelSpan, None, None]: resp = self._client.otel_trace_start_span( name=name, @@ -460,10 +473,16 @@ def otel_start_span( parent_id=parent_id, links=links if links is not None else [], attributes=attributes, + http_headers=http_headers if http_headers is not None else [], ) span = _TestOtelSpan(self._client, resp["span_id"], resp["trace_id"]) yield span + return { + "span_id": resp["span_id"], + "trace_id": resp["trace_id"], + } + def flush(self) -> bool: return self._client.trace_flush() From ab21a3fbe3ee3ecb5fbe05171f8ea066aba56954 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 20 Nov 2024 12:06:40 -0500 Subject: [PATCH 18/18] Update _library_client.py --- utils/parametric/_library_client.py | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/utils/parametric/_library_client.py b/utils/parametric/_library_client.py index 5fbdff47fe..b8d9df4f8b 100644 --- a/utils/parametric/_library_client.py +++ b/utils/parametric/_library_client.py @@ -32,9 +32,8 @@ class SpanResponse(TypedDict): class Link(TypedDict): - parent_id: int # 0 to extract from headers + parent_id: int attributes: dict - http_headers: List[Tuple[str, str]] class APMLibraryClient: @@ -188,19 +187,10 @@ def span_set_error(self, span_id: int, typestr: str, message: str, stack: str) - json={"span_id": span_id, "type": typestr, "message": message, "stack": stack}, ) - def span_add_link( - self, span_id: int, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None - ): - # Avoid using http_headers when creating a span link in the parametric apps - # Alternative endpoints will be provided to set these values. This will be documented in a future PR. + def span_add_link(self, span_id: int, parent_id: int, attributes: dict = None): self._session.post( self._url("/trace/span/add_link"), - json={ - "span_id": span_id, - "parent_id": parent_id, - "attributes": attributes or {}, - "http_headers": http_headers or [], - }, + json={"span_id": span_id, "parent_id": parent_id, "attributes": attributes or {},}, ) def span_get_baggage(self, span_id: int, key: str) -> str: @@ -236,7 +226,6 @@ def otel_trace_start_span( span_kind: SpanKind, parent_id: int, links: List[Link], - http_headers: List[Tuple[str, str]], attributes: dict = None, ) -> StartSpanResponse: resp = self._session.post( @@ -247,7 +236,6 @@ def otel_trace_start_span( "span_kind": span_kind.value, "parent_id": parent_id, "links": links, - "http_headers": http_headers, "attributes": attributes or {}, }, ).json() @@ -369,8 +357,8 @@ def remove_all_baggage(self): def set_error(self, typestr: str = "", message: str = "", stack: str = ""): self._client.span_set_error(self.span_id, typestr, message, stack) - def add_link(self, parent_id: int, attributes: dict = None, http_headers: List[Tuple[str, str]] = None): - self._client.span_add_link(self.span_id, parent_id, attributes, http_headers) + def add_link(self, parent_id: int, attributes: dict = None): + self._client.span_add_link(self.span_id, parent_id, attributes) def finish(self): self._client.finish_span(self.span_id) @@ -464,7 +452,6 @@ def otel_start_span( parent_id: int = 0, links: Optional[List[Link]] = None, attributes: dict = None, - http_headers: Optional[List[Tuple[str, str]]] = None, ) -> Generator[_TestOtelSpan, None, None]: resp = self._client.otel_trace_start_span( name=name, @@ -473,7 +460,6 @@ def otel_start_span( parent_id=parent_id, links=links if links is not None else [], attributes=attributes, - http_headers=http_headers if http_headers is not None else [], ) span = _TestOtelSpan(self._client, resp["span_id"], resp["trace_id"]) yield span