Skip to content
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

Switch from global service registry to dependency "injection" #1818

Closed
12 tasks done
olksdr opened this issue Feb 6, 2023 · 1 comment
Closed
12 tasks done

Switch from global service registry to dependency "injection" #1818

olksdr opened this issue Feb 6, 2023 · 1 comment
Assignees

Comments

@olksdr
Copy link
Contributor

olksdr commented Feb 6, 2023

Currently we use a global registry

pub static REGISTRY: OnceBox<Registry> = OnceBox::new();
with all the services having a record there.

This introduces few challenges:

  • it's kind of difficult to test separate parts of the services when they are depend on each other
  • it's not possible to predict the oder in which services are shutting down, and we might be losing some data

Subtasks

Preview Give feedback
  1. 0 of 1
    olksdr
  2. maintenance
    Dav1dde
  3. bug

As an example how this could look like you can refer to #1770, which introduces also a small isolated test for only 1 service. This will have to be changed for all services:

@olksdr
Copy link
Contributor Author

olksdr commented Mar 22, 2023

At this point we still have few things left where the global registry is used.

Here is the quick grep:

relay-server/src/utils/managed_envelope.rs
191:        let outcome_aggregator = TrackOutcome::from_registry();

relay-server/src/endpoints/public_keys.rs
12:    let relay_cache = RelayCache::from_registry();

relay-server/src/endpoints/outcomes.rs
13:    let producer = OutcomeProducer::from_registry();

and

relay-server/src/actors/envelopes.rs
110:                        ProjectCache::from_registry().send(UpdateRateLimits::new(

relay-server/src/endpoints/common.rs
289:        EnvelopeProcessor::from_registry().send(ProcessMetrics {
309:        ProjectCache::from_registry().send(ValidateEnvelope::new(event_context));
318:        ProjectCache::from_registry().send(ValidateEnvelope::new(managed_envelope));

relay-server/src/endpoints/health_check.rs
16:    Ok(match HealthCheck::from_registry().send(message).await {

relay-server/src/endpoints/forward.rs
243:    UpstreamRelay::from_registry().send(SendRequest(ForwardRequest {

relay-server/src/utils/managed_envelope.rs
257:        TestStore::from_registry().send(Capture::rejected(self.envelope.event_id(), &outcome));

Which are mostly endpoints and utils/managed_envelope.rs which in few cases is also used in the endpoints.

This is basically a followup for #1938. Once the migration to axum is done we can remove the rest of the global registry access and we might also remove global registry all together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants