make airbyte-ci pass a parameter to the java connectors to the locati…

…on of the logs

airbyte-ci/connectors/pipelines/README.md

@@ -644,6 +644,7 @@ E.G.: running Poe tasks on the modified internal packages of the current branch:
 
 | Version | PR                                                         | Description                                                                                                                |
 | ------- | ---------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- |
+| 4.4.0   | [#35317](https://github.com/airbytehq/airbyte/pull/35317)  | Augment java connector reports to include full logs and junit test results                                                 |
 | 4.3.1   | [#35437](https://github.com/airbytehq/airbyte/pull/35437)  | Do not run QA checks on publish, just MetadataValidation.                                                             |
 | 4.3.0   | [#35438](https://github.com/airbytehq/airbyte/pull/35438)  | Optionally disable telemetry with environment variable.                                                                    |
 | 4.2.4   | [#35325](https://github.com/airbytehq/airbyte/pull/35325)  | Use `connectors_qa` for QA checks and remove redundant checks.                                                             |

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/reports.py

@@ -4,11 +4,14 @@
 from __future__ import annotations
 
 import json
+import os
 import webbrowser
 from dataclasses import dataclass
-from typing import TYPE_CHECKING
+from pathlib import Path
+from types import MappingProxyType
+from typing import TYPE_CHECKING, Optional
+from zipfile import ZIP_DEFLATED, ZipFile
 
-from anyio import Path
 from connector_ops.utils import console  # type: ignore
 from jinja2 import Environment, PackageLoader, select_autoescape
 from pipelines.consts import GCS_PUBLIC_DOMAIN
@@ -42,13 +45,19 @@ def report_output_prefix(self) -> str:
     def html_report_file_name(self) -> str:
         return self.filename + ".html"
 
+    def file_remote_storage_key(self, file_name: str) -> str:
+        return f"{self.report_output_prefix}/{file_name}"
+
     @property
     def html_report_remote_storage_key(self) -> str:
-        return f"{self.report_output_prefix}/{self.html_report_file_name}"
+        return self.file_remote_storage_key(self.html_report_file_name)
+
+    def file_url(self, file_name: str) -> str:
+        return f"{GCS_PUBLIC_DOMAIN}/{self.pipeline_context.ci_report_bucket}/{self.file_remote_storage_key(file_name)}"
 
     @property
     def html_report_url(self) -> str:
-        return f"{GCS_PUBLIC_DOMAIN}/{self.pipeline_context.ci_report_bucket}/{self.html_report_remote_storage_key}"
+        return self.file_url(self.html_report_file_name)
 
     def to_json(self) -> str:
         """Create a JSON representation of the connector test report.
@@ -91,7 +100,11 @@ async def to_html(self) -> str:
         template = env.get_template("test_report.html.j2")
         template.globals["StepStatus"] = StepStatus
         template.globals["format_duration"] = format_duration
-        local_icon_path = await Path(f"{self.pipeline_context.connector.code_directory}/icon.svg").resolve()
+        local_icon_path = Path(f"{self.pipeline_context.connector.code_directory}/icon.svg").resolve()
+        step_result_to_artifact_link = {}
+        for step_result in self.steps_results:
+            if test_artifacts_link := await self.upload_path(step_result.test_artifacts_path):
+                step_result_to_artifact_link[step_result.step.title] = test_artifacts_link
         template_context = {
             "connector_name": self.pipeline_context.connector.technical_name,
             "step_results": self.steps_results,
@@ -104,6 +117,8 @@ async def to_html(self) -> str:
             "git_revision": self.pipeline_context.git_revision,
             "commit_url": None,
             "icon_url": local_icon_path.as_uri(),
+            "report": self,
+            "step_result_to_artifact_link": MappingProxyType(step_result_to_artifact_link),
         }
 
         if self.pipeline_context.is_ci:
@@ -118,15 +133,14 @@ async def to_html(self) -> str:
 
     async def save(self) -> None:
         local_html_path = await self.save_local(self.html_report_file_name, await self.to_html())
-        absolute_path = await local_html_path.resolve()
+        absolute_path = local_html_path.resolve()
         if self.pipeline_context.enable_report_auto_open:
             self.pipeline_context.logger.info(f"HTML report saved locally: {absolute_path}")
             if self.pipeline_context.enable_report_auto_open:
                 self.pipeline_context.logger.info("Opening HTML report in browser.")
                 webbrowser.open(absolute_path.as_uri())
         if self.remote_storage_enabled:
             await self.save_remote(local_html_path, self.html_report_remote_storage_key, "text/html")
-            self.pipeline_context.logger.info(f"HTML report uploaded to {self.html_report_url}")
         await super().save()
 
     def print(self) -> None:
@@ -155,3 +169,31 @@ def print(self) -> None:
 
         main_panel = Panel(Group(*to_render), title=main_panel_title, subtitle=duration_subtitle)
         console.print(main_panel)
+
+    async def upload_path(self, path: Optional[Path]) -> Optional[str]:
+        if not path or not path.exists():
+            return None
+        if self.pipeline_context.is_local:
+            return str(path.resolve())
+
+        zip_file_path = Path(str(path) + ".zip")
+        with ZipFile(zip_file_path, mode="w") as zip_file:
+            # lifted from https://github.com/python/cpython/blob/3.12/Lib/zipfile/__init__.py#L2277C9-L2295C44
+            def add_to_zip(zf: ZipFile, path_to_zip: str, zippath: str) -> None:
+                if os.path.isfile(path_to_zip):
+                    zf.write(path_to_zip, zippath, ZIP_DEFLATED)
+                elif os.path.isdir(path_to_zip):
+                    if zippath:
+                        zf.write(path_to_zip, zippath)
+                    for nm in sorted(os.listdir(path_to_zip)):
+                        add_to_zip(zf, os.path.join(path_to_zip, nm), os.path.join(zippath, nm))
+
+            add_to_zip(zip_file, str(path), "")
+
+        if not self.remote_storage_enabled:
+            self.pipeline_context.logger.info(f"remote storage is disable. zip file is at {zip_file_path.resolve()}")
+            return str(zip_file_path.resolve())
+        else:
+            await self.save_remote(zip_file_path, self.file_remote_storage_key(zip_file_path.name), "application/zip")
+            self.pipeline_context.logger.info(f"zip file uploaded to {self.file_url(str(zip_file_path))}")
+            return self.file_url(zip_file_path.name)

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/java_connectors.py

@@ -38,7 +38,7 @@ class IntegrationTests(GradleTask):
     gradle_task_name = "integrationTestJava"
     mount_connector_secrets = True
     bind_to_docker_host = True
-    with_test_report = True
+    with_test_artifacts = True
 
     @property
     def default_params(self) -> STEP_PARAMS:
@@ -80,7 +80,7 @@ class UnitTests(GradleTask):
     title = "Java Connector Unit Tests"
     gradle_task_name = "test"
     bind_to_docker_host = True
-    with_test_report = True
+    with_test_artifacts = True
 
 
 def _create_integration_step_args_factory(context: ConnectorContext) -> Callable:

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/templates/test_report.html.j2

@@ -82,7 +82,7 @@
       transition: max-height .25s ease-in-out;
     }
     .toggle:checked + .lbl-toggle + .collapsible-content {
-      max-height: 100vh;
+      max-height: 70vh;
     }
     .toggle:checked + .lbl-toggle {
       border-bottom-right-radius: 0;
@@ -110,6 +110,14 @@
     }
   </style>
 </head>
+<script>
+function copyToClipBoard(htmlElement) {
+  var copyText = htmlElement.parentNode.parentNode.getElementsByTagName('pre')[0].innerText;
+  htmlElement.parentNode.parentNode.getElementsByTagName('pre')[0].setSelectionRange
+
+  navigator.clipboard.writeText(copyText);
+}
+</script>
 <body>
   <h1><img src="{{ icon_url }}" width="40" height="40"> {{ connector_name }} test report</h1>
   <ul>
@@ -159,19 +167,18 @@
       <label for="{{ step_result.step.title }}" class="lbl-toggle">{{ step_result.step.title }} | {{ format_duration(step_result.step.run_duration) }}</label>
     {% endif %}
     <div class="collapsible-content">
+      {% if step_result_to_artifact_link[step_result.step.title] %}
+        <div><a href="{{ step_result_to_artifact_link[step_result.step.title] }}">Test Artifacts</a></div>
+      {% endif %}
       <div class="content-inner">
         {% if step_result.stdout %}
-        <span class="std">Standard output:</span>
+        <span class="std">Standard output(<button onclick="copyToClipBoard(this)">Copy to clipboard</button>):</span>
         <pre>{{ step_result.stdout }}</pre>
         {% endif %}
         {% if step_result.stderr %}
-        <span class="std">Standard error:</span>
+        <span class="std">Standard error(<button onclick="copyToClipBoard(this)">Copy to clipboard</button>):</span>
         <pre>{{ step_result.stderr }}</pre>
         {% endif %}
-        {% if step_result.report %}
-        <span class="std">Report:</span>
-        <pre lang="xml">{{ step_result.report }}</pre>
-        {% endif %}
       </div>
     </div>
   </div>

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/steps/gradle.py

@@ -1,13 +1,12 @@
 #
 # Copyright (c) 2023 Airbyte, Inc., all rights reserved.
 #
-import html
-import re
 from abc import ABC
-from typing import Any, ClassVar, List, Optional, Tuple
+from datetime import datetime
+from pathlib import Path
+from typing import Any, ClassVar, List, Optional, Tuple, cast
 
 import pipelines.dagger.actions.system.docker
-import xmltodict
 from dagger import CacheSharingMode, CacheVolume, Container, QueryError
 from pipelines.airbyte_ci.connectors.context import ConnectorContext
 from pipelines.consts import AMAZONCORRETTO_IMAGE
@@ -36,9 +35,18 @@ class GradleTask(Step, ABC):
     gradle_task_name: ClassVar[str]
     bind_to_docker_host: ClassVar[bool] = False
     mount_connector_secrets: ClassVar[bool] = False
-    with_test_report: ClassVar[bool] = False
+    with_test_artifacts: ClassVar[bool] = False
     accept_extra_params = True
 
+    @property
+    def airbyte_logs_subdir(self) -> str:
+        return datetime.fromtimestamp(cast(float, self.context.pipeline_start_timestamp)).isoformat() + "-" + self.gradle_task_name
+
+    @property
+    def test_artifacts_path(self) -> Path:
+        test_artifacts_path = f"{self.context.connector.code_directory}/build/test-artifacts/{self.airbyte_logs_subdir}"
+        return Path(test_artifacts_path)
+
     @property
     def gradle_task_options(self) -> Tuple[str, ...]:
         return self.STATIC_GRADLE_OPTIONS + (f"-Ds3BuildCachePrefix={self.context.connector.technical_name}",)
@@ -103,6 +111,8 @@ async def _run(self, *args: Any, **kwargs: Any) -> StepResult:
             .with_env_variable("GRADLE_HOME", self.GRADLE_HOME_PATH)
             # Same for GRADLE_USER_HOME.
             .with_env_variable("GRADLE_USER_HOME", self.GRADLE_HOME_PATH)
+            # Set the AIRBYTE_LOG_SUBDIR for log4j
+            .with_env_variable("AIRBYTE_LOG_SUBDIR", self.airbyte_logs_subdir)
             # Install a bunch of packages as early as possible.
             .with_exec(
                 sh_dash_c(
@@ -193,6 +203,8 @@ async def _run(self, *args: Any, **kwargs: Any) -> StepResult:
         connector_gradle_task = f":airbyte-integrations:connectors:{self.context.connector.technical_name}:{self.gradle_task_name}"
         gradle_command = self._get_gradle_command(connector_gradle_task, task_options=self.params_as_cli_options)
         gradle_container = gradle_container.with_(never_fail_exec([gradle_command]))
+        await self._collect_logs(gradle_container)
+        await self._collect_test_report(gradle_container)
         return await self.get_step_result(gradle_container)
 
     async def get_step_result(self, container: Container) -> StepResult:
@@ -203,76 +215,49 @@ async def get_step_result(self, container: Container) -> StepResult:
             status=step_result.status,
             stdout=step_result.stdout,
             stderr=step_result.stderr,
-            report=await self._collect_test_report(container),
             output_artifact=step_result.output_artifact,
+            test_artifacts_path=self.test_artifacts_path,
         )
 
-    async def _collect_test_report(self, gradle_container: Container) -> Optional[str]:
-        if not self.with_test_report:
+    async def _collect_logs(self, gradle_container: Container) -> None:
+        """
+        Exports the java docs from the container into the host filesystem.
+        The docs in the container are expected to be in build/test-logs, and will end up test-artifact directory by default
+        One can change the destination directory by setting the test_artifacts_path
+        """
+        if not self.with_test_artifacts:
+            return None
+        logs_dir_path = f"{self.context.connector.code_directory}/build/test-logs/{self.airbyte_logs_subdir}"
+        try:
+            container_logs_dir = await gradle_container.directory(logs_dir_path)
+            # the gradle task didn't create any logs.
+            if not container_logs_dir:
+                return None
+
+            self.test_artifacts_path.mkdir(parents=True, exist_ok=True)
+            if not await container_logs_dir.export(str(self.test_artifacts_path)):
+                self.context.logger.error("Error when trying to export log files from container")
+        except QueryError as e:
+            self.context.logger.error(str(e))
+            self.context.logger.warn(f"Failed to retrieve junit test results from {logs_dir_path} gradle container.")
+            return None
+
+    async def _collect_test_report(self, gradle_container: Container) -> None:
+        """
+        Exports the junit test reports from the container into the host filesystem.
+        The docs in the container are expected to be in build/test-results, and will end up test-artifact directory by default
+        Only the XML files generated by junit are downloaded into the host filesystem
+        One can change the destination directory by setting the test_artifacts_path
+        """
+        if not self.with_test_artifacts:
             return None
 
         junit_xml_path = f"{self.context.connector.code_directory}/build/test-results/{self.gradle_task_name}"
-        testsuites = []
         try:
             junit_xml_dir = await gradle_container.directory(junit_xml_path)
             for file_name in await junit_xml_dir.entries():
                 if file_name.endswith(".xml"):
-                    junit_xml = await junit_xml_dir.file(file_name).contents()
-                    # This will be embedded in the HTML report in a <pre lang="xml"> block.
-                    # The java logging backend will have already taken care of masking any secrets.
-                    # Nothing to do in that regard.
-                    try:
-                        if testsuite := xmltodict.parse(junit_xml):
-                            testsuites.append(testsuite)
-                    except Exception as e:
-                        self.context.logger.error(str(e))
-                        self.context.logger.warn(f"Failed to parse junit xml file {file_name}.")
+                    await junit_xml_dir.file(file_name).export(str(self.test_artifacts_path), allow_parent_dir_path=True)
         except QueryError as e:
             self.context.logger.error(str(e))
-            self.context.logger.warn(f"Failed to retrieve junit test results from {junit_xml_path} gradle container.")
             return None
-        return render_junit_xml(testsuites)
-
-
-MAYBE_STARTS_WITH_XML_TAG = re.compile("^ *<")
-ESCAPED_ANSI_COLOR_PATTERN = re.compile(r"\?\[0?m|\?\[[34][0-9]m")
-
-
-def render_junit_xml(testsuites: List[Any]) -> str:
-    """Renders the JUnit XML report as something readable in the HTML test report."""
-    # Transform the dict contents.
-    indent = "  "
-    for testsuite in testsuites:
-        testsuite = testsuite.get("testsuite")
-        massage_system_out_and_err(testsuite, indent, 4)
-        if testcases := testsuite.get("testcase"):
-            if not isinstance(testcases, list):
-                testcases = [testcases]
-            for testcase in testcases:
-                massage_system_out_and_err(testcase, indent, 5)
-    # Transform back to XML string.
-    # Try to respect the JUnit XML test result schema.
-    root = {"testsuites": {"testsuite": testsuites}}
-    xml = xmltodict.unparse(root, pretty=True, short_empty_elements=True, indent=indent)
-    # Escape < and > and so forth to make them render properly, but not in the log messages.
-    # These lines will already have been escaped by xmltodict.unparse.
-    lines = xml.splitlines()
-    for idx, line in enumerate(lines):
-        if MAYBE_STARTS_WITH_XML_TAG.match(line):
-            lines[idx] = html.escape(line)
-    return "\n".join(lines)
-
-
-def massage_system_out_and_err(d: dict, indent: str, indent_levels: int) -> None:
-    """Makes the system-out and system-err text prettier."""
-    if d:
-        for key in ["system-out", "system-err"]:
-            if s := d.get(key):
-                lines = s.splitlines()
-                s = ""
-                for line in lines:
-                    stripped = line.strip()
-                    if stripped:
-                        s += "\n" + indent * indent_levels + ESCAPED_ANSI_COLOR_PATTERN.sub("", line.strip())
-                s = s + "\n" + indent * (indent_levels - 1) if s else None
-                d[key] = s

airbyte-ci/connectors/pipelines/pipelines/models/reports.py

@@ -10,10 +10,9 @@
 import typing
 from dataclasses import dataclass, field
 from datetime import datetime, timedelta
+from pathlib import Path
 from typing import List
 
-import anyio
-from anyio import Path
 from connector_ops.utils import console  # type: ignore
 from pipelines.consts import GCS_PUBLIC_DOMAIN, LOCAL_REPORTS_PATH_ROOT
 from pipelines.dagger.actions import remote_storage
@@ -86,16 +85,16 @@ def remote_storage_enabled(self) -> bool:
 
     async def save_local(self, filename: str, content: str) -> Path:
         """Save the report files locally."""
-        local_path = anyio.Path(f"{LOCAL_REPORTS_PATH_ROOT}/{self.report_output_prefix}/{filename}")
-        await local_path.parents[0].mkdir(parents=True, exist_ok=True)
-        await local_path.write_text(content)
+        local_path = Path(f"{LOCAL_REPORTS_PATH_ROOT}/{self.report_output_prefix}/{filename}")
+        local_path.parents[0].mkdir(parents=True, exist_ok=True)
+        local_path.write_text(content)
         return local_path
 
     async def save_remote(self, local_path: Path, remote_key: str, content_type: str) -> int:
         assert self.pipeline_context.ci_report_bucket is not None, "The ci_report_bucket must be set to save reports."
-
         gcs_cp_flags = None if content_type is None else [f"--content-type={content_type}"]
         local_file = self.pipeline_context.dagger_client.host().directory(".", include=[str(local_path)]).file(str(local_path))
+
         report_upload_exit_code, _, _ = await remote_storage.upload_to_gcs(
             dagger_client=self.pipeline_context.dagger_client,
             file_to_upload=local_file,
@@ -116,7 +115,7 @@ async def save(self) -> None:
         """Save the report files."""
 
         local_json_path = await self.save_local(self.json_report_file_name, self.to_json())
-        absolute_path = await local_json_path.absolute()
+        absolute_path = local_json_path.absolute()
         self.pipeline_context.logger.info(f"Report saved locally at {absolute_path}")
         if self.remote_storage_enabled:
             await self.save_remote(local_json_path, self.json_report_remote_storage_key, "application/json")

airbyte-ci/connectors/pipelines/pipelines/models/steps.py

@@ -74,6 +74,7 @@ class Result:
     report: Optional[str] = None
     exc_info: Optional[Exception] = None
     output_artifact: Any = None
+    test_artifacts_path: Optional[Path] = None
 
     @property
     def success(self) -> bool: