flyteorg / flyteorg/flyte

Delegate RunLogsService.TailLogs to DataProxyService to remove duplicate implementation

Open
#7,252 2 comments 0 reactions 1 assignee Claimed by @Carina-TzuHsuan View on GitHub
flyte2
Dominant language
Go
Stars
7.5k
Forks
886
Avg merge
1d 12h
Merged PRs (30d)
120

Description

## What

`RunLogsService.TailLogs` (`runs/service/run_logs_service.go:44`) and `dataproxy.Service.TailLogs` (`dataproxy/service/dataproxy_service.go:503`) are essentially doing the same thing:

1. Resolve the `LogContext` for the given action attempt.
2. Hand it to a `LogStreamer` which streams pod logs back to the client.

The canonical home for this is now the DataProxy service. The `RunLogsService.TailLogs` RPC is only kept for backward compatibility with existing clients.

## Why

Today both services:

- Depend on a `LogStreamer` implementation (`runs/service/k8s_log_streamer.go` and `dataproxy/logs/k8s_log_streamer.go` — two copies).
- Implement their own concurrency limit / context resolution code path.
- Must be kept in sync whenever we evolve log streaming behavior (filtering, pagination, new backends, auth, etc.).

This is duplicate surface area with no good reason now that DataProxy owns logs.

## Proposed change

Refactor `RunLogsService.TailLogs` into a thin shim that forwards to `DataProxyServiceClient.TailLogs`:

- Inject a `dataproxyconnect.DataProxyServiceClient` into `RunLogsService` instead of (or in addition to) the `LogStreamer`.
- In `TailLogs`:
1. Validate `action_id` / acquire the concurrency semaphore as today.
2. Open a client stream to `DataProxyService.TailLogs` with the same `action_id` + `attempt`.
3. Forward each `dataproxy.TailLogsResponse` chunk to the outbound `workflow.TailLogsResponse` stream (converting the proto message — the schemas need to be checked for 1:1 field compatibility).
- Delete `runs/service/k8s_log_streamer.go` and its test once the delegation is in place; DataProxy's `logs/k8s_log_streamer.go` becomes the single implementation.
- Keep the `runs` RPC registered so existing clients keep working.

## Open questions

- Response proto shape: confirm `workflow.TailLogsResponse` and `dataproxy.TailLogsResponse` are structurally compatible, or define a small converter.
- Wiring: the run service will need to be given a DataProxy client at startup (likely the in-process one in the all-in-one binary).
- Concurrency limit: decide whether the `defaultMaxConcurrentStreams` semaphore stays on the run-service shim, moves entirely to DataProxy, or both.

## References

- `runs/service/run_logs_service.go:44-61` — current run-service TailLogs
- `dataproxy/service/dataproxy_service.go:503-520` — DataProxy TailLogs (target delegate)
- `runs/service/k8s_log_streamer.go` / `dataproxy/logs/k8s_log_streamer.go` — duplicated streamer impls

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.