-
Notifications
You must be signed in to change notification settings - Fork 29
feat: Rework tenant cache #1601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "flag" | ||
| "strings" | ||
| ) | ||
|
|
||
| type StringList []string | ||
|
|
||
| var _ flag.Value = (*StringList)(nil) | ||
|
|
||
| func (l *StringList) String() string { | ||
| if l == nil || len(*l) == 0 { | ||
| return "" | ||
| } | ||
|
|
||
| return strings.Join(*l, ", ") | ||
| } | ||
|
|
||
| func (l *StringList) Set(value string) error { | ||
| for v := range strings.SplitSeq(value, ",") { | ||
| *l = append(*l, strings.TrimSpace(v)) | ||
| } | ||
|
|
||
| return nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestStringList(t *testing.T) { | ||
| { | ||
| var sl StringList | ||
| require.Equal(t, "", sl.String()) | ||
| } | ||
|
|
||
| { | ||
| var sl StringList | ||
| require.NoError(t, sl.Set("a")) | ||
| require.Equal(t, []string{"a"}, []string(sl)) | ||
| require.Equal(t, "a", sl.String()) | ||
| } | ||
|
|
||
| { | ||
| var sl StringList | ||
| require.NoError(t, sl.Set("a,b")) | ||
| require.Equal(t, []string{"a", "b"}, []string(sl)) | ||
| require.Equal(t, "a, b", sl.String()) | ||
| } | ||
|
|
||
| { | ||
| var sl StringList | ||
| require.NoError(t, sl.Set("a,b,c")) | ||
| require.Equal(t, []string{"a", "b", "c"}, []string(sl)) | ||
| require.Equal(t, "a, b, c", sl.String()) | ||
| } | ||
|
|
||
| { | ||
| var sl StringList | ||
| require.NoError(t, sl.Set("a, b")) | ||
| require.Equal(t, []string{"a", "b"}, []string(sl)) | ||
| require.Equal(t, "a, b", sl.String()) | ||
| } | ||
|
|
||
| { | ||
| var sl StringList | ||
| require.NoError(t, sl.Set(" a, b ")) | ||
| require.Equal(t, []string{"a", "b"}, []string(sl)) | ||
| require.Equal(t, "a, b", sl.String()) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| // Package cache provides an interface-based caching system with multiple implementations. | ||
| // | ||
| // The package defines a Cache interface that all implementations must satisfy, | ||
| // allowing the agent to use different cache backends interchangeably. This design | ||
| // eliminates the need for nil checks throughout the codebase. | ||
| // | ||
| // Available implementations: | ||
| // - Memcached: Distributed cache using memcached servers | ||
| // - Noop: No-op cache for testing and fallback (always returns ErrCacheMiss) | ||
| // | ||
| // The cache supports multiple memcached servers, custom expiration times, | ||
| // and automatic serialization/deserialization of Go values using encoding/gob. | ||
| // | ||
| // Example usage with memcached: | ||
| // | ||
| // cc, err := cache.New(cache.Config{ | ||
| // Servers: []string{"localhost:11211", "cache1:11211"}, | ||
| // Logger: logger, | ||
| // }) | ||
| // if err != nil { | ||
| // // Fall back to noop cache | ||
| // cc = cache.NewNoop(logger) | ||
| // } | ||
| // | ||
| // // Store a struct | ||
| // err = cc.Set(ctx, "user:123", user, 5*time.Minute) | ||
| // | ||
| // // Retrieve a struct | ||
| // var user User | ||
| // err = cc.Get(ctx, "user:123", &user) | ||
| // if err == cache.ErrCacheMiss { | ||
| // // Key not found, fetch from source | ||
| // } else if err != nil { | ||
| // // Other error (network, deserialization, etc.) | ||
| // log.Warn("cache error: %v", err) | ||
| // } | ||
| // | ||
| // Example usage with noop cache (for testing): | ||
| // | ||
| // cc := cache.NewNoop(logger) | ||
| // // All Get operations return ErrCacheMiss | ||
| // // All Set/Delete/Flush operations succeed but do nothing | ||
| package cache | ||
|
|
||
| import ( | ||
| "context" | ||
| "flag" | ||
| "time" | ||
|
|
||
| "github.com/grafana/synthetic-monitoring-agent/internal/error_types" | ||
| ) | ||
|
|
||
| const ( | ||
| // ErrCacheMiss is returned when a key is not found in the cache. | ||
| // This is a sentinel error that allows callers to distinguish between | ||
| // a cache miss and other errors (network issues, serialization failures, etc.). | ||
| ErrCacheMiss = error_types.BasicError("cache miss") | ||
| ) | ||
|
|
||
| // Cache defines the interface for cache operations. | ||
| // All cache implementations (memcached, noop, local, etc.) must implement this interface. | ||
| // | ||
| // This interface allows the agent to use different cache backends interchangeably, | ||
| // including a no-op implementation that eliminates the need for nil checks in client code. | ||
| type Cache interface { | ||
| // Set stores a value in the cache with the specified expiration time. | ||
| // Returns an error if the operation fails. | ||
| Set(ctx context.Context, key string, value any, expiration time.Duration) error | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the bit where I struggled the most. Otter uses generics to have a more strongly-typed cache. Since the goal is to use memcached, which does not have a strongly-typed API, I thought about multiple clients, one per type, but it was weird. |
||
|
|
||
| // Get retrieves a value from the cache and deserializes it into dest. | ||
| // Returns ErrCacheMiss if the key is not found. | ||
| // Returns an error for other failures (network, serialization, etc.). | ||
| Get(ctx context.Context, key string, dest any) error | ||
|
|
||
| // Delete removes a key from the cache. | ||
| // Returns nil if the key doesn't exist (idempotent). | ||
| Delete(ctx context.Context, key string) error | ||
|
|
||
| // Flush removes all items from the cache. | ||
| // Use this operation cautiously as it affects all cache users. | ||
| Flush(ctx context.Context) error | ||
| } | ||
|
|
||
| type Kind string | ||
|
|
||
| var _ flag.Value = (*Kind)(nil) | ||
|
|
||
| const KindAuto Kind = "auto" | ||
|
|
||
| const ErrUnsupportedCacheKind = error_types.BasicError("unsupported cache type") | ||
|
|
||
| func (val *Kind) Set(s string) error { | ||
| switch s { | ||
| case string(KindAuto), string(KindLocal), string(KindMemcached), string(KindNoop): | ||
| *val = Kind(s) | ||
|
|
||
| return nil | ||
|
|
||
| default: | ||
| return ErrUnsupportedCacheKind | ||
| } | ||
| } | ||
|
|
||
| func (val Kind) String() string { | ||
| return string(val) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are three implementations of this interface.
A memcached one. This is the one I would like to use eventually.
A local one, based on Otter.
A noop one, which is a fallback in case the other two are not available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider a multi-tiered implementation, too, using L1 with Otter and L2 with memcached. I would prefer to get some numbers before going that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think I initially expected a multi-tiered implementation. It's not something we need to implement immediately, I'm fine with running this using one or the other first and getting some data out of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to run with Otter by default and test with memcached in a couple of places.