CodeBuild event streaming#1809
Conversation
📝 WalkthroughWalkthroughAdds AWS CodeBuild event parsing and dispatch into the BYOC AWS log streaming pipeline, introduces a CodebuildEvent type and Event interface, updates stream constructors/subscriptions and ByocAws to register/dispatch CodeBuild handlers, and updates README sample links. Changes
Sequence DiagramsequenceDiagram
participant LogStream as CloudWatch Log Stream
participant Parser as byocServerStream
participant CBHandler as CodebuildEventHandler
participant Subscriber as byocSubscribeServerStream
participant Client as CLI Client
LogStream->>Parser: deliver log entries
activate Parser
Parser->>Parser: parse entries (ECS / Firelens / CodeBuild)
Parser->>Parser: ParseCodebuildEvent(entry)
alt CodeBuild handler registered
Parser->>CBHandler: HandleCodebuildEvent(evt)
activate CBHandler
CBHandler->>Subscriber: build SubscribeResponse
Subscriber->>Client: stream SubscribeResponse
deactivate CBHandler
else No handler
Parser->>Subscriber: skip CodeBuild dispatch
Subscriber->>Client: continue other events
end
deactivate Parser
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" Comment |
47e8bf6 to
9e00e22
Compare
9e00e22 to
6f0e03c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/cli/client/byoc/aws/stream.go (1)
102-113:⚠️ Potential issue | 🟠 MajorCodeBuild service filtering currently drops all build logs when
servicesis set.
response.Serviceremains<service>-image, while CodeBuild entries use the trimmed<service>. The pre-filter checks<service>-imageand the per-entry filter checks<service>, so any non-empty services filter will exclude CodeBuild logs unless the client supplies both names.🛠️ Suggested fix (use a single canonical service name for filtering)
// Client-side filtering on etag and service (if provided) + serviceForFilter := response.GetService() + if parseCodeBuildRecords { + serviceForFilter = strings.TrimSuffix(serviceForFilter, "-image") + } if response.Etag != "" && bs.etag != "" && bs.etag != response.Etag { return nil // TODO: filter these out using the AWS StartLiveTail API } - if len(bs.services) > 0 && !slices.Contains(bs.services, response.GetService()) { + if len(bs.services) > 0 && !slices.Contains(bs.services, serviceForFilter) { return nil // TODO: filter these out using the AWS StartLiveTail API } @@ } else if parseCodeBuildRecords { - entry.Service = strings.TrimSuffix(response.Service, "-image") + entry.Service = serviceForFilter entry.Etag = response.Etag entry.Host = response.Host evt := codebuild.ParseCodebuildEvent(entry)Also applies to: 144-150, 184-199
🤖 Fix all issues with AI agents
In `@src/pkg/cli/client/byoc/aws/subscribe.go`:
- Around line 22-31: HandleCodebuildEvent on byocSubscribeServerStream currently
forwards every CodeBuild event to s.ch; apply the same etag/service filtering
used for ECS events by early-returning when the event doesn't match the
subscriber's filters (e.g., check s.etag and s.service or call an existing
s.matchesFilter(evt) before building resp). Only construct and send
defangv1.SubscribeResponse if the event passes the filter, otherwise return;
preserve the select on s.ch and s.done for sending when matched.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/cli/client/byoc/aws/stream.go (1)
94-199:⚠️ Potential issue | 🟠 MajorCodeBuild events get dropped when services filtering is enabled.
response.Service/entry.Serviceare set to<service>-image, so the Line 145 and Line 197 filters reject typical<service>names. Normalize to the trimmed service for filtering (without changing the emitted service if you still want-imagein logs).🛠️ Suggested fix
- if len(bs.services) > 0 && !slices.Contains(bs.services, response.GetService()) { + filterService := response.GetService() + if parseCodeBuildRecords { + filterService = strings.TrimSuffix(filterService, "-image") + } + if len(bs.services) > 0 && !slices.Contains(bs.services, filterService) { return nil // TODO: filter these out using the AWS StartLiveTail API } ... - if entry.Service != "" && len(bs.services) > 0 && !slices.Contains(bs.services, entry.Service) { + entryService := entry.Service + if parseCodeBuildRecords { + entryService = strings.TrimSuffix(entryService, "-image") + } + if entryService != "" && len(bs.services) > 0 && !slices.Contains(bs.services, entryService) { continue }
🤖 Fix all issues with AI agents
In `@src/pkg/clouds/aws/codebuild/event.go`:
- Around line 63-65: The Host() method on CodebuildEvent currently returns the
literal "codebuild" which discards the parsed host/build id set during
ParseCodebuildEvent (entry.Host); update CodebuildEvent.Host() to return the
stored field (e.host) and use "codebuild" only as a fallback when e.host is
empty so downstream consumers get the actual build id when available; locate the
Host() method on the CodebuildEvent type and change its return to e.host (with a
conditional fallback).
- Around line 67-68: The Status() method on CodebuildEvent currently returns an
empty string causing SubscribeResponse.Status to be blank; implement Status() to
return a meaningful value by extracting a status from the event payload (e.g.,
return a dedicated field like e.Status or e.BuildStatus if present, otherwise
fall back to the raw message string such as e.RawMessage or the JSON
"detail"->"build-status" value), so subscribers receive a non-empty status;
update the CodebuildEvent.Status() implementation to check and return these
fields in priority order.
🧹 Nitpick comments (1)
src/pkg/cli/client/byoc/aws/subscribe.go (1)
23-35: Consider emitting the trimmed service name in responses.
You filter withstrings.TrimSuffix(..., "-image")but still emitName: evt.Service(). If you want output to match what users subscribe to, consider using the trimmed value.♻️ Suggested refactor
- service := strings.TrimSuffix(evt.Service(), "-image") + service := strings.TrimSuffix(evt.Service(), "-image") if len(s.services) > 0 && !slices.Contains(s.services, service) { return } resp := defangv1.SubscribeResponse{ - Name: evt.Service(), + Name: service, Status: evt.Status(), State: evt.State(), }
Description
This PR implements CodeBuild Event streaming by parsing CodeBuild log messages and emitting events on the subscribe stream (just like we do for ecs events).
Linked Issues
Checklist
Summary by CodeRabbit
Documentation
New Features
Tests