feat(connector): record connector endpoint on LogContext#7317
feat(connector): record connector endpoint on LogContext#7317pingsutw wants to merge 14 commits intoflyteorg:mainfrom
Conversation
Adds a ConnectorLogContext { string endpoint } sub-message to flyteidl2.core.LogContext.
The connector plugin populates it from the resolved deployment endpoint when emitting
PhaseInfo, so the dataplane dataproxy can stream logs straight from the connector via
AsyncConnectorService.GetTaskLogs instead of trying to read pod logs.
Resource metadata still lives in plugin state and is fetched at log-stream time —
this commit only handles the routing part.
Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds connector endpoint metadata into Flyte’s LogContext so dataplane log streaming can route directly to connector-owned logs (for connector tasks that don’t have pods).
Changes:
- Extend
flyteidl2.core.LogContextwithConnectorLogContext { string endpoint }. - Regenerate Go protobuf + validation code to include the new message/field.
- Populate
TaskInfo.LogContext.connector.endpointin the connector pluginStatus()path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gen/go/gateway/flyteidl2/core/execution.swagger.json | Regenerated OpenAPI definitions after proto update. |
| gen/go/flyteidl2/core/execution.pb.validate.go | Adds validation plumbing for LogContext.connector and new ConnectorLogContext. |
| gen/go/flyteidl2/core/execution.pb.go | Adds ConnectorLogContext type and LogContext.connector field in Go bindings. |
| flyteplugins/go/tasks/plugins/webapi/connector/plugin.go | Sets TaskInfo.LogContext.connector.endpoint based on resolved connector deployment. |
| flyteidl2/core/execution.proto | Defines ConnectorLogContext and adds it to LogContext. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… bindings - Status() no longer calls ResourceMeta() / getFinalConnector() — the connector endpoint is resolved in Get() and stashed on ResourceWrapper.ConnectorEndpoint, so existing tests with mocked StatusContext don't have to stub ResourceMeta. - Regenerate Python/TS/Rust protobuf bindings for the new ConnectorLogContext message so downstream consumers stay in sync. Signed-off-by: Kevin Su <[email protected]>
Create() now returns a populated ResourceWrapper carrying ConnectorEndpoint, so the very first PhaseInfo emitted after task creation already records the connector endpoint on its LogContext. Previously only later Status() calls (after Get() re-resolved the connector) carried the endpoint. Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Kevin Su <[email protected]>
Lets the UI forward the connector deployment's gRPC endpoint (sourced from ActionAttempt.log_context.connector.endpoint) on TailLogs so the dataproxy can stream from AsyncConnectorService.GetTaskLogs for connector tasks. Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Helps confirm in production logs that the connector plugin is populating the endpoint that the dataproxy needs to stream logs. Signed-off-by: Kevin Su <[email protected]>
Quick instrumentation to confirm the run service sees the connector endpoint recorded by the connector plugin on each ActionEvent. Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 24 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 24 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Plumbs the connector endpoint through to the log-streaming path so the dataplane dataproxy can proxy log requests directly to the connector instead of trying to tail pod logs that don't exist for connector-served actions.
Proto changes
flyteidl2/core/execution.proto— addsConnectorLogContext { string endpoint }and a newLogContext.connectorfield. The connector plugin records the resolved deployment's gRPC endpoint here, and the dataproxy reads it at log-stream time.flyteidl2/connector/connector.proto— adds structuredrepeated flyteidl2.logs.dataplane.LogLine lines = 2toGetTaskLogsResponseBody(carries timestamp + originator). Marks the existingrepeated string results = 1deprecated. Old connectors that emitresultskeep working; new connectors emitlines.flyteidl2/dataproxy/dataproxy_service.proto— addsstring connector_endpoint = 4onTailLogsRequestso the UI can pass the endpoint sourced fromActionAttempt.LogContext.connector.endpoint(avoids the dataproxy needing to re-resolve it from a possibly-stale event).Connector plugin changes (
flyteplugins/go/tasks/plugins/webapi/connector/plugin.go)ResourceWrappercarries the resolved connector endpoint.Getstamps the endpoint from the resolvedConnectorDeploymentonto each returnedResourceWrapper.StatuswritestaskInfo.LogContext = { Connector: { Endpoint: ... } }whenever the endpoint is set, so the action event carries it forward.Misc
runs/service/run_service.go— drops a strayconnect.NotFoundcheck that returned early when onlyLogContextwas nil but the cluster name was still available; callers (dataproxy) now decide whether the missing LogContext is fatal.Why
Connector tasks have no Kubernetes pod, so the dataplane dataproxy can't tail logs the usual way. Recording the connector's gRPC endpoint on
LogContextand passing it throughTailLogsRequestlets the dataproxy proxy log requests straight to the connector'sAsyncConnectorService.GetTaskLogswithout a separate registry, ConfigMap watcher, or stale-event dance. The structuredlinesfield lets connectors emit timestamps and originator metadata that survive end-to-end (UI included), instead of being stamped with observe-time on the dataproxy.Out of scope
GetTaskLogs(resource_meta,task_category) still lives in plugin state and is fetched at log-stream time.