Replaced the IPS_TO_SKIP regex with the upstream list of excluded IP addresses

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 # Upcoming
 
+### ⚠️ Breaking
+
+- Replaced the `IPS_TO_SKIP` regex with the upstream list of excluded IP addresses in `~/.s3-log-extraction/excluded_ips.txt`, which extraction now also reads, so that file is the only list. Only exact addresses are accepted, so convert any regex to a list of addresses before upgrading. The upstream pin is raised to `>=1.14.1`. ([#108](https://github.com/dandi/dandi-s3-log-extraction/pull/108))
+
 ### 🚀 Enhancement
 
 - Added a `--check` flag to `dandis3logextraction extract`, which reports whether any log files are pending extraction and exits without extracting. It exits with status 3 when some are pending and 0 when none are, so a scheduled job can start an extraction only when needed. Raised the upstream pin to `s3_log_extraction>=1.14.1`, the release that provides the check. ([#107](https://github.com/dandi/dandi-s3-log-extraction/pull/107))

README.md

@@ -40,14 +40,11 @@ pip install dandi-s3-log-extraction
 
 Usage on the DANDI archive logs requires a bit more customization than the generic package.
 
-Begin by ensuring a special required environment variable is set:
+Begin by listing the IP addresses to exclude, such as all associated Drogon IPs (ask Yarik or Cody for the list), in `~/.s3-log-extraction/excluded_ips.txt`. This file, read by the upstream `s3-log-extraction` package, is the only list of excluded addresses. Their requests are dropped during extraction and left out of every summary.
 
-**IPS_TO_SKIP**
-  - A regular expression matching all associated Drogon IPs, which are skipped during extraction.
+The file holds one IPv4 or IPv6 address per line. Anything after a `#` is a comment, and blank lines are ignored. Every entry must be a single exact address. Networks in CIDR notation, wildcards, and regular expressions are refused with an error.
 
-```bash
-export IPS_TO_SKIP="ask_yarik_or_cody_for_regex"
-```
+Exclusion only works in one direction over time. Adding an address drops its requests from future extraction. Removing an address from the file does not bring its requests back, since they were never extracted. To include an address again, rerun the extraction from scratch with `s3logextraction reset extraction` followed by a full `dandis3logextraction extract`.
 
 The `S3_LOG_EXTRACTION_PASSWORD` is no longer required by this package. It is only relevant if you opt in to the upstream `s3-log-extraction` encryption of the IP index and geolocation caches.
 

pyproject.toml

@@ -15,7 +15,7 @@ allow-direct-references = true
 
 [project]
 name = "dandi-s3-log-extraction"
-version="1.6.2"
+version="1.7.0"
 authors = [
   { name="Cody Baker", email="cody.c.baker.phd@gmail.com" },
 ]

src/dandi_s3_log_extraction/extractors/_dandi_extraction.awk

@@ -1,18 +1,18 @@
 BEGIN {
     FS = "HTTP/1\\."
 
-    if (!("IPS_TO_SKIP_REGEX" in ENVIRON)) {
-        print "Environment variable 'IPS_TO_SKIP_REGEX' is not set" > "/dev/stderr"
-        exit 1
-    }
-    IPS_TO_SKIP_REGEX = ENVIRON["IPS_TO_SKIP_REGEX"]
-
     if (!("EXTRACTION_DIRECTORY" in ENVIRON)) {
         print "Environment variable 'EXTRACTION_DIRECTORY' is not set" > "/dev/stderr"
         exit 1
     }
     EXTRACTION_DIRECTORY = ENVIRON["EXTRACTION_DIRECTORY"] "/"
 
+    # A space-separated list of exact addresses, empty or unset to exclude nothing
+    number_of_excluded_ips = split(ENVIRON["EXCLUDED_IPS"], excluded_ip_list, " ")
+    for (i = 1; i <= number_of_excluded_ips; i++) {
+        EXCLUDED_IPS[excluded_ip_list[i]] = 1
+    }
+
     MONTH_TO_NUMERIC["Jan"] = "01"
     MONTH_TO_NUMERIC["Feb"] = "02"
     MONTH_TO_NUMERIC["Mar"] = "03"
@@ -36,7 +36,7 @@ BEGIN {
     if (request_type != "REST.GET.OBJECT") {next}
 
     ip = pre_uri_fields[5]
-    if (ip ~ IPS_TO_SKIP_REGEX) {next}
+    if (ip in EXCLUDED_IPS) {next}
 
     split($2, post_uri_fields, " ")
     status = post_uri_fields[2]

src/dandi_s3_log_extraction/extractors/_dandi_remote_s3_log_access_extractor.py

@@ -1,4 +1,4 @@
-import os
+import collections.abc
 import pathlib
 
 import s3_log_extraction
@@ -21,12 +21,25 @@ class DandiRemoteS3LogAccessExtractor(s3_log_extraction.extractors.RemoteS3LogAc
             - Invoke the command `s3logextraction stop` to end the processes after the current round of completion.
             - Manually create a file in the extraction cache called '.stop_extraction'.
       - updatable
+
+    Parameters
+    ----------
+    cache_directory : pathlib.Path | None, optional
+        The directory holding the extraction cache and its records.
+        Defaults to the configured cache directory.
+    excluded_ips : iterable of str, optional
+        The IP addresses whose requests are dropped during extraction, so they never enter the cache.
+        Defaults to `s3_log_extraction.config.get_excluded_ips()`, the addresses listed in
+        `~/.s3-log-extraction/excluded_ips.txt`.
+        An explicit value replaces that default, so pass an empty list to drop nothing.
     """
 
-    def __init__(self, cache_directory: pathlib.Path | None = None) -> None:
-        super().__init__(cache_directory=cache_directory, use_encryption=False)
+    def __init__(
+        self,
+        *,
+        cache_directory: pathlib.Path | None = None,
+        excluded_ips: collections.abc.Iterable[str] | None = None,
+    ) -> None:
+        super().__init__(cache_directory=cache_directory, use_encryption=False, excluded_ips=excluded_ips)
 
         self._relative_script_path = pathlib.Path(__file__).parent / "_dandi_extraction.awk"
-
-        ips_to_skip_regex = os.environ.get("IPS_TO_SKIP", "")
-        self._awk_env["IPS_TO_SKIP_REGEX"] = ips_to_skip_regex

src/dandi_s3_log_extraction/summarize/_generate_dandiset_summaries.py

@@ -99,9 +99,9 @@ def generate_dandiset_summaries(
         the GeoLite2 database in the cache directory.
     excluded_ips : iterable of str, optional
         Individual addresses left out of every summary, including bytes sent, requests, downloads, views, and
-        the requester counts. Defaults to the addresses listed in the upstream `EXCLUDED_IPS_FILE_PATH`
-        (`~/.s3-log-extraction/excluded_ips.txt`), which is none when that file is absent. Pass an empty
-        iterable to exclude no address regardless of that file.
+        the requester counts. Defaults to the upstream `s3_log_extraction.config.get_excluded_ips()`, the
+        addresses listed in `~/.s3-log-extraction/excluded_ips.txt`, which is none when that file is absent.
+        Pass an empty iterable to exclude no address regardless of that file.
     """
     cache_directory = (
         pathlib.Path(cache_directory) if cache_directory is not None else s3_log_extraction.config.get_cache_directory()

tests/test_dandi_extra_coverage.py

@@ -61,19 +61,68 @@ def test_handle_max_workers_exceeds_cpu_count() -> None:
 
 
 @pytest.mark.ai_generated
-def test_dandi_remote_extractor_init(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> None:
-    """DandiRemoteS3LogAccessExtractor.__init__ sets expected attributes."""
+@pytest.mark.parametrize(
+    ("excluded_ips", "expected_awk_excluded_ips"),
+    [
+        pytest.param(None, "192.0.2.1", id="defaults_to_upstream_excluded_ips"),
+        pytest.param([], "", id="empty_overrides_upstream_excluded_ips"),
+        pytest.param(["192.0.2.3", "192.0.2.2"], "192.0.2.2 192.0.2.3", id="explicit_overrides_upstream_excluded_ips"),
+    ],
+)
+def test_dandi_remote_extractor_init(
+    tmp_path: pathlib.Path,
+    monkeypatch: pytest.MonkeyPatch,
+    excluded_ips: list[str] | None,
+    expected_awk_excluded_ips: str,
+) -> None:
+    """DandiRemoteS3LogAccessExtractor.__init__ sets expected attributes and passes the excluded IPs to awk."""
     from dandi_s3_log_extraction.extractors import DandiRemoteS3LogAccessExtractor
 
-    monkeypatch.setenv("IPS_TO_SKIP", "192.168.0.1|10.0.0.1")
+    monkeypatch.setattr(
+        "s3_log_extraction.extractors._remote_s3_log_access_extractor.get_excluded_ips",
+        lambda: frozenset({"192.0.2.1"}),
+    )
 
-    extractor = DandiRemoteS3LogAccessExtractor(cache_directory=tmp_path)
+    extractor = DandiRemoteS3LogAccessExtractor(cache_directory=tmp_path, excluded_ips=excluded_ips)
     assert extractor._relative_script_path.exists()
-    assert "IPS_TO_SKIP_REGEX" in extractor._awk_env
-    assert extractor._awk_env["IPS_TO_SKIP_REGEX"] == "192.168.0.1|10.0.0.1"
+    assert extractor._awk_env["EXCLUDED_IPS"] == expected_awk_excluded_ips
+    assert "IPS_TO_SKIP_REGEX" not in extractor._awk_env
     assert extractor.use_encryption is False
 
 
+@pytest.mark.ai_generated
+@pytest.mark.parametrize(
+    ("excluded_ips", "expected_ips"),
+    [
+        pytest.param([], ["192.0.2.1", "192.0.2.10"], id="nothing_excluded"),
+        pytest.param(["192.0.2.1"], ["192.0.2.10"], id="only_exact_address_excluded"),
+    ],
+)
+def test_dandi_extraction_script_drops_excluded_ips(
+    tmp_path: pathlib.Path, excluded_ips: list[str], expected_ips: list[str]
+) -> None:
+    """The DANDI awk script drops the requests of exactly the excluded addresses, and no neighboring one."""
+    from dandi_s3_log_extraction.extractors import DandiRemoteS3LogAccessExtractor
+
+    log_file_path = tmp_path / "2020-01-01-00-00-00-0123456789ABCDEF"
+    log_file_path.write_text(
+        "".join(
+            f"abc123 dandiarchive [01/Jan/2020:00:00:00 +0000] {ip} - REQ123 REST.GET.OBJECT blobs/abc/def "
+            f'"GET /blobs/abc/def HTTP/1.1" 200 - 10 10 10 10 "-" "TestAgent" -\n'
+            for ip in ("192.0.2.1", "192.0.2.10")
+        )
+    )
+    output_directory = tmp_path / "output"
+    output_directory.mkdir()
+    cache_directory = tmp_path / "cache"
+    cache_directory.mkdir()
+
+    extractor = DandiRemoteS3LogAccessExtractor(cache_directory=cache_directory, excluded_ips=excluded_ips)
+    extractor._run_extraction(file_path=log_file_path, extraction_directory=output_directory)
+
+    assert (output_directory / "blobs" / "abc" / "def" / "ips.txt").read_text().splitlines() == expected_ips
+
+
 # ─── generate_dandiset_summaries error cases ─────────────────────────────────