add ingester interface #3352
add ingester interface #3352
Conversation
replay
force-pushed
the
ingester_interface
branch
2 times, most recently
from
bf2a859 to
015efd4
Compare
|
I would suggest moving new |
Thx, that's a great idea, I'll do that |
replay
force-pushed
the
ingester_interface
branch
from
015efd4 to
6435d71
Compare
Signed-off-by: Mauro Stettler <[email protected]>
replay
force-pushed
the
ingester_interface
branch
from
6435d71 to
18a66d8
Compare
| @@ -201,8 +200,15 @@ func (a *API) RegisterDistributor(d *distributor.Distributor, pushConfig distrib | |||
| a.RegisterRoute("/ha-tracker", d.HATracker, false, "GET") | |||
| } | |||
|
|
|||
| type ingester interface { | |||
There was a problem hiding this comment.
For what I'm trying to do it's not necessary to export it. My goal is just to pass RegisterIngesters() an alternative struct which satisfies the same interface as the "original" ingester. Or is there another reason to export it?
There was a problem hiding this comment.
I don't think export is necessary, but a comment explaining why we do this would be nice. Otherwise it looks like magic.
There was a problem hiding this comment.
ok, good point, I'll add one
| @@ -201,8 +200,15 @@ func (a *API) RegisterDistributor(d *distributor.Distributor, pushConfig distrib | |||
| a.RegisterRoute("/ha-tracker", d.HATracker, false, "GET") | |||
| } | |||
|
|
|||
| type ingester interface { | |||
There was a problem hiding this comment.
I don't think export is necessary, but a comment explaining why we do this would be nice. Otherwise it looks like magic.
…rgid-ctx * 'master' of github.com:cortexproject/cortex: Enforce integration tests default flags config to never be overwritten (cortexproject#3370) Avoid deletion of blocks which are not shipped (cortexproject#3346) Upgrade Thanos to latest master (cortexproject#3363) Migrate CircleCI workflows to GitHub Actions (2/3) (cortexproject#3341) Remove comments that doesn't seem right (cortexproject#3361) add ingester interface (cortexproject#3352) Fail fast an ingester if unable to load existing TSDBs (cortexproject#3354) Fixed Gossip memberlist members joining when addresses are configured using DNS-based service discovery (cortexproject#3360) Export distributor method to get ingester replication set (cortexproject#3356) Correct link for Block Storage reference (cortexproject#3234) Added section on Cleaner. (cortexproject#3327) Update prometheus vendor to master (cortexproject#3345) adding GHA CI env variable check (cortexproject#3351) Add ingesters shuffle sharding support on the read path (cortexproject#3252)
I would like to extend the Ingester by wrapping it in another struct, which overrides some methods and does additional work before calling the original Ingester methods.
Currently the API method
.RegisterIngester()expects the concrete type*ingester.Ingester, so I can't pass it the extended ingester type. Outside theapipackage I also can't easily re-implement the.RegisterIngester()method, because it uses an unexported property of the API (sourceIPs).By changing the API method
RegisterIngester()to expect an interface I'll be able to pass it the extended Ingester type and register its methods on the HTTP and the GRPC server.What do you think about this approach?
An alternative to creating an Ingester interface would be to export that unexported property
.sourceIPswhich is used byAPI.RegisterIngester(), that way I could basically just re-implement what this method is doing outside the package, but that seems less elegant and might be more error-prone because ifRegisterIngester()changes in the future I'll have to ensure that my copy stays up2date.