Adding maximum request size (MiB) to config

- adding a CLI option for this
- synchronizing the default size treatment across gitlab/koji/server/cli

Signed-off-by: Jan Matufka <jmatufka@redhat.com>

Author: Jany26

logdetective/constants.py

@@ -85,5 +85,6 @@
 
 # Other constants
 
-# Maximum log size is 300 MiB
-MAXIMUM_LOG_LENGTH = 300 * 1024 * 1024
+# Default maximum log size is 300 MiB,
+# for server it can be overwritten in config as max_artifact_size (in MiB)
+DEFAULT_MAXIMUM_LOG_MIB = 300

logdetective/logdetective.py

@@ -6,7 +6,11 @@
 
 import aiohttp
 
-from logdetective.constants import DEFAULT_ADVISOR, DEFAULT_TEMPERATURE
+from logdetective.constants import (
+    DEFAULT_ADVISOR,
+    DEFAULT_TEMPERATURE,
+    DEFAULT_MAXIMUM_LOG_MIB,
+)
 from logdetective.utils import (
     process_log,
     initialize_model,
@@ -17,6 +21,7 @@
     check_csgrep,
     mine_logs,
     sanitize_log,
+    mib_to_bytes,
 )
 from logdetective.remote_log import retrieve_log_content
 from logdetective.extractors import DrainExtractor, CSGrepExtractor
@@ -87,6 +92,12 @@ def setup_args():
     parser.add_argument(
         "--csgrep", action="store_true", help="Use csgrep to process the log."
     )
+    parser.add_argument(
+        "--mib_limit",
+        type=int,
+        default=DEFAULT_MAXIMUM_LOG_MIB,
+        help="Maximum log size in MiB (default: %(default)s)",
+    )
     return parser.parse_args()
 
 
@@ -151,7 +162,7 @@ async def run():  # pylint: disable=too-many-statements,too-many-locals,too-many
 
     async with aiohttp.ClientSession() as http:
         try:
-            log = await retrieve_log_content(http, args.file)
+            log = await retrieve_log_content(http, args.file, mib_to_bytes(args.mib_limit))
         except ValueError as e:
             # file does not exist
             LOG.error(e)

logdetective/remote_log.py

@@ -5,10 +5,11 @@
 import aiohttp
 from aiohttp.web import HTTPBadRequest
 
-from logdetective.constants import MAXIMUM_LOG_LENGTH
+from logdetective.constants import DEFAULT_MAXIMUM_LOG_MIB
 from logdetective.utils import (
     ContentSizeCheck,
     check_content_size,
+    mib_to_bytes,
 )
 
 LOG = logging.getLogger("logdetective")
@@ -19,16 +20,23 @@ class RemoteLog:
     Handles retrieval of remote log files.
     """
 
-    def __init__(self, url: str, http_session: aiohttp.ClientSession):
+    def __init__(
+        self,
+        url: str,
+        http_session: aiohttp.ClientSession,
+        limit_bytes: int = mib_to_bytes(DEFAULT_MAXIMUM_LOG_MIB)
+    ):
         """
         Initialize with a remote log URL and HTTP session.
 
         Args:
             url: A remote URL pointing to a log file
             http_session: The HTTP session used to retrieve the remote file
+            limit_bytes: For checking the log size on the accessed URL
         """
         self._url = url
         self._http_session = http_session
+        self._limit_bytes = limit_bytes
 
     @property
     def url(self) -> str:
@@ -67,7 +75,7 @@ async def get_url_content(self) -> str:
             raise RuntimeError(f"We couldn't obtain the logs: {ex}") from ex
         size_check: ContentSizeCheck = check_content_size(
             head_response.headers,
-            MAXIMUM_LOG_LENGTH
+            self._limit_bytes
         )
         if not size_check.result:
             raise RuntimeError(
@@ -86,7 +94,7 @@ async def process_url(self) -> str:
             raise HTTPBadRequest(reason=f"We couldn't obtain the logs: {ex}") from ex
 
 
-async def retrieve_log_content(http: aiohttp.ClientSession, log_path: str) -> str:
+async def retrieve_log_content(http: aiohttp.ClientSession, log_path: str, size_limit: int) -> str:
     """Get content of the file on the log_path path.
     Path is assumed to be valid URL if it has a scheme.
     Otherwise it attempts to pull it from local filesystem."""
@@ -101,7 +109,8 @@ async def retrieve_log_content(http: aiohttp.ClientSession, log_path: str) -> st
             log = f.read()
 
     else:
-        remote_log = RemoteLog(log_path, http)
+        remote_log = RemoteLog(log_path, http, limit_bytes=size_limit)
+        # limited to DEFAULT_MAXIMUM_LOG_LENGTH (300 MiB)
         log = await remote_log.get_url_content()
 
     return log

logdetective/server/metric.py

@@ -9,7 +9,7 @@
 from starlette.responses import StreamingResponse
 
 from logdetective.remote_log import RemoteLog
-from logdetective.server.config import LOG
+from logdetective.server.config import LOG, SERVER_CONFIG
 from logdetective.server.compressors import (
     TextCompressor,
     RemoteLogCompressor,
@@ -45,7 +45,11 @@ async def add_new_metrics_url(
             raise LogDetectiveMetricsError(
                 f"""Remote log can not be retrieved without URL and http session.
                 URL: {url}, http session:{http_session}""")
-        remote_log = RemoteLog(url, http_session)
+        remote_log = RemoteLog(
+            url,
+            http_session,
+            limit_bytes=SERVER_CONFIG.general.max_artifact_size
+        )
         compressed_log_content = await RemoteLogCompressor(remote_log).zip_content()
 
     # gitlab and koji always fall through here

logdetective/server/models.py

@@ -19,9 +19,9 @@
     USER_ROLE_DEFAULT,
     LLM_MAX_CONCURRENT_REQUESTS,
     LLM_MAX_KEEP_ALIVE_CONNECTIONS,
-    MAXIMUM_LOG_LENGTH,
+    DEFAULT_MAXIMUM_LOG_MIB
 )
-from logdetective.utils import check_csgrep
+from logdetective.utils import check_csgrep, mib_to_bytes
 
 
 class BuildLogFile(BaseModel):
@@ -33,7 +33,10 @@ class BuildLogFile(BaseModel):
         max_length=255,
         pattern=r"^[a-zA-Z0-9._\-\/ ]+$"
     )
-    content: str = Field(max_length=MAXIMUM_LOG_LENGTH)
+    content: str = Field(
+        description="Log file content as a string. By default, request size "
+        "is limited to 300 MiB. This can affect what kind of logs can be submitted."
+    )
 
 
 class BuildLogRequest(BaseModel):
@@ -257,16 +260,15 @@ class GitLabInstanceConfig(BaseModel):  # pylint: disable=too-many-instance-attr
 
     timeout: float = 5.0
 
-    # Maximum size of artifacts.zip in MiB. (default: 300 MiB)
-    max_artifact_size: int = 300 * 1024 * 1024
+    # Maximum size of artifacts.zip (default: 300 MiB)
+    # In config, the unit is in MiB, but this max_artifact_size attribute will be in bytes
+    max_artifact_size: int = mib_to_bytes(DEFAULT_MAXIMUM_LOG_MIB)
 
     @field_validator("max_artifact_size", mode="before")
     @classmethod
     def megabytes_to_bytes(cls, v: Any):
         """Convert max_artifact_size from megabytes to bytes."""
-        if isinstance(v, int):
-            return v * 1024 * 1024
-        return 300 * 1024 * 1024
+        return mib_to_bytes(v if isinstance(v, int) else DEFAULT_MAXIMUM_LOG_MIB)
 
 
 class GitLabConfig(BaseModel):
@@ -302,15 +304,15 @@ class KojiConfig(BaseModel):
 
     instances: Dict[str, KojiInstanceConfig] = {}
     analysis_timeout: int = 15
-    max_artifact_size: int = 300 * 1024 * 1024
+
+    # in yaml config, this is given in MiB, but we use bytes in code (same as gitlab)
+    max_artifact_size: int = mib_to_bytes(DEFAULT_MAXIMUM_LOG_MIB)
 
     @field_validator("max_artifact_size", mode="before")
     @classmethod
     def megabytes_to_bytes(cls, v: Any):
         """Convert max_artifact_size from megabytes to bytes."""
-        if isinstance(v, int):
-            return v * 1024 * 1024
-        return 300 * 1024 * 1024
+        return mib_to_bytes(v if isinstance(v, int) else DEFAULT_MAXIMUM_LOG_MIB)
 
     @model_validator(mode="before")
     @classmethod
@@ -347,6 +349,14 @@ class GeneralConfig(BaseModel):
     collect_emojis_interval: int = 60 * 60  # seconds
     top_k_snippets: int = 0
     report_certainty: bool = False
+    # max_artifact_size in config.yml is in MiBs, here (GeneralConfig class) is in bytes
+    max_artifact_size: int = mib_to_bytes(DEFAULT_MAXIMUM_LOG_MIB)
+
+    @field_validator("max_artifact_size", mode="before")
+    @classmethod
+    def megabytes_to_bytes(cls, v: Any):
+        """Convert max_artifact_size from megabytes to bytes."""
+        return mib_to_bytes(v if isinstance(v, int) else DEFAULT_MAXIMUM_LOG_MIB)
 
 
 class Config(BaseModel):

logdetective/server/utils.py

@@ -4,12 +4,12 @@
 import aiohttp
 from fastapi import Request, HTTPException
 
-from logdetective.constants import SNIPPET_DELIMITER, MAXIMUM_LOG_LENGTH
+from logdetective.constants import SNIPPET_DELIMITER
 from logdetective.utils import (
     ContentSizeCheck,
     check_content_size,
 )
-from logdetective.server.config import LOG
+from logdetective.server.config import LOG, SERVER_CONFIG
 from logdetective.server.exceptions import LogDetectiveConnectionError
 from logdetective.remote_log import RemoteLog
 from logdetective.server.models import (
@@ -115,7 +115,11 @@ async def get_log_from_payload(
     log_text = ""
     if payload.url:
         LOG.info("Handling log as URL")
-        remote_log = RemoteLog(payload.url, http_session)
+        remote_log = RemoteLog(
+            payload.url,
+            http_session,
+            limit_bytes=SERVER_CONFIG.general.max_artifact_size
+        )
         log_text = await remote_log.process_url()
     elif payload.files:
         # pydantic field validators make sure at least one element is present,
@@ -156,16 +160,20 @@ def validate_request_size(request: Request) -> None:
         HTTPException(411): If Content-Length header is missing or invalid
         HTTPException(413): If Content-Length exceeds maximum allowed size
     """
-    size_check: ContentSizeCheck = check_content_size(dict(request.headers), MAXIMUM_LOG_LENGTH)
+    size_check: ContentSizeCheck = check_content_size(
+        request.headers,
+        SERVER_CONFIG.general.max_artifact_size
+    )
     if not (size_check.value_present and size_check.size_in_bytes is not None):
         raise HTTPException(status_code=411, detail="Content-Length is missing or invalid.")
     if not size_check.result:
-        size = size_check.size_in_bytes
         raise HTTPException(
             status_code=413,
             detail=(
                 f"Content-Length is too large: "
-                f"{size} B ({size / (1024 * 1024):.2f} MiB) > "
-                f"{MAXIMUM_LOG_LENGTH} B ({MAXIMUM_LOG_LENGTH / (1024 * 1024):.2f} MiB)"
+                f"{size_check.size_in_bytes} B "
+                f"({size_check.size_in_bytes / (1024 * 1024):.2f} MiB) > "
+                f"{SERVER_CONFIG.general.max_artifact_size} B "
+                f"({SERVER_CONFIG.general.max_artifact_size / (1024 * 1024):.2f} MiB)"
             )
         )

logdetective/utils.py

@@ -60,6 +60,11 @@
 ]
 
 
+# pylint: disable=missing-function-docstring
+def mib_to_bytes(mib: int) -> int:
+    return mib * 1024 * 1024
+
+
 def new_message(text: str) -> bool:
     """Set of heuristics for determining whether or not
     does the current chunk of log text continue on next line.

server/config.yml

@@ -57,6 +57,7 @@ extractor:
 #       tokens:
 #         - example_token
 general:
+  max_artifact_size: 300  # in MiB
   devmode: False
   packages:
     - .*

tests/base/test_utils.py

@@ -1,5 +1,4 @@
 from unittest import mock
-from contextlib import nullcontext
 
 import aiohttp
 import aioresponses
@@ -14,8 +13,9 @@
     filter_snippet_patterns,
     load_skip_snippet_patterns,
     get_chunks,
+    mib_to_bytes,
 )
-
+from logdetective.constants import DEFAULT_MAXIMUM_LOG_MIB
 from logdetective.remote_log import RemoteLog
 from logdetective.models import PromptConfig, SkipSnippets
 from logdetective import constants
@@ -78,26 +78,30 @@ def test_load_prompts_correct_path():
 
 @pytest.mark.asyncio
 @pytest.mark.parametrize(
-    "content_length, exception",
+    "content_length, does_raise, exception_match",
     [
-        ("3", None),
-        (str(301 * 1024 * 1024), "over the limit"),
-        ("test", "invalid"),
+        ("3", False, None),
+        (str(mib_to_bytes(DEFAULT_MAXIMUM_LOG_MIB) + 1), True, "over the limit"),
+        ("test", True, "invalid"),
     ],
     indirect=False
 )
-async def test_get_url_content(content_length, exception):
+async def test_get_url_content(content_length, does_raise, exception_match):
+    url = "http://localhost:8999/"
     mock_head_response = {"Content-Length": content_length}
     mock_response = "123"
     with aioresponses.aioresponses() as mock:
-        mock.head("http://localhost:8999/", status=200, headers=mock_head_response)
-        mock.get("http://localhost:8999/", status=200, body=mock_response)
+        mock.head(url, status=200, headers=mock_head_response)
+        mock.get(url, status=200, body=mock_response)
         async with aiohttp.ClientSession() as http:
-            with pytest.raises(RuntimeError, match=exception) if exception else nullcontext():
-                url_output_cr = RemoteLog("http://localhost:8999/", http).get_url_content()
+            if does_raise:
+                with pytest.raises(RuntimeError, match=exception_match):
+                    url_output_cr = RemoteLog(url, http).get_url_content()
+                    url_output = await url_output_cr
+            else:
+                url_output_cr = RemoteLog(url, http).get_url_content()
                 url_output = await url_output_cr
-                if not exception:
-                    assert url_output == "123"
+                assert url_output == "123"
 
 
 @pytest.mark.parametrize("user_role", ["user", "something"])

tests/server/test_server.py

@@ -276,7 +276,9 @@ async def test_get_log_from_payload_url_sanitization():
     mock_remote_log = flexmock(RemoteLog)
 
     async with aiohttp.ClientSession() as session:
-        mock_remote_log.should_receive("__init__").with_args("http://path.to/file.log", session)
+        mock_remote_log.should_receive("__init__").with_args(
+            "http://path.to/file.log", session, limit_bytes=SERVER_CONFIG.general.max_artifact_size
+        )
         mock_remote_log.should_receive("process_url").and_return(awaited_dirty_log)
         log_text = await get_log_from_payload(payload, session)