diff --git a/docs/GRAPHITE.md b/docs/GRAPHITE.md index 2f011293..df37c4cb 100644 --- a/docs/GRAPHITE.md +++ b/docs/GRAPHITE.md @@ -73,10 +73,6 @@ tests: ``` ### Tags - -> [!WARNING] -> Tags do not work as expected in the current version. See https://github.com/apache/otava/issues/24 for more details - The optional `tags` property contains the tags that are used to query for Graphite events that store additional test run metadata such as run identifier, commit, branch and product version information. @@ -94,6 +90,21 @@ $ curl -X POST "http://graphite_address/events/" \ Posting those events is not mandatory, but when they are available, Otava is able to filter data by commit or version using `--since-commit` or `--since-version` selectors. +#### Supported Metadata Schema +The following tags are supported: + +| Field | Description | Default Value | +| :--- | :--- |:-----------------------| +| **`test_owner`** | The user or team responsible for the run. | `None` | +| **`test_name`** | The name of the test suite executed. | `None` | +| **`run_id`** | Unique identifier for the specific run. | `None` | +| **`status`** | The outcome (e.g., `success`, `failure`). | `None` | +| **`version`** | The product version being tested. | `None` | +| **`branch`** | The VCS branch name. | `None` | +| **`commit`** | The specific commit hash. | `None` | +| **`start_time`** | Timestamp of test start. | `None` | +| **`end_time`** | Timestamp of test end. | `None` | + ## Example Start docker-compose with Graphite in one tab: @@ -111,15 +122,15 @@ docker-compose -f examples/graphite/docker-compose.yaml run --rm otava analyze m Expected output: ```bash -time run branch version commit throughput response_time cpu_usage -------------------------- ----- -------- --------- -------- ------------ --------------- ----------- -2024-12-14 22:45:10 +0000 61160 87 0.2 -2024-12-14 22:46:10 +0000 60160 85 0.3 -2024-12-14 22:47:10 +0000 60960 89 0.1 - ············ ··········· - -5.6% +300.0% - ············ ··········· -2024-12-14 22:48:10 +0000 57123 88 0.8 -2024-12-14 22:49:10 +0000 57980 87 0.9 -2024-12-14 22:50:10 +0000 56950 85 0.7 +time run branch version commit throughput response_time cpu_usage +------------------------- ----- ----------- --------- -------- ------------ --------------- ----------- +2026-04-18 15:42:50 +0000 new-feature 0.0.1 p7q8r9 61160 87 0.2 +2026-04-18 15:43:50 +0000 new-feature 0.0.1 m4n5o6 60160 85 0.3 +2026-04-18 15:44:50 +0000 new-feature 0.0.1 j1k2l3 60960 89 0.1 + ············ ··········· + -5.6% +300.0% + ············ ··········· +2026-04-18 15:45:50 +0000 new-feature 0.0.1 g7h8i9 57123 88 0.8 +2026-04-18 15:46:50 +0000 new-feature 0.0.1 d4e5f6 57980 87 0.9 +2026-04-18 15:47:50 +0000 new-feature 0.0.1 a1b2c3 56950 85 0.7 ``` diff --git a/examples/graphite/datagen/datagen.sh b/examples/graphite/datagen/datagen.sh index 8217ceb0..b9e8b5dd 100755 --- a/examples/graphite/datagen/datagen.sh +++ b/examples/graphite/datagen/datagen.sh @@ -42,14 +42,13 @@ send_to_graphite() { # send the metric echo "${throughput_path} ${value} ${timestamp}" | nc ${GRAPHITE_SERVER} ${GRAPHITE_PORT} # annotate the metric - # Commented out, waiting for https://github.com/apache/otava/issues/24 to be fixed - # curl -X POST "http://${GRAPHITE_SERVER}/events/" \ - # -d "{ - # \"what\": \"Performance Test\", - # \"tags\": [\"perf-test\", \"daily\", \"my-product\"], - # \"when\": ${timestamp}, - # \"data\": {\"commit\": \"${commit}\", \"branch\": \"new-feature\", \"version\": \"0.0.1\"} - # }" + curl -X POST "http://${GRAPHITE_SERVER}/events/" \ + -d "{ + \"what\": \"Performance Test\", + \"tags\": [\"perf-test\", \"daily\", \"my-product\"], + \"when\": ${timestamp}, + \"data\": {\"commit\": \"${commit}\", \"branch\": \"new-feature\", \"version\": \"0.0.1\"} + }" } diff --git a/examples/graphite/docker-compose.yaml b/examples/graphite/docker-compose.yaml index ea3c107e..2e2d2143 100644 --- a/examples/graphite/docker-compose.yaml +++ b/examples/graphite/docker-compose.yaml @@ -43,13 +43,14 @@ services: - otava-graphite data-sender: - image: bash + image: alpine:latest container_name: data-sender depends_on: - graphite volumes: - ./datagen:/datagen - entrypoint: ["bash", "/datagen/datagen.sh"] + # Install bash and curl before running the script + entrypoint: ["/bin/sh", "-c", "apk add --no-cache bash curl && bash /datagen/datagen.sh"] networks: - otava-graphite @@ -68,7 +69,6 @@ services: - otava-graphite volumes: - ./config:/config - networks: otava-graphite: driver: bridge diff --git a/otava/graphite.py b/otava/graphite.py index 69a7592e..b81a2ebc 100644 --- a/otava/graphite.py +++ b/otava/graphite.py @@ -24,7 +24,7 @@ from typing import Dict, Iterable, List, Optional from otava.data_selector import DataSelector -from otava.util import parse_datetime +from otava.util import clean_str, parse_datetime @dataclass @@ -57,7 +57,6 @@ class TimeSeries: def decode_graphite_datapoints(series: Dict[str, List[List[float]]]) -> List[DataPoint]: - points = series["datapoints"] return [DataPoint(int(p[1]), p[0]) for p in points if p[0] is not None] @@ -80,49 +79,32 @@ class GraphiteError(IOError): @dataclass class GraphiteEvent: - test_owner: str - test_name: str - run_id: str - status: str - start_time: datetime pub_time: datetime - end_time: datetime - version: Optional[str] - branch: Optional[str] - commit: Optional[str] + test_owner: Optional[str] = None + test_name: Optional[str] = None + run_id: Optional[str] = None + status: Optional[str] = None + start_time: Optional[datetime] = None + end_time: Optional[datetime] = None + version: Optional[str] = None + branch: Optional[str] = None + commit: Optional[str] = None - def __init__( - self, - pub_time: int, - test_owner: str, - test_name: str, - run_id: str, - status: str, - start_time: int, - end_time: int, - version: Optional[str], - branch: Optional[str], - commit: Optional[str], - ): - self.test_owner = test_owner - self.test_name = test_name - self.run_id = run_id - self.status = status - self.start_time = parse_datetime(str(start_time)) - self.pub_time = parse_datetime(str(pub_time)) - self.end_time = parse_datetime(str(end_time)) - if len(version) == 0 or version == "null": - self.version = None - else: - self.version = version - if len(branch) == 0 or branch == "null": - self.branch = None - else: - self.branch = branch - if len(commit) == 0 or commit == "null": - self.commit = None - else: - self.commit = commit + + def __post_init__(self): + if self.pub_time is None: + raise ValueError("pub_time is required and cannot be None") + # Ensure pub_time is always a datetime + self.pub_time = parse_datetime(str(self.pub_time)) + + + self.version = clean_str(self.version) + self.branch = clean_str(self.branch) + self.commit = clean_str(self.commit) + self.test_owner = clean_str(self.test_owner) + self.test_name = clean_str(self.test_name) + self.run_id = clean_str(self.run_id) + self.status = clean_str(self.status) def compress_target_paths(paths: List[str]) -> List[str]: @@ -190,7 +172,7 @@ def fetch_events( data_str = urllib.request.urlopen(url).read() data_as_json = json.loads(data_str) return [ - GraphiteEvent(event.get("when"), **ast.literal_eval(event.get("data"))) + GraphiteEvent(pub_time=event.get("when"), **ast.literal_eval(event.get("data"))) for event in data_as_json if event.get("what") == "Performance Test" ] diff --git a/otava/util.py b/otava/util.py index 1fcf072e..9cbf2716 100644 --- a/otava/util.py +++ b/otava/util.py @@ -20,10 +20,10 @@ import sys from collections import OrderedDict, deque from dataclasses import dataclass -from datetime import datetime +from datetime import datetime, timezone from functools import reduce from itertools import islice -from typing import Dict, List, Optional, Set, TypeVar +from typing import Dict, List, Optional, Set, TypeVar, Union import dateparser from pytz import UTC @@ -134,18 +134,47 @@ class DateFormatError(ValueError): message: str -def parse_datetime(date: Optional[str]) -> Optional[datetime]: +def parse_datetime(date: Optional[Union[str, int, float, datetime]]) -> Optional[datetime]: """ - Converts a human-readable string into a datetime object. - Accepts many formats and many languages, see dateparser package. - Raises DataFormatError if the input string format hasn't been recognized. + Normalize various datetime inputs into a timezone-aware datetime. + + Supports: + - datetime (returned as-is) + - int/float (treated as Unix timestamp) + - str (parsed via dateparser) + - None (returns None) """ if date is None: return None - parsed: datetime = dateparser.parse(date, settings={"RETURN_AS_TIMEZONE_AWARE": True}) - if parsed is None: - raise DateFormatError(f"Invalid datetime value: {date}") - return parsed + + if isinstance(date, datetime): + return date + + if isinstance(date, (int, float)): + return datetime.fromtimestamp(date, tz=timezone.utc) + + if isinstance(date, str): + parsed = dateparser.parse(date, settings={"RETURN_AS_TIMEZONE_AWARE": True}) + if parsed is None: + raise DateFormatError(f"Invalid datetime value: {date}") + return parsed + + raise TypeError(f"Unsupported type for datetime parsing: {type(date)}") + + +def clean_str(value: Optional[str]) -> Optional[str]: + if value is None: + return None + + if not isinstance(value, str): + return value # or raise TypeError if you want strictness + + value = value.strip() + + if value == "" or value.lower() == "null": + return None + + return value def sliding_window(iterable, size): diff --git a/tests/graphite_test.py b/tests/graphite_test.py index 0fbfd4c6..8c7f8dbc 100644 --- a/tests/graphite_test.py +++ b/tests/graphite_test.py @@ -14,8 +14,17 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import json +import urllib +from datetime import datetime +from unittest.mock import patch -from otava.graphite import compress_target_paths +from otava.graphite import ( + Graphite, + GraphiteConfig, + GraphiteEvent, + compress_target_paths, +) def test_compress_target_paths(): @@ -34,3 +43,79 @@ def test_compress_target_paths(): "foo.foo.baz.{p50,p75,throughput}", "something.else", } + + + +def test_graphite_event_cleaning_and_datetime(): + event = GraphiteEvent( + pub_time=1700000000, # int timestamp + version="0.1", + branch="null", + commit="", + ) + + assert isinstance(event.pub_time, datetime) + assert event.pub_time.tzinfo is not None + + assert event.version == "0.1" + assert event.branch is None + assert event.commit is None + +def test_graphite_event_dirty_data_handling(): + event = GraphiteEvent( + pub_time="2024-01-01 10:00:00", + version="null", + branch="", + commit=None, + ) + + assert event.version is None + assert event.branch is None + assert event.commit is None + assert isinstance(event.pub_time, datetime) + + + +class FakeResponse: + def __init__(self, data: bytes): + self._data = data + + def read(self): + return self._data + + +def test_fetch_events_parses_graphite_response(): + sample_data = [ + { + "when": 1776526076, + "what": "Performance Test", + "data": "{'commit': 'p7q8r9', 'branch': 'new-feature', 'version': '0.0.1'}", + "tags": ["perf-test", "daily", "my-product"], + "id": 16, + }, + { + "when": 1776526136, + "what": "Performance Test", + "data": "{'commit': 'm4n5o6', 'branch': 'new-feature', 'version': '0.0.1'}", + "tags": ["perf-test", "daily", "my-product"], + "id": 17, + }, + ] + + response_bytes = json.dumps(sample_data).encode("utf-8") + + def fake_urlopen(*args, **kwargs): + return FakeResponse(response_bytes) + + with patch.object(urllib.request, "urlopen", fake_urlopen): + g = Graphite(GraphiteConfig(url="http://graphite/")) + + events = g.fetch_events(tags=["perf-test"]) + + assert len(events) == 2 + + assert events[0].commit == "p7q8r9" + assert events[0].branch == "new-feature" + assert events[0].version == "0.0.1" + + assert events[1].commit == "m4n5o6" \ No newline at end of file diff --git a/tests/utils_test.py b/tests/utils_test.py new file mode 100644 index 00000000..e5f09ed5 --- /dev/null +++ b/tests/utils_test.py @@ -0,0 +1,96 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from datetime import datetime, timezone + +import pytest + +from otava.util import clean_str, parse_datetime + + +def test_clean_str_none(): + assert clean_str(None) is None + + +def test_clean_str_empty_string(): + assert clean_str("") is None + + +@pytest.mark.parametrize("value", ["null", "NULL", "NuLl"]) +def test_clean_str_null_variants(value): + assert clean_str(value) is None + + +@pytest.mark.parametrize("value", [" ", " ", "\t", "\n", "\r\n"]) +def test_clean_str_whitespace_only(value): + assert clean_str(value) is None + + +@pytest.mark.parametrize("value", [ + "abc", + "commit123", + "new-feature", + "v1.0.0", +]) +def test_clean_str_valid_strings(value): + assert clean_str(value) == value + + +@pytest.mark.parametrize("value,expected", [ + (" abc ", "abc"), + (" null ", None), + ("NULL", None), + (" ", None), +]) +def test_clean_str_mixed_cases(value, expected): + assert clean_str(value) == expected + + +def test_clean_str_non_string_input(): + assert clean_str(123) == 123 + assert clean_str(0) == 0 + assert clean_str(False) is False + + + +def test_parse_datetime_none(): + assert parse_datetime(None) is None + + +def test_parse_datetime_pass_through_datetime(): + dt = datetime(2024, 1, 1, tzinfo=timezone.utc) + assert parse_datetime(dt) == dt + + +def test_parse_datetime_timestamp_int(): + ts = 1700000000 + result = parse_datetime(ts) + + assert isinstance(result, datetime) + assert result.tzinfo is not None # ensures timezone-aware + + +def test_parse_datetime_string(): + result = parse_datetime("2024-01-01 10:00:00") + + assert isinstance(result, datetime) + assert result.tzinfo is not None + + +def test_parse_datetime_invalid_type(): + with pytest.raises(TypeError): + parse_datetime(["bad input"]) \ No newline at end of file