Storage Extension 1/N #2883
Storage Extension 1/N #2883
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2883 +/- ##
=======================================
Coverage 91.55% 91.55%
=======================================
Files 464 464
Lines 22860 22860
=======================================
+ Hits 20929 20930 +1
+ Misses 1438 1437 -1
Partials 493 493
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Is this only for logs? |
|
@jpkrohling It's intended to be for all signals/components. |
|
You'll want to run |
extension/storage/nop_client.go
Outdated
|
|
||
| package storage | ||
|
|
||
| type NopClient struct{} |
There was a problem hiding this comment.
Please follow the model we have in core. and provide a func NewNopClient() Client and avoid exposing the new struct.
May also want to have this in a storagetest package, can be done later.
There was a problem hiding this comment.
I've updated this to follow the pattern established in core. However, I have a question about potentially keeping NewNopClient available.
One of the expectations is that components that make use of a storage extension should seamlessly handle the situation where no storage extension is configured. My intention with introducing this nopClient was to provide an easy way for components to fulfill this expectation. Consider:
// Start the component
func (c *myComponent) Start(ctx context.Context, host component.Host) error {
client := getStorageClient(ctx, host)
// component implementation uses client but doesn't care if it's nop or not
}
func getStorageClient(ctx context.Context, host component.Host) storage.Client {
for _, ext := range host.GetExtensions() {
if ext is a storage extension {
return ext.GetClient()
}
}
return storage.NewNopClient()
}Do you think this is a useful pattern? I suppose components can implement their own nopClient if necessary, but I thought it might be helpful to include.
There was a problem hiding this comment.
Let's see in practice how this will be used in few examples, probably we should make sure we re-evaluate this after couple of PRs that uses it.
Follows #2883 Will resolve #2287 ### Summary This PR includes: - A full implementation of a `file_storage` extension, which can read and write data to the local file system. Any component in the collector may make use of this extension. - Updates to `stanza/internal` to allow stanza-based receivers to use the extension for checkpoints. - A new testbed scenario that has the filelogreceiver using the extension Configuration of the extension is simple. ```yaml file_storage: file_storage/all_settings: directory: /var/lib/otelcol/mydir timeout: 2s ``` The extension is made available to component's via the `host` parameter in their `Start` method: ```go func (r *receiver) Start(ctx context.Context, host component.Host) error { for _, ext := range host.GetExtensions() { if se, ok := ext.(storage.Extension); ok { client, err := se.GetClient(ctx, component.KindReceiver, r.NamedEntity) if err != nil { return err } r.storageClient = client return nil } } r.storageClient = storage.NewNopClient() ... } ```
* WIP - Add storage extension framework * Cleaned up initial implmenetation. Still much to do * Remove extraneous dependency * go tidy * rm gopls from go.mod * Fix typo in TODO * Finish thought in readme * Document storage.Extension, fix doc issues * Run make gendependabot and make gotidy * Use core nop pattern, add context to methods, clean up comments
Follows open-telemetry#2883 Will resolve #2287 ### Summary This PR includes: - A full implementation of a `file_storage` extension, which can read and write data to the local file system. Any component in the collector may make use of this extension. - Updates to `stanza/internal` to allow stanza-based receivers to use the extension for checkpoints. - A new testbed scenario that has the filelogreceiver using the extension Configuration of the extension is simple. ```yaml file_storage: file_storage/all_settings: directory: /var/lib/otelcol/mydir timeout: 2s ``` The extension is made available to component's via the `host` parameter in their `Start` method: ```go func (r *receiver) Start(ctx context.Context, host component.Host) error { for _, ext := range host.GetExtensions() { if se, ok := ext.(storage.Extension); ok { client, err := se.GetClient(ctx, component.KindReceiver, r.NamedEntity) if err != nil { return err } r.storageClient = client return nil } } r.storageClient = storage.NewNopClient() ... } ``` # Conflicts: # go.mod
First step towards implementing plan laid out in #2287 (direct link to design doc)
storagepackagestorage.Extensioninterface that extendscomponent.Extensionstorage.Clientinterface that requiresGet,Set,DeletemethodsNopClientwhich can be used when no storage extension is enabledlocal_file_storageextension which implementsstorage.Extension(functionality not yet implemented)TODO
local_file_storage