Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding mypy checks to parametric #3488

Merged
merged 20 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/edit/format.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
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:
Expand Down
19 changes: 17 additions & 2 deletions format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,26 @@ 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

# 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"
rachelyangdog marked this conversation as resolved.
Show resolved Hide resolved
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)$'
Expand Down
7 changes: 7 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 4 additions & 7 deletions tests/parametric/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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(
Expand Down
7 changes: 3 additions & 4 deletions tests/parametric/test_library_tracestats.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions tests/parametric/test_telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 6 additions & 4 deletions tests/parametric/test_tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
rachelyangdog marked this conversation as resolved.
Show resolved Hide resolved
child.set_meta("key", "val")

traces = test_agent.wait_for_num_traces(1, sort_by_start=False)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"]

Expand All @@ -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"]
2 changes: 1 addition & 1 deletion tests/parametric/test_tracer_flare.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
44 changes: 27 additions & 17 deletions utils/parametric/_library_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand All @@ -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},)
Expand All @@ -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"),
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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
rachelyangdog marked this conversation as resolved.
Show resolved Hide resolved
# return {
# "span_id": resp["span_id"],
# "trace_id": resp["trace_id"],
# }

def flush(self) -> bool:
return self._client.trace_flush()
Expand Down
2 changes: 1 addition & 1 deletion utils/parametric/spec/trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading