Skip to content

Commit 9493e35

Browse files
alexluongclaude
andauthored
refactor: remove unused DestinationID from RetrieveEventRequest (#679)
* refactor: remove unused DestinationID from RetrieveEventRequest After iterations of access patterns, the current version doesn't require DestinationID in this operation. Events are destination-agnostic - use ListAttempt with DestinationIDs filter for destination-scoped queries. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: query events table in RetrieveEvent RetrieveEvent now queries the events table directly instead of the attempts table. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: use QueryRow in chlogstore retrieve functions to prevent silent errors RetrieveEvent and RetrieveAttempt used Query + rows.Next() without checking rows.Err(), causing ClickHouse query errors to be silently treated as "not found" (404) instead of surfacing as errors (500). Switch to QueryRow which handles this correctly via sql.ErrNoRows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: align API response structs with internal models --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 689943e commit 9493e35

File tree

11 files changed

+83
-97
lines changed

11 files changed

+83
-97
lines changed

cmd/e2e/log_queries_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,10 @@ func (s *basicSuite) TestLogQueries_Attempts() {
7676
first := resp.Models[0]
7777
s.NotEmpty(first["id"])
7878
s.NotEmpty(first["event_id"])
79+
s.Equal(setup.tenantID, first["tenant_id"])
7980
s.Equal(setup.destinationID, first["destination_id"])
8081
s.NotEmpty(first["status"])
81-
s.NotEmpty(first["delivered_at"])
82+
s.NotEmpty(first["time"])
8283
s.Equal(float64(0), first["attempt_number"])
8384
})
8485

docs/apis/openapi.yaml

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,6 +1735,10 @@ components:
17351735
id:
17361736
type: string
17371737
example: "evt_123"
1738+
tenant_id:
1739+
type: string
1740+
description: The tenant this event belongs to.
1741+
example: "tnt_123"
17381742
destination_id:
17391743
type: string
17401744
example: "des_456"
@@ -1768,29 +1772,6 @@ components:
17681772
description: Freeform JSON data of the event.
17691773
additionalProperties: true
17701774
example: { "user_id": "userid", "status": "active" }
1771-
DeliveryAttempt:
1772-
type: object
1773-
properties:
1774-
delivered_at:
1775-
type: string
1776-
format: date-time
1777-
example: "2024-01-01T00:00:00Z"
1778-
status:
1779-
type: string
1780-
enum: [success, failed]
1781-
example: "success"
1782-
response_status_code:
1783-
type: integer
1784-
example: 200
1785-
response_body:
1786-
type: string # Or potentially object if JSON
1787-
example: '{"status":"ok"}'
1788-
response_headers:
1789-
type: object
1790-
additionalProperties:
1791-
type: string
1792-
example: { "content-type": "application/json" }
1793-
17941775
# Attempt schemas for attempts-first API
17951776
Attempt:
17961777
type: object
@@ -1800,12 +1781,16 @@ components:
18001781
type: string
18011782
description: Unique identifier for this attempt.
18021783
example: "atm_123"
1784+
tenant_id:
1785+
type: string
1786+
description: The tenant this attempt belongs to.
1787+
example: "tnt_123"
18031788
status:
18041789
type: string
18051790
enum: [success, failed]
18061791
description: The attempt status.
18071792
example: "success"
1808-
delivered_at:
1793+
time:
18091794
type: string
18101795
format: date-time
18111796
description: Time the attempt was made.
@@ -1847,6 +1832,14 @@ components:
18471832
id:
18481833
type: string
18491834
example: "evt_123"
1835+
tenant_id:
1836+
type: string
1837+
description: The tenant this event belongs to.
1838+
example: "tnt_123"
1839+
destination_id:
1840+
type: string
1841+
description: The destination this event was delivered to.
1842+
example: "des_456"
18501843
topic:
18511844
type: string
18521845
example: "user.created"
@@ -1872,6 +1865,14 @@ components:
18721865
id:
18731866
type: string
18741867
example: "evt_123"
1868+
tenant_id:
1869+
type: string
1870+
description: The tenant this event belongs to.
1871+
example: "tnt_123"
1872+
destination_id:
1873+
type: string
1874+
description: The destination this event was delivered to.
1875+
example: "des_456"
18751876
topic:
18761877
type: string
18771878
example: "user.created"
@@ -2058,7 +2059,7 @@ tags:
20582059
Each attempt contains:
20592060
- `id`: Unique attempt identifier
20602061
- `status`: success or failed
2061-
- `delivered_at`: Timestamp of the attempt
2062+
- `time`: Timestamp of the attempt
20622063
- `code`: HTTP status code or error code
20632064
- `attempt`: Attempt number (1 for first attempt, 2+ for retries)
20642065
- `event`: Associated event (ID or included object)
@@ -2640,14 +2641,14 @@ paths:
26402641
models:
26412642
- id: "atm_123"
26422643
status: "success"
2643-
delivered_at: "2024-01-01T00:00:05Z"
2644+
time: "2024-01-01T00:00:05Z"
26442645
code: "200"
26452646
attempt_number: 1
26462647
event_id: "evt_123"
26472648
destination_id: "des_456"
26482649
- id: "att_124"
26492650
status: "failed"
2650-
delivered_at: "2024-01-02T10:00:01Z"
2651+
time: "2024-01-02T10:00:01Z"
26512652
code: "503"
26522653
attempt_number: 2
26532654
event_id: "evt_789"
@@ -2664,7 +2665,7 @@ paths:
26642665
models:
26652666
- id: "del_123"
26662667
status: "success"
2667-
delivered_at: "2024-01-01T00:00:05Z"
2668+
time: "2024-01-01T00:00:05Z"
26682669
code: "200"
26692670
attempt_number: 1
26702671
event_id: "evt_123"
@@ -2734,7 +2735,7 @@ paths:
27342735
value:
27352736
id: "atm_123"
27362737
status: "success"
2737-
delivered_at: "2024-01-01T00:00:05Z"
2738+
time: "2024-01-01T00:00:05Z"
27382739
code: "200"
27392740
attempt_number: 1
27402741
event_id: "evt_123"
@@ -2744,7 +2745,7 @@ paths:
27442745
value:
27452746
id: "atm_123"
27462747
status: "success"
2747-
delivered_at: "2024-01-01T00:00:05Z"
2748+
time: "2024-01-01T00:00:05Z"
27482749
code: "200"
27492750
response_data:
27502751
status_code: 200
@@ -3271,14 +3272,14 @@ paths:
32713272
models:
32723273
- id: "atm_123"
32733274
status: "success"
3274-
delivered_at: "2024-01-01T00:00:05Z"
3275+
time: "2024-01-01T00:00:05Z"
32753276
code: "200"
32763277
attempt_number: 1
32773278
event_id: "evt_123"
32783279
destination_id: "des_456"
32793280
- id: "atm_124"
32803281
status: "failed"
3281-
delivered_at: "2024-01-02T10:00:01Z"
3282+
time: "2024-01-02T10:00:01Z"
32823283
code: "503"
32833284
attempt_number: 2
32843285
event_id: "evt_789"
@@ -3350,7 +3351,7 @@ paths:
33503351
value:
33513352
id: "atm_123"
33523353
status: "success"
3353-
delivered_at: "2024-01-01T00:00:05Z"
3354+
time: "2024-01-01T00:00:05Z"
33543355
code: "200"
33553356
attempt_number: 1
33563357
event_id: "evt_123"

internal/apirouter/log_handlers.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,9 @@ func parseIncludeOptions(c *gin.Context) IncludeOptions {
8686
// APIAttempt is the API response for an attempt
8787
type APIAttempt struct {
8888
ID string `json:"id"`
89+
TenantID string `json:"tenant_id"`
8990
Status string `json:"status"`
90-
DeliveredAt time.Time `json:"delivered_at"`
91+
Time time.Time `json:"time"`
9192
Code string `json:"code,omitempty"`
9293
ResponseData map[string]interface{} `json:"response_data,omitempty"`
9394
AttemptNumber int `json:"attempt_number"`
@@ -101,6 +102,8 @@ type APIAttempt struct {
101102
// APIEventSummary is the event object when expand=event (without data)
102103
type APIEventSummary struct {
103104
ID string `json:"id"`
105+
TenantID string `json:"tenant_id"`
106+
DestinationID string `json:"destination_id"`
104107
Topic string `json:"topic"`
105108
Time time.Time `json:"time"`
106109
EligibleForRetry bool `json:"eligible_for_retry"`
@@ -110,6 +113,8 @@ type APIEventSummary struct {
110113
// APIEventFull is the event object when expand=event.data
111114
type APIEventFull struct {
112115
ID string `json:"id"`
116+
TenantID string `json:"tenant_id"`
117+
DestinationID string `json:"destination_id"`
113118
Topic string `json:"topic"`
114119
Time time.Time `json:"time"`
115120
EligibleForRetry bool `json:"eligible_for_retry"`
@@ -120,6 +125,8 @@ type APIEventFull struct {
120125
// APIEvent is the API response for retrieving a single event
121126
type APIEvent struct {
122127
ID string `json:"id"`
128+
TenantID string `json:"tenant_id"`
129+
DestinationID string `json:"destination_id"`
123130
Topic string `json:"topic"`
124131
Time time.Time `json:"time"`
125132
EligibleForRetry bool `json:"eligible_for_retry"`
@@ -143,8 +150,9 @@ type EventPaginatedResult struct {
143150
func toAPIAttempt(ar *logstore.AttemptRecord, opts IncludeOptions) APIAttempt {
144151
api := APIAttempt{
145152
ID: ar.Attempt.ID,
153+
TenantID: ar.Attempt.TenantID,
146154
Status: ar.Attempt.Status,
147-
DeliveredAt: ar.Attempt.Time,
155+
Time: ar.Attempt.Time,
148156
Code: ar.Attempt.Code,
149157
AttemptNumber: ar.Attempt.AttemptNumber,
150158
Manual: ar.Attempt.Manual,
@@ -160,6 +168,8 @@ func toAPIAttempt(ar *logstore.AttemptRecord, opts IncludeOptions) APIAttempt {
160168
if opts.EventData {
161169
api.Event = APIEventFull{
162170
ID: ar.Event.ID,
171+
TenantID: ar.Event.TenantID,
172+
DestinationID: ar.Event.DestinationID,
163173
Topic: ar.Event.Topic,
164174
Time: ar.Event.Time,
165175
EligibleForRetry: ar.Event.EligibleForRetry,
@@ -169,6 +179,8 @@ func toAPIAttempt(ar *logstore.AttemptRecord, opts IncludeOptions) APIAttempt {
169179
} else if opts.Event {
170180
api.Event = APIEventSummary{
171181
ID: ar.Event.ID,
182+
TenantID: ar.Event.TenantID,
183+
DestinationID: ar.Event.DestinationID,
172184
Topic: ar.Event.Topic,
173185
Time: ar.Event.Time,
174186
EligibleForRetry: ar.Event.EligibleForRetry,
@@ -314,6 +326,8 @@ func (h *LogHandlers) RetrieveEvent(c *gin.Context) {
314326
}
315327
c.JSON(http.StatusOK, APIEvent{
316328
ID: event.ID,
329+
TenantID: event.TenantID,
330+
DestinationID: event.DestinationID,
317331
Topic: event.Topic,
318332
Time: event.Time,
319333
EligibleForRetry: event.EligibleForRetry,
@@ -443,6 +457,8 @@ func (h *LogHandlers) listEventsInternal(c *gin.Context, tenantID string) {
443457
for i, e := range response.Data {
444458
apiEvents[i] = APIEvent{
445459
ID: e.ID,
460+
TenantID: e.TenantID,
461+
DestinationID: e.DestinationID,
446462
Topic: e.Topic,
447463
Time: e.Time,
448464
EligibleForRetry: e.EligibleForRetry,

internal/logstore/chlogstore/chlogstore.go

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package chlogstore
22

33
import (
44
"context"
5+
"database/sql"
56
"encoding/json"
7+
"errors"
68
"fmt"
79
"strconv"
810
"strings"
@@ -527,11 +529,6 @@ func (s *logStoreImpl) RetrieveEvent(ctx context.Context, req driver.RetrieveEve
527529
conditions = append(conditions, "event_id = ?")
528530
args = append(args, req.EventID)
529531

530-
if req.DestinationID != "" {
531-
conditions = append(conditions, "destination_id = ?")
532-
args = append(args, req.DestinationID)
533-
}
534-
535532
whereClause := strings.Join(conditions, " AND ")
536533

537534
query := fmt.Sprintf(`
@@ -546,22 +543,14 @@ func (s *logStoreImpl) RetrieveEvent(ctx context.Context, req driver.RetrieveEve
546543
data
547544
FROM %s
548545
WHERE %s
549-
LIMIT 1`, s.attemptsTable, whereClause)
546+
LIMIT 1`, s.eventsTable, whereClause)
550547

551-
rows, err := s.chDB.Query(ctx, query, args...)
552-
if err != nil {
553-
return nil, fmt.Errorf("query failed: %w", err)
554-
}
555-
defer rows.Close()
556-
557-
if !rows.Next() {
558-
return nil, nil
559-
}
548+
row := s.chDB.QueryRow(ctx, query, args...)
560549

561550
var metadataStr, dataStr string
562551
event := &models.Event{}
563552

564-
if err := rows.Scan(
553+
if err := row.Scan(
565554
&event.ID,
566555
&event.TenantID,
567556
&event.DestinationID,
@@ -571,6 +560,9 @@ func (s *logStoreImpl) RetrieveEvent(ctx context.Context, req driver.RetrieveEve
571560
&metadataStr,
572561
&dataStr,
573562
); err != nil {
563+
if errors.Is(err, sql.ErrNoRows) {
564+
return nil, nil
565+
}
574566
return nil, fmt.Errorf("scan failed: %w", err)
575567
}
576568

@@ -623,15 +615,7 @@ func (s *logStoreImpl) RetrieveAttempt(ctx context.Context, req driver.RetrieveA
623615
WHERE %s
624616
LIMIT 1`, s.attemptsTable, whereClause)
625617

626-
rows, err := s.chDB.Query(ctx, query, args...)
627-
if err != nil {
628-
return nil, fmt.Errorf("query failed: %w", err)
629-
}
630-
defer rows.Close()
631-
632-
if !rows.Next() {
633-
return nil, nil
634-
}
618+
row := s.chDB.QueryRow(ctx, query, args...)
635619

636620
var (
637621
eventID string
@@ -651,7 +635,7 @@ func (s *logStoreImpl) RetrieveAttempt(ctx context.Context, req driver.RetrieveA
651635
attemptNumber uint32
652636
)
653637

654-
err = rows.Scan(
638+
err := row.Scan(
655639
&eventID,
656640
&tenantID,
657641
&destinationID,
@@ -669,6 +653,9 @@ func (s *logStoreImpl) RetrieveAttempt(ctx context.Context, req driver.RetrieveA
669653
&attemptNumber,
670654
)
671655
if err != nil {
656+
if errors.Is(err, sql.ErrNoRows) {
657+
return nil, nil
658+
}
672659
return nil, fmt.Errorf("scan failed: %w", err)
673660
}
674661

internal/logstore/driver/driver.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,8 @@ type ListAttemptResponse struct {
6161
}
6262

6363
type RetrieveEventRequest struct {
64-
TenantID string // optional - filter by tenant (if empty, searches all tenants)
65-
EventID string // required
66-
DestinationID string // optional - if provided, scopes to that destination
64+
TenantID string // optional - filter by tenant (if empty, searches all tenants)
65+
EventID string // required
6766
}
6867

6968
type RetrieveAttemptRequest struct {

internal/logstore/drivertest/crud.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -296,17 +296,6 @@ func testCRUD(t *testing.T, newHarness HarnessMaker) {
296296
assert.Equal(t, tenantID, retrieved.TenantID)
297297
})
298298

299-
t.Run("RetrieveEvent with destination filter", func(t *testing.T) {
300-
retrieved, err := logStore.RetrieveEvent(ctx, driver.RetrieveEventRequest{
301-
TenantID: tenantID,
302-
EventID: knownEventID,
303-
DestinationID: destinationIDs[0],
304-
})
305-
require.NoError(t, err)
306-
require.NotNil(t, retrieved)
307-
assert.Equal(t, destinationIDs[0], retrieved.DestinationID)
308-
})
309-
310299
t.Run("RetrieveEvent non-existent returns nil", func(t *testing.T) {
311300
retrieved, err := logStore.RetrieveEvent(ctx, driver.RetrieveEventRequest{
312301
TenantID: tenantID,

internal/logstore/memlogstore/memlogstore.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,6 @@ func (s *memLogStore) RetrieveEvent(ctx context.Context, req driver.RetrieveEven
357357
if req.TenantID != "" && event.TenantID != req.TenantID {
358358
return nil, nil
359359
}
360-
if req.DestinationID != "" && event.DestinationID != req.DestinationID {
361-
return nil, nil
362-
}
363360
return copyEvent(event), nil
364361
}
365362

0 commit comments

Comments
 (0)