Skip to content

Commit 649946c

Browse files
yanivagmanyanivagman
committed
feat(datastores): standardize error returns and document nil contract
- Change all datastore query methods to return (value, error) pattern - Return datastores.ErrNotFound instead of (nil, false) - Document that Registry accessors never return nil stores - Update all datastore implementations and tests
1 parent 1755444 commit 649946c

11 files changed

Lines changed: 96 additions & 71 deletions

File tree

‎api/v1beta1/datastores/interfaces.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ type ProcessStore interface {
4242
DataStore
4343

4444
// GetProcess retrieves process information by entity ID
45-
// Returns nil, false if the process is not found
46-
GetProcess(entityId uint32) (*ProcessInfo, bool)
45+
// Returns ErrNotFound if the process is not found
46+
GetProcess(entityId uint32) (*ProcessInfo, error)
4747

4848
// GetChildProcesses returns all child processes of the given process
4949
// Returns empty slice if no children found
@@ -61,12 +61,12 @@ type ContainerStore interface {
6161
DataStore
6262

6363
// GetContainer retrieves container information by container ID
64-
// Returns nil, false if the container is not found
65-
GetContainer(id string) (*ContainerInfo, bool)
64+
// Returns ErrNotFound if the container is not found
65+
GetContainer(id string) (*ContainerInfo, error)
6666

6767
// GetContainerByName retrieves container information by container name
68-
// Returns nil, false if no container with that name is found
69-
GetContainerByName(name string) (*ContainerInfo, bool)
68+
// Returns ErrNotFound if no container with that name is found
69+
GetContainerByName(name string) (*ContainerInfo, error)
7070
}
7171

7272
// KernelSymbolStore provides access to kernel symbol information
@@ -95,8 +95,8 @@ type DNSStore interface {
9595
DataStore
9696

9797
// GetDNSResponse retrieves cached DNS response for a query
98-
// Returns nil, false if no cached response is found
99-
GetDNSResponse(query string) (*DNSResponse, bool)
98+
// Returns ErrNotFound if no cached response is found
99+
GetDNSResponse(query string) (*DNSResponse, error)
100100
}
101101

102102
// SystemStore provides access to immutable system information
@@ -113,12 +113,12 @@ type SyscallStore interface {
113113
DataStore
114114

115115
// GetSyscallName returns the syscall name for a given ID
116-
// Returns empty string and false if the syscall ID is not found
116+
// Returns ErrNotFound if the syscall ID is not found
117117
// Note: Syscall IDs are architecture-specific (x86 vs ARM)
118-
GetSyscallName(id int32) (string, bool)
118+
GetSyscallName(id int32) (string, error)
119119

120120
// GetSyscallID returns the syscall ID for a given name
121-
// Returns 0 and false if the syscall name is not found
121+
// Returns ErrNotFound if the syscall name is not found
122122
// Note: Syscall IDs are architecture-specific (x86 vs ARM)
123-
GetSyscallID(name string) (int32, bool)
123+
GetSyscallID(name string) (int32, error)
124124
}

‎api/v1beta1/datastores/registry.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,46 @@
11
package datastores
22

33
// Registry provides unified access to all datastores
4+
//
5+
// IMPORTANT: All datastore accessor methods (Processes, Containers, etc.)
6+
// always return a non-nil store instance, even if the store is unavailable
7+
// or disabled. Use IsAvailable() or check store.GetHealth() to verify
8+
// availability before use.
9+
//
10+
// Example checking availability:
11+
//
12+
// registry := params.DataStores
13+
// if !registry.IsAvailable("process") {
14+
// return errors.New("process store required but unavailable")
15+
// }
16+
// procStore := registry.Processes() // Never nil
17+
// proc, err := procStore.GetProcess(entityId)
18+
// if errors.Is(err, ErrNotFound) {
19+
// // Process not found
20+
// }
421
type Registry interface {
522
// Processes returns the process datastore
23+
// Never returns nil - check IsAvailable("process") for availability
624
Processes() ProcessStore
725

826
// Containers returns the container datastore
27+
// Never returns nil - check IsAvailable("container") for availability
928
Containers() ContainerStore
1029

1130
// KernelSymbols returns the kernel symbol datastore
31+
// Never returns nil - check IsAvailable("symbol") for availability
1232
KernelSymbols() KernelSymbolStore
1333

1434
// DNS returns the DNS cache datastore
35+
// Never returns nil - check IsAvailable("dns") for availability
1536
DNS() DNSStore
1637

1738
// System returns the system information datastore
39+
// Never returns nil - check IsAvailable("system") for availability
1840
System() SystemStore
1941

2042
// Syscalls returns the syscall information datastore
43+
// Never returns nil - check IsAvailable("syscall") for availability
2144
Syscalls() SyscallStore
2245

2346
// GetCustom retrieves a custom datastore by name

‎pkg/datastores/container/store.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,22 +75,22 @@ func (m *Manager) GetMetrics() *datastores.DataStoreMetrics {
7575
// ContainerStore interface implementation
7676

7777
// GetContainer retrieves container information by container ID
78-
func (m *Manager) GetContainer(id string) (*datastores.ContainerInfo, bool) {
78+
func (m *Manager) GetContainer(id string) (*datastores.ContainerInfo, error) {
7979
m.lastAccessNano.Store(time.Now().UnixNano())
8080

8181
m.lock.RLock()
8282
cont, ok := m.containerMap[id]
8383
m.lock.RUnlock()
8484

8585
if !ok {
86-
return nil, false
86+
return nil, datastores.ErrNotFound
8787
}
8888

89-
return convertContainer(&cont), true
89+
return convertContainer(&cont), nil
9090
}
9191

9292
// GetContainerByName retrieves container information by container name
93-
func (m *Manager) GetContainerByName(name string) (*datastores.ContainerInfo, bool) {
93+
func (m *Manager) GetContainerByName(name string) (*datastores.ContainerInfo, error) {
9494
m.lastAccessNano.Store(time.Now().UnixNano())
9595

9696
m.lock.RLock()
@@ -107,10 +107,10 @@ func (m *Manager) GetContainerByName(name string) (*datastores.ContainerInfo, bo
107107
m.lock.RUnlock()
108108

109109
if foundCont == nil {
110-
return nil, false
110+
return nil, datastores.ErrNotFound
111111
}
112112

113-
return convertContainer(foundCont), true
113+
return convertContainer(foundCont), nil
114114
}
115115

116116
// convertContainer converts internal Container to public ContainerInfo

‎pkg/datastores/container/store_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,14 @@ func TestManager_DataStoreInterface(t *testing.T) {
3939
})
4040

4141
t.Run("GetContainer_NotFound", func(t *testing.T) {
42-
info, found := mgr.GetContainer("nonexistent")
43-
assert.False(t, found)
42+
info, err := mgr.GetContainer("nonexistent")
43+
assert.ErrorIs(t, err, datastores.ErrNotFound)
4444
assert.Nil(t, info)
4545
})
4646

4747
t.Run("GetContainerByName_NotFound", func(t *testing.T) {
48-
info, found := mgr.GetContainerByName("nonexistent")
49-
assert.False(t, found)
48+
info, err := mgr.GetContainerByName("nonexistent")
49+
assert.ErrorIs(t, err, datastores.ErrNotFound)
5050
assert.Nil(t, info)
5151
})
5252
}

‎pkg/datastores/dns/store.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,17 @@ func (nc *DNSCache) GetMetrics() *datastores.DataStoreMetrics {
7272
// DNSStore interface implementation
7373

7474
// GetDNSResponse retrieves cached DNS response for a query
75-
func (nc *DNSCache) GetDNSResponse(query string) (*datastores.DNSResponse, bool) {
75+
func (nc *DNSCache) GetDNSResponse(query string) (*datastores.DNSResponse, error) {
7676
nc.lastAccessNano.Store(time.Now().UnixNano())
7777

7878
result, err := nc.Get(query)
7979
if err != nil {
80-
return nil, false
80+
return nil, datastores.ErrNotFound
8181
}
8282

8383
return &datastores.DNSResponse{
8484
Query: query,
8585
IPs: result.IPResults(),
8686
Domains: result.DNSResults(),
87-
}, true
87+
}, nil
8888
}

‎pkg/datastores/dns/store_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ func TestDNSCache_DataStoreInterface(t *testing.T) {
3737
})
3838

3939
t.Run("GetDNSResponse_NotFound", func(t *testing.T) {
40-
resp, found := cache.GetDNSResponse("nonexistent.example.com")
41-
assert.False(t, found)
40+
resp, err := cache.GetDNSResponse("nonexistent.example.com")
41+
assert.ErrorIs(t, err, datastores.ErrNotFound)
4242
assert.Nil(t, resp)
4343
})
4444

‎pkg/datastores/process/store.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,13 @@ func (pt *ProcessTree) GetMetrics() *datastores.DataStoreMetrics {
9292
// ProcessStore interface implementation
9393

9494
// GetProcess retrieves process information by entity ID (hash)
95-
func (pt *ProcessTree) GetProcess(entityId uint32) (*datastores.ProcessInfo, bool) {
95+
func (pt *ProcessTree) GetProcess(entityId uint32) (*datastores.ProcessInfo, error) {
9696
pt.lastAccessNano.Store(time.Now().UnixNano())
9797

9898
hash := entityId
9999
proc, ok := pt.GetProcessByHash(hash)
100100
if !ok {
101-
return nil, false
101+
return nil, datastores.ErrNotFound
102102
}
103103

104104
info := proc.GetInfo()
@@ -119,7 +119,7 @@ func (pt *ProcessTree) GetProcess(entityId uint32) (*datastores.ProcessInfo, boo
119119
ExitTime: info.GetExitTime(),
120120
UID: info.GetUid(),
121121
GID: info.GetGid(),
122-
}, true
122+
}, nil
123123
}
124124

125125
// GetChildProcesses returns all child processes of the given process
@@ -139,7 +139,7 @@ func (pt *ProcessTree) GetChildProcesses(entityId uint32) ([]*datastores.Process
139139

140140
children := make([]*datastores.ProcessInfo, 0, len(childrenMap))
141141
for childHash := range childrenMap {
142-
if childInfo, ok := pt.GetProcess(childHash); ok {
142+
if childInfo, err := pt.GetProcess(childHash); err == nil {
143143
children = append(children, childInfo)
144144
}
145145
}

‎pkg/datastores/process/store_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ func TestProcessTree_DataStoreInterface(t *testing.T) {
4040
})
4141

4242
t.Run("GetProcess_NotFound", func(t *testing.T) {
43-
info, found := pt.GetProcess(999999)
44-
assert.False(t, found)
43+
info, err := pt.GetProcess(999999)
44+
assert.ErrorIs(t, err, datastores.ErrNotFound)
4545
assert.Nil(t, info)
4646
})
4747

@@ -170,8 +170,8 @@ func TestProcessStore_GetAncestry(t *testing.T) {
170170
}
171171
proc.GetInfo().SetFeed(feed)
172172

173-
info, ok := pt.GetProcess(uint32(999))
174-
require.True(t, ok)
173+
info, err := pt.GetProcess(uint32(999))
174+
require.NoError(t, err)
175175
assert.NotZero(t, info.ExitTime, "ExitTime should be populated")
176176
})
177177
}

‎pkg/datastores/registry_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ type mockProcessStore struct {
2626
mockDataStore
2727
}
2828

29-
func (m *mockProcessStore) GetProcess(entityId uint32) (*datastores.ProcessInfo, bool) {
30-
return nil, false
29+
func (m *mockProcessStore) GetProcess(entityId uint32) (*datastores.ProcessInfo, error) {
30+
return nil, datastores.ErrNotFound
3131
}
3232

3333
func (m *mockProcessStore) GetChildProcesses(entityId uint32) ([]*datastores.ProcessInfo, error) {
@@ -43,12 +43,12 @@ type mockContainerStore struct {
4343
mockDataStore
4444
}
4545

46-
func (m *mockContainerStore) GetContainer(id string) (*datastores.ContainerInfo, bool) {
47-
return nil, false
46+
func (m *mockContainerStore) GetContainer(id string) (*datastores.ContainerInfo, error) {
47+
return nil, datastores.ErrNotFound
4848
}
4949

50-
func (m *mockContainerStore) GetContainerByName(name string) (*datastores.ContainerInfo, bool) {
51-
return nil, false
50+
func (m *mockContainerStore) GetContainerByName(name string) (*datastores.ContainerInfo, error) {
51+
return nil, datastores.ErrNotFound
5252
}
5353

5454
func TestRegistry_RegisterStore(t *testing.T) {

‎pkg/datastores/syscall/store.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,36 +48,36 @@ func (s *Store) GetMetrics() *datastores.DataStoreMetrics {
4848
}
4949

5050
// GetSyscallName returns the syscall name for a given ID
51-
// Returns empty string and false if the syscall ID is not found or is not a syscall
52-
func (s *Store) GetSyscallName(id int32) (string, bool) {
51+
// Returns ErrNotFound if the syscall ID is not found or is not a syscall
52+
func (s *Store) GetSyscallName(id int32) (string, error) {
5353
def := s.eventCore.GetDefinitionByID(events.ID(id))
5454

5555
// Check if definition was found (Undefined means not found)
5656
if def.GetID() == events.Undefined {
57-
return "", false
57+
return "", datastores.ErrNotFound
5858
}
5959

6060
// Only return name if this is actually a syscall
6161
if !def.IsSyscall() {
62-
return "", false
62+
return "", datastores.ErrNotFound
6363
}
6464

65-
return def.GetName(), true
65+
return def.GetName(), nil
6666
}
6767

6868
// GetSyscallID returns the syscall ID for a given name
69-
// Returns 0 and false if the syscall name is not found or is not a syscall
70-
func (s *Store) GetSyscallID(name string) (int32, bool) {
69+
// Returns ErrNotFound if the syscall name is not found or is not a syscall
70+
func (s *Store) GetSyscallID(name string) (int32, error) {
7171
id, found := s.eventCore.GetDefinitionIDByName(name)
7272
if !found {
73-
return 0, false
73+
return 0, datastores.ErrNotFound
7474
}
7575

7676
// Verify this is actually a syscall
7777
def := s.eventCore.GetDefinitionByID(id)
7878
if def.GetID() == events.Undefined || !def.IsSyscall() {
79-
return 0, false
79+
return 0, datastores.ErrNotFound
8080
}
8181

82-
return int32(id), true
82+
return int32(id), nil
8383
}

0 commit comments

Comments
 (0)