Skip to content

Commit 581b353

Browse files
committed
feat(stovepipe): read request history by URI
Summary: Intent: - Support history lookup from an exact commit URI while preserving log-only authority. - Keep the plural response model ready for a future multi-attempt URI index. Changes: - Resolve the current URI mapping and load its retained request log. - Return one grouped history under the insert-once mapping contract. - Cover selector validation, missing history, infrastructure errors, and metric tags. This PR builds on #670, which adds request-ID history reads. --- <sub>Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-code/devexp-agent-marketplace/-/blob/claude-code/plugins/dev/uber-dev/skills/pr-create/SKILL.md) skill in devexp-agent-marketplace</sub>
1 parent f2e8410 commit 581b353

3 files changed

Lines changed: 178 additions & 12 deletions

File tree

stovepipe/controller/read_errors.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ func validateHistoryIdentifier(name, value string) error {
4444
// RequestHistoryNotFoundError indicates that no retained history exists for a selector.
4545
type RequestHistoryNotFoundError struct {
4646
RequestID string
47+
URI string
4748
}
4849

4950
// Error implements error.
5051
func (e *RequestHistoryNotFoundError) Error() string {
52+
if e.RequestID == "" {
53+
return fmt.Sprintf("request history not found for URI %q", e.URI)
54+
}
5155
return fmt.Sprintf("request history not found for request ID %q", e.RequestID)
5256
}
5357

stovepipe/controller/request_history.go

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
// RequestHistoryController handles retained request-history lookups.
3030
type RequestHistoryController interface {
3131
GetRequestHistoryByID(ctx context.Context, req entity.GetRequestHistoryByIDRequest) ([]entity.RequestLog, error)
32+
GetRequestHistoryByURI(ctx context.Context, req entity.GetRequestHistoryByURIRequest) ([]entity.RequestHistory, error)
3233
}
3334

3435
var _ RequestHistoryController = (*requestHistoryController)(nil)
@@ -65,11 +66,8 @@ func (c *requestHistoryController) GetRequestHistoryByID(ctx context.Context, re
6566
return nil, fmt.Errorf("GetRequestHistoryByID failed to resolve storage for queue %q: %w", req.Queue, err)
6667
}
6768

68-
logs, err = stores.GetRequestLogStore().List(ctx, req.ID)
69+
logs, err = loadRequestLogs(ctx, stores.GetRequestLogStore(), req.ID, &RequestHistoryNotFoundError{RequestID: req.ID})
6970
if err != nil {
70-
if storage.IsNotFound(err) {
71-
return nil, errs.NewUserError(&RequestHistoryNotFoundError{RequestID: req.ID})
72-
}
7371
return nil, fmt.Errorf("GetRequestHistoryByID failed to list request logs request_id=%s: %w", req.ID, err)
7472
}
7573

@@ -80,3 +78,50 @@ func (c *requestHistoryController) GetRequestHistoryByID(ctx context.Context, re
8078
)
8179
return logs, nil
8280
}
81+
82+
// GetRequestHistoryByURI returns the retained history mapped to an exact commit URI.
83+
func (c *requestHistoryController) GetRequestHistoryByURI(ctx context.Context, req entity.GetRequestHistoryByURIRequest) (histories []entity.RequestHistory, retErr error) {
84+
op := metrics.Begin(c.metricsScope, "get_by_uri", metrics.StorageLatencyBuckets, metrics.TagsFromContext(ctx)...)
85+
defer func() { op.Complete(retErr) }()
86+
87+
if err := validateHistoryIdentifier("queue", req.Queue); err != nil {
88+
return nil, fmt.Errorf("GetRequestHistoryByURI invalid queue: %w", err)
89+
}
90+
if err := validateHistoryIdentifier("URI", req.URI); err != nil {
91+
return nil, fmt.Errorf("GetRequestHistoryByURI invalid request: %w", err)
92+
}
93+
94+
stores, err := c.stores.For(storage.Config{QueueName: req.Queue})
95+
if err != nil {
96+
return nil, fmt.Errorf("GetRequestHistoryByURI failed to resolve storage for queue %q: %w", req.Queue, err)
97+
}
98+
99+
requestID, err := stores.GetRequestURIStore().GetIDByURI(ctx, req.URI)
100+
if err != nil {
101+
if storage.IsNotFound(err) {
102+
return nil, errs.NewUserError(&RequestHistoryNotFoundError{URI: req.URI})
103+
}
104+
return nil, fmt.Errorf("GetRequestHistoryByURI failed to resolve request URI %s: %w", req.URI, err)
105+
}
106+
107+
logs, err := loadRequestLogs(ctx, stores.GetRequestLogStore(), requestID, &RequestHistoryNotFoundError{URI: req.URI})
108+
if err != nil {
109+
return nil, fmt.Errorf("GetRequestHistoryByURI failed to list request logs uri=%s request_id=%s: %w", req.URI, requestID, err)
110+
}
111+
112+
c.logger.Debugw("request history retrieved by URI",
113+
"uri", req.URI,
114+
"request_id", requestID,
115+
"queue", req.Queue,
116+
"event_count", len(logs),
117+
)
118+
return []entity.RequestHistory{{RequestID: requestID, Events: logs}}, nil
119+
}
120+
121+
func loadRequestLogs(ctx context.Context, store storage.RequestLogStore, requestID string, notFound *RequestHistoryNotFoundError) ([]entity.RequestLog, error) {
122+
logs, err := store.List(ctx, requestID)
123+
if storage.IsNotFound(err) {
124+
return nil, errs.NewUserError(notFound)
125+
}
126+
return logs, err
127+
}

stovepipe/controller/request_history_test.go

Lines changed: 125 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,29 +125,146 @@ func TestGetRequestHistoryByID(t *testing.T) {
125125
start, ok := snapshot.Counters()["test.request_history_controller.get_by_id.start+queue=context-queue"]
126126
require.True(t, ok)
127127
assert.EqualValues(t, 1, start.Value())
128-
assertOperationFinishIncludesContextTag(t, snapshot, err == nil)
128+
assertOperationFinishIncludesContextTag(t, snapshot, "get_by_id", err == nil)
129+
})
130+
}
131+
}
132+
133+
func TestGetRequestHistoryByURI(t *testing.T) {
134+
const (
135+
queue = "monorepo/main"
136+
uri = "git://example.com/repo.git/commit/deadbeef"
137+
requestID = "request/monorepo/main/42"
138+
)
139+
backendErr := errors.New("backend unavailable")
140+
logs := []entity.RequestLog{
141+
{ID: "state/1", RequestID: requestID, TimestampMs: 10, State: entity.RequestStateAccepted},
142+
{ID: "event/a", RequestID: requestID, TimestampMs: 20, Event: entity.RequestEventBuildTriggered},
143+
{ID: "event/a", RequestID: requestID, TimestampMs: 20, Event: entity.RequestEventBuildTriggered},
144+
}
145+
wantHistory := []entity.RequestHistory{{RequestID: requestID, Events: logs}}
146+
147+
tests := []struct {
148+
name string
149+
req entity.GetRequestHistoryByURIRequest
150+
mappedID string
151+
factoryErr error
152+
mappingErr error
153+
listErr error
154+
want []entity.RequestHistory
155+
wantInvalid bool
156+
wantNotFound bool
157+
wantCause error
158+
wantLog bool
159+
}{
160+
{name: "singleton history preserves log order and duplicates", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappedID: requestID, want: wantHistory, wantLog: true},
161+
{name: "empty queue", req: entity.GetRequestHistoryByURIRequest{URI: uri}, wantInvalid: true},
162+
{name: "oversized queue", req: entity.GetRequestHistoryByURIRequest{Queue: strings.Repeat("q", maxHistoryIdentifierBytes+1), URI: uri}, wantInvalid: true},
163+
{name: "empty URI", req: entity.GetRequestHistoryByURIRequest{Queue: queue}, wantInvalid: true},
164+
{name: "oversized URI", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: strings.Repeat("u", maxHistoryIdentifierBytes+1)}, wantInvalid: true},
165+
{name: "storage factory failure", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, factoryErr: backendErr, wantCause: backendErr},
166+
{name: "URI mapping not found", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappingErr: fmt.Errorf("lookup: %w", storage.ErrNotFound), wantNotFound: true},
167+
{name: "URI store failure", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappingErr: backendErr, wantCause: backendErr},
168+
{name: "mapped history not found", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappedID: requestID, listErr: fmt.Errorf("query: %w", storage.ErrNotFound), wantNotFound: true},
169+
{name: "log store failure", req: entity.GetRequestHistoryByURIRequest{Queue: queue, URI: uri}, mappedID: requestID, listErr: backendErr, wantCause: backendErr},
170+
}
171+
172+
for _, tt := range tests {
173+
t.Run(tt.name, func(t *testing.T) {
174+
mockCtrl := gomock.NewController(t)
175+
factory := storagemock.NewMockFactory(mockCtrl)
176+
stores := storagemock.NewMockStorage(mockCtrl)
177+
uriStore := storagemock.NewMockRequestURIStore(mockCtrl)
178+
logStore := storagemock.NewMockRequestLogStore(mockCtrl)
179+
if !tt.wantInvalid {
180+
factory.EXPECT().For(storage.Config{QueueName: tt.req.Queue}).Return(stores, tt.factoryErr)
181+
if tt.factoryErr == nil {
182+
stores.EXPECT().GetRequestURIStore().Return(uriStore)
183+
uriStore.EXPECT().GetIDByURI(gomock.Any(), tt.req.URI).Return(tt.mappedID, tt.mappingErr)
184+
if tt.mappingErr == nil {
185+
stores.EXPECT().GetRequestLogStore().Return(logStore)
186+
logStore.EXPECT().List(gomock.Any(), tt.mappedID).Return(logs, tt.listErr)
187+
}
188+
}
189+
}
190+
191+
core, observed := observer.New(zap.DebugLevel)
192+
scope := tally.NewTestScope("test", nil)
193+
controller := NewRequestHistoryController(zap.New(core).Sugar(), scope, factory)
194+
ctx := metrics.WithContextTags(context.Background(), metrics.NewTag("queue", "context-queue"))
195+
196+
got, err := controller.GetRequestHistoryByURI(ctx, tt.req)
197+
198+
assert.Equal(t, tt.want, got)
199+
if tt.wantInvalid {
200+
assert.True(t, IsInvalidRequest(err))
201+
}
202+
assert.Equal(t, tt.wantNotFound, IsRequestHistoryNotFound(err))
203+
assert.Equal(t, tt.wantInvalid || tt.wantNotFound, errs.IsUserError(err))
204+
if tt.wantCause != nil {
205+
assert.ErrorIs(t, err, tt.wantCause)
206+
}
207+
if tt.want != nil {
208+
require.NoError(t, err)
209+
} else {
210+
require.Error(t, err)
211+
}
212+
if tt.wantNotFound {
213+
var notFound *RequestHistoryNotFoundError
214+
require.ErrorAs(t, err, &notFound)
215+
assert.Empty(t, notFound.RequestID)
216+
assert.Equal(t, uri, notFound.URI)
217+
}
218+
219+
entries := observed.FilterMessage("request history retrieved by URI").All()
220+
if tt.wantLog {
221+
require.Len(t, entries, 1)
222+
assert.Equal(t, uri, entries[0].ContextMap()["uri"])
223+
assert.Equal(t, requestID, entries[0].ContextMap()["request_id"])
224+
assert.Equal(t, queue, entries[0].ContextMap()["queue"])
225+
assert.Equal(t, int64(len(logs)), entries[0].ContextMap()["event_count"])
226+
} else {
227+
assert.Empty(t, entries)
228+
}
229+
230+
snapshot := scope.Snapshot()
231+
start, ok := snapshot.Counters()["test.request_history_controller.get_by_uri.start+queue=context-queue"]
232+
require.True(t, ok)
233+
assert.EqualValues(t, 1, start.Value())
234+
assertOperationFinishIncludesContextTag(t, snapshot, "get_by_uri", err == nil)
129235
})
130236
}
131237
}
132238

133239
func TestRequestHistoryNotFoundError(t *testing.T) {
134-
err := fmt.Errorf("lookup failed: %w", &RequestHistoryNotFoundError{RequestID: "request/queue/1"})
240+
tests := []struct {
241+
name string
242+
err error
243+
want RequestHistoryNotFoundError
244+
}{
245+
{name: "request ID", err: fmt.Errorf("lookup failed: %w", &RequestHistoryNotFoundError{RequestID: "request/queue/1"}), want: RequestHistoryNotFoundError{RequestID: "request/queue/1"}},
246+
{name: "URI", err: fmt.Errorf("lookup failed: %w", &RequestHistoryNotFoundError{URI: "git://repo/commit/1"}), want: RequestHistoryNotFoundError{URI: "git://repo/commit/1"}},
247+
}
135248

136-
assert.True(t, IsRequestHistoryNotFound(err))
249+
for _, tt := range tests {
250+
t.Run(tt.name, func(t *testing.T) {
251+
assert.True(t, IsRequestHistoryNotFound(tt.err))
252+
var notFound *RequestHistoryNotFoundError
253+
require.ErrorAs(t, tt.err, &notFound)
254+
assert.Equal(t, tt.want, *notFound)
255+
})
256+
}
137257
assert.False(t, IsRequestHistoryNotFound(errors.New("other")))
138-
var notFound *RequestHistoryNotFoundError
139-
require.ErrorAs(t, err, &notFound)
140-
assert.Equal(t, "request/queue/1", notFound.RequestID)
141258
}
142259

143-
func assertOperationFinishIncludesContextTag(t *testing.T, snapshot tally.Snapshot, success bool) {
260+
func assertOperationFinishIncludesContextTag(t *testing.T, snapshot tally.Snapshot, operation string, success bool) {
144261
t.Helper()
145262
wantResult := "error"
146263
if success {
147264
wantResult = "success"
148265
}
149266
for _, histogram := range snapshot.Histograms() {
150-
if histogram.Name() == "test.request_history_controller.get_by_id.finish" {
267+
if histogram.Name() == "test.request_history_controller."+operation+".finish" {
151268
assert.Equal(t, "context-queue", histogram.Tags()["queue"])
152269
assert.Equal(t, wantResult, histogram.Tags()["result"])
153270
return

0 commit comments

Comments
 (0)