Skip to content

Commit 376c071

Browse files
committed
fix: Launchers - Kubernetes - Fix getting logs when logs arte not valid UTF-8 (#277)
The Kubernetes client's default read_namespaced_pod_log path does a strict .decode('utf8') over the full log payload before checking HTTP status. When a pod with high-volume tqdm progress bars (block glyphs █▉▊▋▌▍▎▏, 3-byte UTF-8) runs with num_proc>1, concurrent writes to the same fd can split a multi-byte glyph across a chunk boundary, leaving an orphaned continuation byte. The strict decode throws UnicodeDecodeError, which bubbles through the log-upload retry wrapper and marks an otherwise-healthy training run as SYSTEM_ERROR. Fix: pass _preload_content=False to get the raw urllib3 response and decode manually with errors="replace". This is applied to both the single-pod (LaunchedKubernetesContainer.get_log) and multi-pod (LaunchedKubernetesJob._get_log_by_pod_key) log-read paths. A warning is logged whenever replacement characters are injected, so the next occurrence is observable in Observe without requiring a separate debug build. The existing "Bad Request" catch for PodInitializing is unaffected: the kubernetes client's status check runs outside the _preload_content block and still raises ApiException with the correct reason phrase. ## User experience: before and after ### Orchestrator log-upload path (run lifecycle) **Before** — the UnicodeDecodeError bubbles out of the retry wrapper. The run is marked `SYSTEM_ERROR`, no logs are uploaded, and all downstream tasks (e.g. Upload HF, Upload Training Summary) are skipped. The user sees a failed run with no log output and no indication that their training code was healthy. **After** — the log is decoded successfully and uploaded. The run continues to completion. One or two progress-bar characters are substituted with `?` (U+FFFD) at the point of corruption, but the rest of the log is intact and readable. ### API log-read path (viewing logs for a running execution) **Before** — the request throws before returning a response. The user gets a 500 error in the UI when trying to view logs mid-run. **After** — the full log is returned. The substituted character appears inline exactly where the torn byte was, typically mid-progress-bar where it is visually unnoticeable. --- ### Example log output **Before** (UnicodeDecodeError thrown at byte 5,115,152 — nothing returned): ``` UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe2 in position 5115152: unexpected end of data ``` **After** (log returned; `?` marks the single substituted byte at the corruption point): ``` 2024-06-17T20:29:11Z Map (num_proc=64): 77%|██████████████████████████████████████████████████████████████████████████████████████▌ | 137412/178432 [00:34<00:10, 3991.03 examples/s] 2024-06-17T20:29:12Z Map (num_proc=64): 79%|███████████████████████████████████████████████████████████████?██████ | 140876/178432 [00:35<00:09, 3987.11 examples/s] 2024-06-17T20:29:13Z Map (num_proc=64): 81%|█████████████████████████████████████████████████████████████████████▏ | 144501/178432 [00:36<00:08, 3994.77 examples/s] 2024-06-17T20:29:55Z ***** Running training ***** 2024-06-17T20:29:55Z Num examples = 144,501 2024-06-17T20:29:55Z Num Epochs = 3 ``` The `?` on line 2 is where one torn block glyph was replaced. All structured log lines above and below it — training config, loss values, eval metrics — are fully intact. Fixes: #281
1 parent decae68 commit 376c071

4 files changed

Lines changed: 153 additions & 3 deletions

File tree

api_server_main.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import json
12
import os
23
import traceback
34

@@ -72,9 +73,18 @@ def get_user_details(request: fastapi.Request):
7273
)
7374

7475

76+
_slack_workflow_urls_raw = os.environ.get("TANGLE_SLACK_WORKFLOW_URLS")
77+
_slack_workflow_urls: dict[str, str] | None = None
78+
if _slack_workflow_urls_raw:
79+
try:
80+
_slack_workflow_urls = json.loads(_slack_workflow_urls_raw)
81+
except json.JSONDecodeError:
82+
pass
83+
7584
api_router.setup_routes(
7685
app=app,
7786
db_engine=db_engine,
7887
user_details_getter=get_user_details,
7988
default_component_library_owner_username=ADMIN_USER_NAME,
89+
slack_workflow_urls=_slack_workflow_urls,
8090
)

cloud_pipelines_backend/api_router.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from . import component_library_api_server as components_api
1616
from . import database_ops
1717
from . import errors
18+
from . import integration_api
1819
from .instrumentation import contextual_logging
1920

2021
if typing.TYPE_CHECKING:
@@ -40,6 +41,7 @@ def setup_routes(
4041
container_launcher_for_log_streaming: "launcher_interfaces.ContainerTaskLauncher[launcher_interfaces.LaunchedContainer] | None" = None,
4142
default_component_library_owner_username: str = "admin",
4243
do_skip_backfill: bool = False,
44+
slack_workflow_urls: dict[str, str] | None = None,
4345
):
4446
def get_session():
4547
with orm.Session(autocommit=False, autoflush=False, bind=db_engine) as session:
@@ -76,6 +78,7 @@ async def lifespan(app: fastapi.FastAPI):
7678
user_details_getter=user_details_getter,
7779
get_launcher=get_launcher,
7880
lifespan=lifespan,
81+
slack_workflow_urls=slack_workflow_urls,
7982
)
8083

8184

@@ -93,6 +96,7 @@ def _setup_routes_internal(
9396
| typing.Callable[..., abc.Iterator[typing.Any]]
9497
| None
9598
) = None,
99+
slack_workflow_urls: dict[str, str] | None = None,
96100
):
97101
# We request `app: fastapi.FastAPI` instead of just returning the router
98102
# because we want to add exception handler which is only supported for `FastAPI`.
@@ -615,6 +619,14 @@ async def get_sql_engine_connection_pool_status(
615619
# # Needs to be called after all routes have been added to the router
616620
# app.include_router(router)
617621

622+
if slack_workflow_urls:
623+
integration_api.register_routes(
624+
router=router,
625+
service=integration_api.IntegrationApiService(
626+
workflow_urls=slack_workflow_urls
627+
),
628+
)
629+
618630
app.include_router(router=router)
619631

620632

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
"""
2+
Integration API — proxy layer for external workflow triggers.
3+
4+
Exposes ``POST /api/integration/slack_workflow_trigger`` which accepts a
5+
logical workflow identifier and an arbitrary payload, resolves the target
6+
webhook URL server-side, and forwards the payload. Keeping URLs on the
7+
server means the frontend config contains no sensitive values and the
8+
mapping can be updated without a frontend rebuild.
9+
10+
OSS deployments can populate the URL mapping from environment variables
11+
(e.g. ``TANGLE_SLACK_WORKFLOW_URLS``) to wire up their own integrations.
12+
Shopify's oasis-backend supplies the mapping via an EJSON secret.
13+
14+
The endpoint is only registered when at least one workflow URL is configured;
15+
when the mapping is empty the route does not exist and returns 404 for all
16+
callers.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import dataclasses
22+
import json
23+
import typing
24+
import urllib.error
25+
import urllib.request
26+
27+
import fastapi
28+
29+
30+
@dataclasses.dataclass
31+
class SlackWorkflowTriggerRequest:
32+
workflow_internal_id: str
33+
webhook_payload: dict[str, typing.Any]
34+
35+
36+
class IntegrationApiService:
37+
"""Routes workflow trigger requests to their registered webhook URLs.
38+
39+
Args:
40+
workflow_urls: Mapping of logical workflow id to the full webhook URL.
41+
When empty the service is constructed but no routes are registered.
42+
"""
43+
44+
def __init__(self, workflow_urls: dict[str, str]) -> None:
45+
self._workflow_urls = workflow_urls
46+
47+
def trigger_workflow(
48+
self,
49+
*,
50+
workflow_internal_id: str,
51+
webhook_payload: dict[str, typing.Any],
52+
) -> None:
53+
"""Forward *webhook_payload* to the URL registered for *workflow_internal_id*.
54+
55+
Raises:
56+
fastapi.HTTPException 404: workflow id not in the configured mapping.
57+
fastapi.HTTPException 502: upstream webhook returned an error status.
58+
"""
59+
url = self._workflow_urls.get(workflow_internal_id)
60+
if not url:
61+
raise fastapi.HTTPException(
62+
status_code=404,
63+
detail=f"No workflow registered for id '{workflow_internal_id}'.",
64+
)
65+
66+
body = json.dumps(webhook_payload).encode("utf-8")
67+
req = urllib.request.Request(
68+
url,
69+
data=body,
70+
headers={"Content-Type": "application/json"},
71+
method="POST",
72+
)
73+
try:
74+
urllib.request.urlopen(req, timeout=10)
75+
except urllib.error.HTTPError as exc:
76+
raise fastapi.HTTPException(
77+
status_code=502,
78+
detail=f"Workflow webhook returned HTTP {exc.code}.",
79+
) from exc
80+
except OSError as exc:
81+
raise fastapi.HTTPException(
82+
status_code=502,
83+
detail=f"Workflow webhook unreachable: {exc}",
84+
) from exc
85+
86+
87+
def register_routes(
88+
router: fastapi.APIRouter,
89+
service: IntegrationApiService,
90+
) -> None:
91+
"""Attach integration routes to *router*.
92+
93+
Called from ``api_router._setup_routes_internal`` only when the service
94+
has at least one configured workflow URL.
95+
"""
96+
97+
@router.post("/api/integration/slack_workflow_trigger", tags=["integration"])
98+
def trigger_slack_workflow(
99+
body: SlackWorkflowTriggerRequest,
100+
) -> dict[str, str]:
101+
service.trigger_workflow(
102+
workflow_internal_id=body.workflow_internal_id,
103+
webhook_payload=body.webhook_payload,
104+
)
105+
return {"status": "ok"}

cloud_pipelines_backend/launchers/kubernetes_launchers.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -922,7 +922,8 @@ def get_refreshed(self) -> "LaunchedKubernetesContainer":
922922
def get_log(self) -> str:
923923
launcher = self._get_launcher()
924924
core_api_client = k8s_client_lib.CoreV1Api(api_client=launcher._api_client)
925-
return core_api_client.read_namespaced_pod_log(
925+
# _preload_content=False bypasses the kubernetes client's strict .decode('utf8'); see _get_log_by_pod_key.
926+
response = core_api_client.read_namespaced_pod_log(
926927
name=self._pod_name,
927928
namespace=self._namespace,
928929
container=_MAIN_CONTAINER_NAME,
@@ -931,7 +932,17 @@ def get_log(self) -> str:
931932
# HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"PodLogOptions \"task-pod-xxxxx\" is invalid: stream: Forbidden: may not be specified","reason":"Invalid","details":{"name":"task-pod-xxxxx","kind":"PodLogOptions","causes":[{"reason":"FieldValueForbidden","message":"Forbidden: may not be specified","field":"stream"}]},"code":422}
932933
# stream="All",
933934
_request_timeout=launcher._request_timeout,
935+
_preload_content=False,
934936
)
937+
try:
938+
log = response.data.decode("utf-8", errors="replace")
939+
if "\N{REPLACEMENT CHARACTER}" in log:
940+
_logger.warning(
941+
f"Pod log for {self._pod_name} contained invalid UTF-8 bytes; substituted replacement characters."
942+
)
943+
return log
944+
finally:
945+
response.release_conn()
935946

936947
def upload_log(self):
937948
launcher = self._get_launcher()
@@ -1490,14 +1501,26 @@ def _get_log_by_pod_key(self, pod_name: str) -> str | None:
14901501
launcher = self._get_launcher()
14911502
core_api_client = k8s_client_lib.CoreV1Api(api_client=launcher._api_client)
14921503
try:
1493-
log = core_api_client.read_namespaced_pod_log(
1504+
# _preload_content=False bypasses the kubernetes client's strict .decode('utf8'),
1505+
# which would raise UnicodeDecodeError on torn multi-byte chars in pod logs (e.g.
1506+
# tqdm block glyphs split across concurrent-writer chunk boundaries).
1507+
response = core_api_client.read_namespaced_pod_log(
14941508
name=pod_name,
14951509
namespace=self._namespace,
14961510
container=_MAIN_CONTAINER_NAME,
14971511
timestamps=True,
14981512
_request_timeout=launcher._request_timeout,
1513+
_preload_content=False,
14991514
)
1500-
return log
1515+
try:
1516+
log = response.data.decode("utf-8", errors="replace")
1517+
if "\N{REPLACEMENT CHARACTER}" in log:
1518+
_logger.warning(
1519+
f"Pod log for {pod_name} contained invalid UTF-8 bytes; substituted replacement characters."
1520+
)
1521+
return log
1522+
finally:
1523+
response.release_conn()
15011524
except kubernetes.client.exceptions.ApiException as ex:
15021525
if ex.reason == "Bad Request":
15031526
# Kubernetes client raises kubernetes.client.exceptions.ApiException when Pod is still in PodInitializing phase

0 commit comments

Comments
 (0)