From e66986559e697bab08f07f3318fb895905b5d683 Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Wed, 7 Jun 2023 13:12:27 -0400 Subject: [PATCH 01/13] Initial Service Discovery --- Cargo.lock | 47 +++++++ Cargo.toml | 2 + registry/Cargo.toml | 18 +++ registry/proto/Cargo.toml | 17 +++ registry/proto/build.rs | 8 ++ .../proto/chariott/v1/chariott_registry.proto | 80 ++++++++++++ registry/proto/src/lib.rs | 10 ++ registry/src/main.rs | 45 +++++++ registry/src/registry_impl.rs | 115 ++++++++++++++++++ 9 files changed, 342 insertions(+) create mode 100644 registry/Cargo.toml create mode 100644 registry/proto/Cargo.toml create mode 100644 registry/proto/build.rs create mode 100644 registry/proto/chariott/v1/chariott_registry.proto create mode 100644 registry/proto/src/lib.rs create mode 100644 registry/src/main.rs create mode 100644 registry/src/registry_impl.rs diff --git a/Cargo.lock b/Cargo.lock index 293f597c..40162c19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2024,6 +2024,29 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14f2252c834a40ed9bb5422029649578e63aa341ac401f74e719dd1afda8394e" +[[package]] +name = "parking_lot" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9069cbb9f99e3a5083476ccb29ceb1de18b9118cafa53e90c9551235de2b9521" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall 0.2.16", + "smallvec", + "windows-sys 0.45.0", +] + [[package]] name = "password-hash" version = "0.4.2" @@ -2267,6 +2290,16 @@ dependencies = [ "prost", ] +[[package]] +name = "proto-registry" +version = "0.1.0" +dependencies = [ + "prost", + "tokio", + "tonic", + "tonic-build", +] + [[package]] name = "protobuf" version = "2.27.1" @@ -2444,6 +2477,20 @@ version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "436b050e76ed2903236f032a59761c1eb99e1b0aead2c257922771dab1fc8c78" +[[package]] +name = "registry" +version = "0.1.0" +dependencies = [ + "parking_lot", + "prost", + "proto-registry", + "tokio", + "tonic", + "tonic-build", + "tracing", + "tracing-subscriber", +] + [[package]] name = "reqwest" version = "0.11.18" diff --git a/Cargo.toml b/Cargo.toml index 9440fc4d..50642f39 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ members = [ "examples/common", "keyvalue", "proto.rs", + "registry" ] exclude = [] @@ -55,6 +56,7 @@ chariott-common = { path = "./common/" } chariott-proto = { path = "./proto.rs/" } futures = { version = "0.3" } lazy_static = "1.4.0" +parking_lot = "0.12.1" prost = "0.11" prost-types = "0.11" regex = "1.7" diff --git a/registry/Cargo.toml b/registry/Cargo.toml new file mode 100644 index 00000000..2c289d42 --- /dev/null +++ b/registry/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "registry" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +prost = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +tonic = { workspace = true } +tracing = { version = "0.1" } +tracing-subscriber = { version = "0.3", features = ["env-filter"] } +parking_lot = { workspace = true } +proto-registry = { path = "./proto/"} + +[build-dependencies] +tonic-build = { workspace = true } \ No newline at end of file diff --git a/registry/proto/Cargo.toml b/registry/proto/Cargo.toml new file mode 100644 index 00000000..e748776e --- /dev/null +++ b/registry/proto/Cargo.toml @@ -0,0 +1,17 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +# SPDX-License-Identifier: MIT + +[package] +name = "proto-registry" +version = "0.1.0" +edition = "2021" +license = "MIT" + +[dependencies] +prost = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +tonic = { workspace = true } + +[build-dependencies] +tonic-build = { workspace = true } \ No newline at end of file diff --git a/registry/proto/build.rs b/registry/proto/build.rs new file mode 100644 index 00000000..34beaafd --- /dev/null +++ b/registry/proto/build.rs @@ -0,0 +1,8 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +fn main() -> Result<(), Box> { + tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; + Ok(()) +} \ No newline at end of file diff --git a/registry/proto/chariott/v1/chariott_registry.proto b/registry/proto/chariott/v1/chariott_registry.proto new file mode 100644 index 00000000..15f49745 --- /dev/null +++ b/registry/proto/chariott/v1/chariott_registry.proto @@ -0,0 +1,80 @@ +syntax = "proto3"; + +package chariott_registry; + +// The Registry service definition. +service Registry { + // Register + rpc Register(RegisterRequest) returns (RegisterResponse) {} + + // Unregister + //rpc Unregister(UnregisterRequest) returns (UnregisterResponse) {} + + // Discover fully qualified + rpc DiscoverService(DiscoverServiceRequest) returns (DiscoverServiceResponse) {} + + // Inspect (dumps the contents of the registry for now) + rpc Inspect(InspectRequest) returns (InspectResponse) {} + +} + +// Service definition +message ServiceMetadata { + string namespace = 1; + string name = 2; + string version = 3; + string uri = 4; + string communication_kind = 5; + string communication_reference = 6; +} + +// Register status +enum RegistrationStatus { + NEWLY_REGISTERED = 0; // An entry did not exist in the service registry for this + UPDATED = 1; // An entry already existed in the service registry and it has now been updated + COULD_NOT_REGISTER = 2; // TODO: Should we keep this, or just rely on Status from grpc call? Could fail for many reasons (i.e. policy, authz, system errors) +} + +enum UnregisterStatus { + UNREGISTERED = 0; // unregister was successful + COULD_NOT_UNREGISTER = 1; // TODO: Should we keep this, or just rely on Status from grpc call? Could fail for many reasons (i.e. not found, policy, authz, system errors) +} + +// The request with the service description +message RegisterRequest { + ServiceMetadata service = 1; +} + +// The response with a status code +message RegisterResponse { +} + +// The request with the service description +message UnregisterRequest { + ServiceMetadata service = 1; +} + +// The response with a status code +message UnregisterResponse { +} + +// The request with the service description +message DiscoverServiceRequest { + string namespace = 1; + string name = 2; + string version = 3; +} + +// The response with a status code +message DiscoverServiceResponse { + ServiceMetadata service = 1; +} + +// The request with the service description +message InspectRequest { +} + +// The response with a status code +message InspectResponse { + repeated ServiceMetadata services = 1; +} diff --git a/registry/proto/src/lib.rs b/registry/proto/src/lib.rs new file mode 100644 index 00000000..efbf6da6 --- /dev/null +++ b/registry/proto/src/lib.rs @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +pub mod chariott_registry { + pub mod v1 { + #![allow(clippy::derive_partial_eq_without_eq)] + tonic::include_proto!("chariott_registry"); + } +} \ No newline at end of file diff --git a/registry/src/main.rs b/registry/src/main.rs new file mode 100644 index 00000000..2caaceb5 --- /dev/null +++ b/registry/src/main.rs @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +use proto_registry::chariott_registry::v1::registry_server::RegistryServer; +use parking_lot::RwLock; +use tonic::transport::Server; +use std::net::SocketAddr; +use std::collections::HashMap; +use std::sync::Arc; + +use tracing_subscriber::util::SubscriberInitExt; +use tracing_subscriber::EnvFilter; + +mod registry_impl; + +const CHARIOTT_SERVICE_REGISTRY_ADDR: &str = "[::1]:50000"; + +#[tokio::main] +async fn main() -> Result<(), Box> { + // start up registry server + + + // Set up tracing + let collector = tracing_subscriber::fmt() + .with_env_filter( + EnvFilter::builder() + .with_default_directive(tracing::Level::INFO.into()) + .from_env_lossy(), + ) + .finish(); + + collector.init(); + + let addr: SocketAddr = CHARIOTT_SERVICE_REGISTRY_ADDR.parse()?; + let registry_impl = registry_impl::RegistryImpl { + registry_map: Arc::new(RwLock::new(HashMap::new())), + }; + + let server_future = Server::builder().add_service(RegistryServer::new(registry_impl)).serve(addr); + + server_future.await?; + println!("Helloworld!"); + Ok(()) +} diff --git a/registry/src/registry_impl.rs b/registry/src/registry_impl.rs new file mode 100644 index 00000000..02a4bbd7 --- /dev/null +++ b/registry/src/registry_impl.rs @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +use proto_registry::chariott_registry::v1::registry_server::Registry; +use proto_registry::chariott_registry::v1::{DiscoverServiceRequest, DiscoverServiceResponse, InspectRequest, InspectResponse, RegisterRequest, RegisterResponse, ServiceMetadata}; +use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; +use std::{collections::HashMap}; +use std::sync::Arc; +use tonic::{Status, Request, Response}; + +#[derive(Clone, Debug)] +pub struct RegistryImpl { + pub registry_map: Arc>>, +} + +#[tonic::async_trait] +impl Registry for RegistryImpl { + async fn register( + &self, + request: Request, + ) -> Result, Status> { + let request_inner = request.into_inner(); + let service_to_register = request_inner.service.ok_or_else(|| Status::invalid_argument("service is required"))?; + let service_identifiers = ServiceIdentifiers { + namespace: service_to_register.namespace.clone(), + name: service_to_register.name.clone(), + version: service_to_register.version.clone() + }; + + // This block controls the lifetime of the lock. + { + let mut lock: RwLockWriteGuard> = + self.registry_map.write(); + match lock.get(&service_identifiers) { + Some(_) => { + // TODO: Add log that we overwrote the value + lock.insert(service_identifiers.clone(), service_to_register.clone()); + } + None => { + // Add log that we added a new entry + lock.insert(service_identifiers.clone(), service_to_register.clone()); + } + }; + } + + + let register_response = RegisterResponse { }; + Ok(Response::new(register_response)) + } + + async fn discover_service( + &self, + request: Request, + ) -> Result, Status> { + let request_inner = request.into_inner(); + + // TODO: check that all of them are included? + let service_identifiers = ServiceIdentifiers { + namespace: request_inner.namespace.clone(), + name: request_inner.name.clone(), + version: request_inner.version.clone() + }; + + // This block controls the lifetime of the lock. + { + let lock: RwLockReadGuard> = + self.registry_map.read(); + match lock.get(&service_identifiers) { + Some(service) => { + // TODO: add log that we read it + let discover_service_response = DiscoverServiceResponse { service: Some(service.clone()) }; + Ok(Response::new(discover_service_response)) + } + None => { + // TODO: add log that it was not found + Err(Status::not_found(format!("No service found for namespace: {0}, name: {1}, version: {2}", service_identifiers.namespace, service_identifiers.name, service_identifiers.version))) + } + } + } + } + + async fn inspect( + &self, + _request: Request, + ) -> Result, Status> { + // This block controls the lifetime of the lock. + { + let lock: RwLockReadGuard> = + self.registry_map.read(); + // transfer ownership to services_list, map can't be used anymore + let services_list = lock.values().cloned().collect(); + let inspect_response = InspectResponse { services: services_list }; + // TODO: Does all of this need to be in the lock block? + // TODO: errors??? + Ok(Response::new(inspect_response)) + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct ServiceIdentifiers { + namespace: String, + name: String, + version: String, +} + +#[cfg(test)] +mod registry_impl_test { + #[test] + fn test_equality() { + let one = 1; + assert_eq!(one, 1); + } +} From f60af0c05a9796ea5c6899cd4797383481a440fe Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Wed, 21 Jun 2023 11:51:46 -0400 Subject: [PATCH 02/13] Service Discovery First Iteration --- Cargo.lock | 55 ++- Cargo.toml | 4 +- .../proto/chariott/v1/chariott_registry.proto | 80 ---- registry/src/main.rs | 45 -- registry/src/registry_impl.rs | 115 ----- .../core}/Cargo.toml | 4 +- service_discovery/core/src/main.rs | 52 ++ service_discovery/core/src/registry_impl.rs | 443 ++++++++++++++++++ .../proto/build.rs | 1 + .../proto/chariott/v1/chariott_registry.proto | 98 ++++ .../samples/v1/hello_world_service.proto | 29 ++ .../proto/src/lib.rs | 7 + .../proto_build}/Cargo.toml | 2 +- service_discovery/proto_build/build.rs | 9 + service_discovery/proto_build/src/lib.rs | 17 + .../simple-discovery/consumer/Cargo.toml | 14 + .../simple-discovery/consumer/src/main.rs | 72 +++ .../simple-discovery/provider/Cargo.toml | 15 + .../provider/src/hello_world_impl.rs | 41 ++ .../simple-discovery/provider/src/main.rs | 82 ++++ 20 files changed, 926 insertions(+), 259 deletions(-) delete mode 100644 registry/proto/chariott/v1/chariott_registry.proto delete mode 100644 registry/src/main.rs delete mode 100644 registry/src/registry_impl.rs rename {registry => service_discovery/core}/Cargo.toml (85%) create mode 100644 service_discovery/core/src/main.rs create mode 100644 service_discovery/core/src/registry_impl.rs rename {registry => service_discovery}/proto/build.rs (75%) create mode 100644 service_discovery/proto/chariott/v1/chariott_registry.proto create mode 100644 service_discovery/proto/samples/v1/hello_world_service.proto rename {registry => service_discovery}/proto/src/lib.rs (64%) rename {registry/proto => service_discovery/proto_build}/Cargo.toml (91%) create mode 100644 service_discovery/proto_build/build.rs create mode 100644 service_discovery/proto_build/src/lib.rs create mode 100644 service_discovery/samples/simple-discovery/consumer/Cargo.toml create mode 100644 service_discovery/samples/simple-discovery/consumer/src/main.rs create mode 100644 service_discovery/samples/simple-discovery/provider/Cargo.toml create mode 100644 service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs create mode 100644 service_discovery/samples/simple-discovery/provider/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 40162c19..80092671 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -587,6 +587,18 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" +[[package]] +name = "consumer" +version = "0.1.0" +dependencies = [ + "chariott-common", + "proto-servicediscovery", + "tokio", + "tonic", + "tracing", + "tracing-subscriber", +] + [[package]] name = "core-foundation" version = "0.9.3" @@ -2291,7 +2303,7 @@ dependencies = [ ] [[package]] -name = "proto-registry" +name = "proto-servicediscovery" version = "0.1.0" dependencies = [ "prost", @@ -2306,6 +2318,19 @@ version = "2.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf7e6d18738ecd0902d30d1ad232c9125985a3422929b16c65517b38adc14f96" +[[package]] +name = "provider" +version = "0.1.0" +dependencies = [ + "chariott-common", + "proto-servicediscovery", + "tokio", + "tonic", + "tracing", + "tracing-subscriber", + "url", +] + [[package]] name = "qoi" version = "0.4.1" @@ -2477,20 +2502,6 @@ version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "436b050e76ed2903236f032a59761c1eb99e1b0aead2c257922771dab1fc8c78" -[[package]] -name = "registry" -version = "0.1.0" -dependencies = [ - "parking_lot", - "prost", - "proto-registry", - "tokio", - "tonic", - "tonic-build", - "tracing", - "tracing-subscriber", -] - [[package]] name = "reqwest" version = "0.11.18" @@ -2672,6 +2683,20 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "service_discovery" +version = "0.1.0" +dependencies = [ + "parking_lot", + "prost", + "proto-servicediscovery", + "tokio", + "tonic", + "tonic-build", + "tracing", + "tracing-subscriber", +] + [[package]] name = "sha1" version = "0.10.5" diff --git a/Cargo.toml b/Cargo.toml index 50642f39..837ca0a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,9 @@ members = [ "examples/common", "keyvalue", "proto.rs", - "registry" + "service_discovery/core", + "service_discovery/samples/simple-discovery/consumer", + "service_discovery/samples/simple-discovery/provider" ] exclude = [] diff --git a/registry/proto/chariott/v1/chariott_registry.proto b/registry/proto/chariott/v1/chariott_registry.proto deleted file mode 100644 index 15f49745..00000000 --- a/registry/proto/chariott/v1/chariott_registry.proto +++ /dev/null @@ -1,80 +0,0 @@ -syntax = "proto3"; - -package chariott_registry; - -// The Registry service definition. -service Registry { - // Register - rpc Register(RegisterRequest) returns (RegisterResponse) {} - - // Unregister - //rpc Unregister(UnregisterRequest) returns (UnregisterResponse) {} - - // Discover fully qualified - rpc DiscoverService(DiscoverServiceRequest) returns (DiscoverServiceResponse) {} - - // Inspect (dumps the contents of the registry for now) - rpc Inspect(InspectRequest) returns (InspectResponse) {} - -} - -// Service definition -message ServiceMetadata { - string namespace = 1; - string name = 2; - string version = 3; - string uri = 4; - string communication_kind = 5; - string communication_reference = 6; -} - -// Register status -enum RegistrationStatus { - NEWLY_REGISTERED = 0; // An entry did not exist in the service registry for this - UPDATED = 1; // An entry already existed in the service registry and it has now been updated - COULD_NOT_REGISTER = 2; // TODO: Should we keep this, or just rely on Status from grpc call? Could fail for many reasons (i.e. policy, authz, system errors) -} - -enum UnregisterStatus { - UNREGISTERED = 0; // unregister was successful - COULD_NOT_UNREGISTER = 1; // TODO: Should we keep this, or just rely on Status from grpc call? Could fail for many reasons (i.e. not found, policy, authz, system errors) -} - -// The request with the service description -message RegisterRequest { - ServiceMetadata service = 1; -} - -// The response with a status code -message RegisterResponse { -} - -// The request with the service description -message UnregisterRequest { - ServiceMetadata service = 1; -} - -// The response with a status code -message UnregisterResponse { -} - -// The request with the service description -message DiscoverServiceRequest { - string namespace = 1; - string name = 2; - string version = 3; -} - -// The response with a status code -message DiscoverServiceResponse { - ServiceMetadata service = 1; -} - -// The request with the service description -message InspectRequest { -} - -// The response with a status code -message InspectResponse { - repeated ServiceMetadata services = 1; -} diff --git a/registry/src/main.rs b/registry/src/main.rs deleted file mode 100644 index 2caaceb5..00000000 --- a/registry/src/main.rs +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -// SPDX-License-Identifier: MIT - -use proto_registry::chariott_registry::v1::registry_server::RegistryServer; -use parking_lot::RwLock; -use tonic::transport::Server; -use std::net::SocketAddr; -use std::collections::HashMap; -use std::sync::Arc; - -use tracing_subscriber::util::SubscriberInitExt; -use tracing_subscriber::EnvFilter; - -mod registry_impl; - -const CHARIOTT_SERVICE_REGISTRY_ADDR: &str = "[::1]:50000"; - -#[tokio::main] -async fn main() -> Result<(), Box> { - // start up registry server - - - // Set up tracing - let collector = tracing_subscriber::fmt() - .with_env_filter( - EnvFilter::builder() - .with_default_directive(tracing::Level::INFO.into()) - .from_env_lossy(), - ) - .finish(); - - collector.init(); - - let addr: SocketAddr = CHARIOTT_SERVICE_REGISTRY_ADDR.parse()?; - let registry_impl = registry_impl::RegistryImpl { - registry_map: Arc::new(RwLock::new(HashMap::new())), - }; - - let server_future = Server::builder().add_service(RegistryServer::new(registry_impl)).serve(addr); - - server_future.await?; - println!("Helloworld!"); - Ok(()) -} diff --git a/registry/src/registry_impl.rs b/registry/src/registry_impl.rs deleted file mode 100644 index 02a4bbd7..00000000 --- a/registry/src/registry_impl.rs +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -// SPDX-License-Identifier: MIT - -use proto_registry::chariott_registry::v1::registry_server::Registry; -use proto_registry::chariott_registry::v1::{DiscoverServiceRequest, DiscoverServiceResponse, InspectRequest, InspectResponse, RegisterRequest, RegisterResponse, ServiceMetadata}; -use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; -use std::{collections::HashMap}; -use std::sync::Arc; -use tonic::{Status, Request, Response}; - -#[derive(Clone, Debug)] -pub struct RegistryImpl { - pub registry_map: Arc>>, -} - -#[tonic::async_trait] -impl Registry for RegistryImpl { - async fn register( - &self, - request: Request, - ) -> Result, Status> { - let request_inner = request.into_inner(); - let service_to_register = request_inner.service.ok_or_else(|| Status::invalid_argument("service is required"))?; - let service_identifiers = ServiceIdentifiers { - namespace: service_to_register.namespace.clone(), - name: service_to_register.name.clone(), - version: service_to_register.version.clone() - }; - - // This block controls the lifetime of the lock. - { - let mut lock: RwLockWriteGuard> = - self.registry_map.write(); - match lock.get(&service_identifiers) { - Some(_) => { - // TODO: Add log that we overwrote the value - lock.insert(service_identifiers.clone(), service_to_register.clone()); - } - None => { - // Add log that we added a new entry - lock.insert(service_identifiers.clone(), service_to_register.clone()); - } - }; - } - - - let register_response = RegisterResponse { }; - Ok(Response::new(register_response)) - } - - async fn discover_service( - &self, - request: Request, - ) -> Result, Status> { - let request_inner = request.into_inner(); - - // TODO: check that all of them are included? - let service_identifiers = ServiceIdentifiers { - namespace: request_inner.namespace.clone(), - name: request_inner.name.clone(), - version: request_inner.version.clone() - }; - - // This block controls the lifetime of the lock. - { - let lock: RwLockReadGuard> = - self.registry_map.read(); - match lock.get(&service_identifiers) { - Some(service) => { - // TODO: add log that we read it - let discover_service_response = DiscoverServiceResponse { service: Some(service.clone()) }; - Ok(Response::new(discover_service_response)) - } - None => { - // TODO: add log that it was not found - Err(Status::not_found(format!("No service found for namespace: {0}, name: {1}, version: {2}", service_identifiers.namespace, service_identifiers.name, service_identifiers.version))) - } - } - } - } - - async fn inspect( - &self, - _request: Request, - ) -> Result, Status> { - // This block controls the lifetime of the lock. - { - let lock: RwLockReadGuard> = - self.registry_map.read(); - // transfer ownership to services_list, map can't be used anymore - let services_list = lock.values().cloned().collect(); - let inspect_response = InspectResponse { services: services_list }; - // TODO: Does all of this need to be in the lock block? - // TODO: errors??? - Ok(Response::new(inspect_response)) - } - } -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct ServiceIdentifiers { - namespace: String, - name: String, - version: String, -} - -#[cfg(test)] -mod registry_impl_test { - #[test] - fn test_equality() { - let one = 1; - assert_eq!(one, 1); - } -} diff --git a/registry/Cargo.toml b/service_discovery/core/Cargo.toml similarity index 85% rename from registry/Cargo.toml rename to service_discovery/core/Cargo.toml index 2c289d42..aa799763 100644 --- a/registry/Cargo.toml +++ b/service_discovery/core/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "registry" +name = "service_discovery" version = "0.1.0" edition = "2021" @@ -12,7 +12,7 @@ tonic = { workspace = true } tracing = { version = "0.1" } tracing-subscriber = { version = "0.3", features = ["env-filter"] } parking_lot = { workspace = true } -proto-registry = { path = "./proto/"} +proto-servicediscovery = { path = "../proto_build/"} [build-dependencies] tonic-build = { workspace = true } \ No newline at end of file diff --git a/service_discovery/core/src/main.rs b/service_discovery/core/src/main.rs new file mode 100644 index 00000000..ae88a31c --- /dev/null +++ b/service_discovery/core/src/main.rs @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +//! Project Eclipse Chariott Service Discovery +//! +//! This is the Service Discovery system for Chariott. It includes a service registry, +//! which is a database of services that are currently registered. Other applications +//! can find the metadata for registered services. + +// Tells cargo to warn if a doc comment is missing and should be provided. +#![warn(missing_docs)] + +use parking_lot::RwLock; +use proto_servicediscovery::chariott_registry::v1::registry_server::RegistryServer; +use std::collections::HashMap; +use std::net::SocketAddr; +use std::sync::Arc; +use tonic::transport::Server; +use tracing::{debug, info}; +use tracing_subscriber::util::SubscriberInitExt; +use tracing_subscriber::EnvFilter; + +mod registry_impl; + +/// Endpoint for the chariott service registry +const CHARIOTT_SERVICE_REGISTRY_ADDR: &str = "0.0.0.0:50000"; + +#[tokio::main] +async fn main() -> Result<(), Box> { + // Set up tracing + let collector = tracing_subscriber::fmt() + .with_env_filter( + EnvFilter::builder() + .with_default_directive(tracing::Level::INFO.into()) + .from_env_lossy(), + ) + .finish(); + + collector.init(); + + // Start up registry service + let addr: SocketAddr = CHARIOTT_SERVICE_REGISTRY_ADDR.parse()?; + let registry_impl = + registry_impl::RegistryImpl { registry_map: Arc::new(RwLock::new(HashMap::new())) }; + info!("Chariott Registry listening on {addr}"); + + Server::builder().add_service(RegistryServer::new(registry_impl)).serve(addr).await?; + + debug!("The Chariott Registry has completed."); + Ok(()) +} diff --git a/service_discovery/core/src/registry_impl.rs b/service_discovery/core/src/registry_impl.rs new file mode 100644 index 00000000..660f8b34 --- /dev/null +++ b/service_discovery/core/src/registry_impl.rs @@ -0,0 +1,443 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +//! Module containing gRPC service implementation based on [`proto_servicediscovery::chariott_registry::v1`]. +//! +//! Provides a gRPC endpoint for external services to interact with to register and discover +//! services. Note: Identifiers in all Registry operations are case-sensitive. +//! +use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; +use proto_servicediscovery::chariott_registry::v1::registry_server::Registry; +use proto_servicediscovery::chariott_registry::v1::{ + DiscoverByNamespaceRequest, DiscoverByNamespaceResponse, DiscoverServiceRequest, + DiscoverServiceResponse, InspectRequest, InspectResponse, RegisterRequest, RegisterResponse, + RegistrationStatus, ServiceMetadata, UnregisterRequest, UnregisterResponse +}; +use std::collections::HashMap; +use std::sync::Arc; +use std::vec::Vec; +use tonic::{Request, Response, Status}; +use tracing::{info, warn}; + +/// Base structure for the registry gRPC service +#[derive(Clone, Debug)] +pub struct RegistryImpl { + pub registry_map: Arc>>, +} + +#[tonic::async_trait] +impl Registry for RegistryImpl { + /// Registers a service by adding it to the registry. + /// + /// This function registers a service based on a [`RegisterRequest`]. Returns a + /// [`RegisterResponse`]. + /// + /// # Arguments + /// + /// * `request` - A [`RegisterRequest`] wrapped by a [`tonic::Request`]. + async fn register( + &self, + request: Request, + ) -> Result, Status> { + let request_inner = request.into_inner(); + let service_to_register = + request_inner.service.ok_or_else(|| Status::invalid_argument("Service is required"))?; + let service_identifiers = ServiceIdentifiers { + namespace: service_to_register.namespace.clone(), + name: service_to_register.name.clone(), + version: service_to_register.version.clone(), + }; + info!("Received a register request for: {:?}", service_identifiers); + + let registration_status; + // This block controls the lifetime of the lock. + { + let mut lock: RwLockWriteGuard> = + self.registry_map.write(); + match lock.get(&service_identifiers) { + Some(_) => { + lock.insert(service_identifiers.clone(), service_to_register.clone()); + registration_status = RegistrationStatus::Updated; + info!("Updated the service entry in Chariott service registry; overwrote previous entry: {:?}", service_to_register); + } + None => { + lock.insert(service_identifiers.clone(), service_to_register.clone()); + registration_status = RegistrationStatus::NewlyRegistered; + info!( + "Registered new service in Chariott service registry: {:?}", + service_to_register + ); + } + }; + } + let register_response = + RegisterResponse { registration_status: registration_status as i32 }; + Ok(Response::new(register_response)) + } + + /// Unregisters a service by removing it from the registry. + /// + /// This function registers a service based on a [`UnregisterRequest`]. Returns a + /// [`UnregisterResponse`]. + /// + /// # Arguments + /// + /// * `request` - A [`UnregisterRequest`] wrapped by a [`tonic::Request`]. + async fn unregister( + &self, + request: Request, + ) -> Result, Status> { + let request_inner = request.into_inner(); + let service_identifiers = ServiceIdentifiers { + namespace: request_inner.namespace, + name: request_inner.name, + version: request_inner.version + }; + info!("Received an unregister request for: {:?}", service_identifiers); + + // This block controls the lifetime of the lock. + { + let mut lock: RwLockWriteGuard> = + self.registry_map.write(); + match lock.remove(&service_identifiers) { + Some(removed_service) => { + info!("Successfully removed service entry in Chariott service registry: {:?}", removed_service); + } + None => { + let not_found_message = format!("Unable to remove service from registry: {:?}", service_identifiers); + warn!(not_found_message); //namespace: {0}, name: {1}, version: {2}") + return Err(Status::not_found(not_found_message)); + } + }; + } + Ok(Response::new(UnregisterResponse{})) + } + + /// Discovers a list of services based on the namespace, or logical grouping of services. + /// + /// This function discovers a list of services based on a [`DiscoverByNamespaceRequest`]. Returns a + /// [`DiscoverByNamespaceResponse`]. + /// + /// # Arguments + /// + /// * `request` - A [`DiscoverByNamespaceRequest`] wrapped by a [`tonic::Request`]. + async fn discover_by_namespace( + &self, + request: Request, + ) -> Result, Status> { + let namespace = request.into_inner().namespace; + let mut service_list: Vec = Vec::new(); + + // This block controls the lifetime of the lock. + { + let lock: RwLockReadGuard> = + self.registry_map.read(); + for (service_identifier, service_metadata) in lock.iter() { + if service_identifier.namespace == namespace { + service_list.push(service_metadata.clone()); + } + } + } + if service_list.is_empty() { + Err(Status::not_found(format!("No registrations found for namespace {namespace}"))) + } else { + let discover_by_namespace_response = + DiscoverByNamespaceResponse { services: service_list }; + Ok(Response::new(discover_by_namespace_response)) + } + } + + /// Discovers a single service based on its "fully qualified name", consisting of the namespace, + /// name, and version of the service. + /// + /// This function discovers a service based on a [`DiscoverServiceRequest`]. Returns a + /// [`DiscoverServiceResponse`]. + /// + /// # Arguments + /// + /// * `request` - A [`DiscoverServiceRequest`] wrapped by a [`tonic::Request`]. + async fn discover_service( + &self, + request: Request, + ) -> Result, Status> { + let request_inner = request.into_inner(); + + let service_identifiers = ServiceIdentifiers { + namespace: request_inner.namespace.clone(), + name: request_inner.name.clone(), + version: request_inner.version.clone(), + }; + + // This block controls the lifetime of the lock. + { + let lock: RwLockReadGuard> = + self.registry_map.read(); + match lock.get(&service_identifiers) { + Some(service) => { + info!("Read service in DiscoverService {:?}", service); + let discover_service_response = + DiscoverServiceResponse { service: Some(service.clone()) }; + Ok(Response::new(discover_service_response)) + } + None => { + let not_found_message = format!( + "No service found for namespace: {0}, name: {1}, version: {2}", + service_identifiers.namespace, + service_identifiers.name, + service_identifiers.version + ); + warn!(not_found_message); + Err(Status::not_found(not_found_message)) + } + } + } + } + + /// Inspects the contents of the service registry. + /// + /// This function retrieves all services currently registered based on an [`InspectRequest`]. Returns a + /// [`InspectResponse`]. + /// + /// # Arguments + /// + /// * `request` - A [`InspectRequest`] wrapped by a [`tonic::Request`]. + async fn inspect( + &self, + _request: Request, + ) -> Result, Status> { + let lock: RwLockReadGuard> = + self.registry_map.read(); + let services_list = lock.values().cloned().collect(); + let inspect_response = InspectResponse { services: services_list }; + Ok(Response::new(inspect_response)) + } +} + +/// Identifiers for a given service. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct ServiceIdentifiers { + /// namespace represents a logical grouping of services + namespace: String, + /// the service name (without the namespace) + name: String, + /// the version of the service + version: String, +} + + +#[cfg(test)] +mod registry_impl_test { + use super::*; + + fn has_service( + registry_map: Arc>>, + key: &ServiceIdentifiers, + ) -> bool { + // This block controls the lifetime of the lock. + { + let lock: RwLockReadGuard> = + registry_map.read(); + lock.contains_key(key) + } + } + + #[tokio::test] + async fn register_test() { + // Test creating a new registration + let mut service1 = ServiceMetadata { + namespace: String::from("sdv.test"), + name: String::from("test_service"), + version: String::from("1.0.0.0"), + uri: String::from("localhost:1000"), + communication_kind: String::from("grpc+proto"), + communication_reference: String::from("sdv.test.test_service.v1.proto"), + }; + + let registry_map = Arc::new(RwLock::new(HashMap::new())); + let registry_impl = RegistryImpl { registry_map: registry_map }; + let request = tonic::Request::new(RegisterRequest { service: Some(service1.clone()) }); + let result = registry_impl.register(request).await; + assert!(result.is_ok(), "register result is not okay: {result:?}"); + assert_eq!( + result.unwrap().into_inner().registration_status.clone(), + RegistrationStatus::NewlyRegistered as i32 + ); + let service_identifiers = ServiceIdentifiers { + namespace: service1.namespace.clone(), + name: service1.name.clone(), + version: service1.version.clone(), + }; + assert!( + has_service(registry_impl.registry_map.clone(), &service_identifiers), + "service not present in registry" + ); + + // Test updating a registration + service1.uri = String::from("localhost:1001"); + let update_request = + tonic::Request::new(RegisterRequest { service: Some(service1.clone()) }); + let updated_result = registry_impl.register(update_request).await; + + assert!(updated_result.is_ok(), "register result is not okay: {updated_result:?}"); + assert_eq!( + updated_result.unwrap().into_inner().registration_status.clone(), + RegistrationStatus::Updated as i32 + ); + // This block controls the lifetime of the lock. + { + let lock: RwLockReadGuard> = + registry_impl.registry_map.read(); + let updated_service_result = lock.get(&service_identifiers); + assert_eq!(updated_service_result.unwrap().uri, String::from("localhost:1001")); + } + } + + #[tokio::test] + async fn unregister_test() { + let registry_map = Arc::new(RwLock::new(HashMap::new())); + + let service1 = ServiceMetadata { + namespace: String::from("sdv.test"), + name: String::from("test_service"), + version: String::from("1.0.0.0"), + uri: String::from("localhost:1000"), + communication_kind: String::from("grpc+proto"), + communication_reference: String::from("sdv.test.test_service.v1.proto"), + }; + let service_identifiers1 = ServiceIdentifiers { + namespace: service1.namespace.clone(), + name: service1.name.clone(), + version: service1.version.clone(), + }; + + // This block controls the lifetime of the lock. + { + let mut lock: RwLockWriteGuard> = + registry_map.write(); + lock.insert(service_identifiers1.clone(), service1.clone()); + } + + let registry_impl = RegistryImpl { registry_map: registry_map }; + + // Unregister Service + let request = tonic::Request::new(UnregisterRequest { + namespace: service_identifiers1.namespace.clone(), + name: service_identifiers1.name.clone(), + version: service_identifiers1.version.clone(), + }); + let result = registry_impl.unregister(request).await; + assert!(result.is_ok(), "Unregister result is not okay: {result:?}"); + + // Unregister Service that doesn't exist + let request2 = tonic::Request::new(UnregisterRequest { + namespace: service_identifiers1.namespace.clone(), + name: service_identifiers1.name.clone(), + version: service_identifiers1.version.clone(), + }); + let not_found_status = Status::not_found(format!("Unable to remove service from registry: {:?}", service_identifiers1)); + let result = registry_impl.unregister(request2).await.err().unwrap(); + assert_eq!(result.code(), not_found_status.code()); + assert_eq!(result.message(), not_found_status.message()); + } + + #[tokio::test] + async fn discover_test() { + let registry_map = Arc::new(RwLock::new(HashMap::new())); + + let service1 = ServiceMetadata { + namespace: String::from("sdv.test"), + name: String::from("test_service"), + version: String::from("1.0.0.0"), + uri: String::from("localhost:1000"), + communication_kind: String::from("grpc+proto"), + communication_reference: String::from("sdv.test.test_service.v1.proto"), + }; + let service_identifiers1 = ServiceIdentifiers { + namespace: service1.namespace.clone(), + name: service1.name.clone(), + version: service1.version.clone(), + }; + + // This block controls the lifetime of the lock. + { + let mut lock: RwLockWriteGuard> = + registry_map.write(); + lock.insert(service_identifiers1.clone(), service1.clone()); + } + + let registry_impl = RegistryImpl { registry_map: registry_map }; + + // Discover Service + let request = tonic::Request::new(DiscoverServiceRequest { + namespace: service_identifiers1.namespace.clone(), + name: service_identifiers1.name.clone(), + version: service_identifiers1.version.clone(), + }); + let result = registry_impl.discover_service(request).await; + assert!(result.is_ok(), "DiscoverService result is not okay: {result:?}"); + assert_eq!(result.unwrap().into_inner().service, Some(service1.clone())); + + // Discover by namespace + let request_namespace = tonic::Request::new(DiscoverByNamespaceRequest { + namespace: service_identifiers1.namespace.clone(), + }); + let result_namespace = registry_impl.discover_by_namespace(request_namespace).await; + assert!( + result_namespace.is_ok(), + "DiscoverByNamespace result is not okay: {result_namespace:?}" + ); + assert_eq!(result_namespace.unwrap().into_inner().services[0], service1.clone()); + } + + #[tokio::test] + async fn inspect_test() { + let registry_map = Arc::new(RwLock::new(HashMap::new())); + + let service1 = ServiceMetadata { + namespace: String::from("sdv.test"), + name: String::from("test_service"), + version: String::from("1.0.0.0"), + uri: String::from("localhost:1000"), + communication_kind: String::from("grpc+proto"), + communication_reference: String::from("sdv.test.test_service.v1.proto"), + }; + let service2 = ServiceMetadata { + namespace: String::from("sdv.test"), + name: String::from("test_service"), + version: String::from("2.0.0.0"), + uri: String::from("localhost:2000"), + communication_kind: String::from("grpc+proto"), + communication_reference: String::from("sdv.test.test_service.v2.proto"), + }; + let service_identifiers1 = ServiceIdentifiers { + namespace: service1.namespace.clone(), + name: service1.name.clone(), + version: service1.version.clone(), + }; + let service_identifiers2 = ServiceIdentifiers { + namespace: service2.namespace.clone(), + name: service2.name.clone(), + version: service2.version.clone(), + }; + + // This block controls the lifetime of the lock. + { + let mut lock: RwLockWriteGuard> = + registry_map.write(); + lock.insert(service_identifiers1.clone(), service1.clone()); + lock.insert(service_identifiers2.clone(), service2.clone()); + } + + let registry_impl = RegistryImpl { registry_map: registry_map }; + + // Test that inspect returns the two services + let request = tonic::Request::new(InspectRequest {}); + let result = registry_impl.inspect(request).await; + assert!(result.is_ok(), "Inspect result is not okay: {result:?}"); + let result_services = result.unwrap().into_inner().services.clone(); + assert_eq!(result_services.len(), 2); + assert!(result_services.contains(&service1), "Service1 not present in the inspect response"); + assert!(result_services.contains(&service2), "Service2 not present in the inspect response"); + } +} diff --git a/registry/proto/build.rs b/service_discovery/proto/build.rs similarity index 75% rename from registry/proto/build.rs rename to service_discovery/proto/build.rs index 34beaafd..d58954c0 100644 --- a/registry/proto/build.rs +++ b/service_discovery/proto/build.rs @@ -4,5 +4,6 @@ fn main() -> Result<(), Box> { tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; + tonic_build::compile_protos("../proto/samples/v1/hello_world_service.proto")?; Ok(()) } \ No newline at end of file diff --git a/service_discovery/proto/chariott/v1/chariott_registry.proto b/service_discovery/proto/chariott/v1/chariott_registry.proto new file mode 100644 index 00000000..e017a825 --- /dev/null +++ b/service_discovery/proto/chariott/v1/chariott_registry.proto @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +syntax = "proto3"; +package chariott_registry; + +// Service Registry definition +// +// The protobuf definitions for Chariott's service registry + +// The entry point for the Registry gRPC Service. +service Registry { + // Register, or add a service to the registry + rpc Register(RegisterRequest) returns (RegisterResponse) {} + + // Unregister, or remove a service from the registry + rpc Unregister(UnregisterRequest) returns (UnregisterResponse) {} + + // Discover, or retrieve the metadata for a single service given its fully qualified name + rpc DiscoverService(DiscoverServiceRequest) returns (DiscoverServiceResponse) {} + + // Discover a list of services given their namespace + rpc DiscoverByNamespace(DiscoverByNamespaceRequest) returns (DiscoverByNamespaceResponse) {} + + // Inspect, or retrieve all contents of the service registry + rpc Inspect(InspectRequest) returns (InspectResponse) {} +} + +// Represenation of a service including all of its metadata that the registry stores +message ServiceMetadata { + string namespace = 1; + string name = 2; + string version = 3; + string uri = 4; + string communication_kind = 5; + string communication_reference = 6; +} + +// Status of a registration operation +enum RegistrationStatus { + // An entry did not exist in the service registry for this + NEWLY_REGISTERED = 0; + // An entry already existed in the service registry and it has now been updated (overwritten) + UPDATED = 1; +} + +// Request used to register a service, including all of its metadata +message RegisterRequest { + ServiceMetadata service = 1; +} + +// Response from `Register` which shows the status of the register operation +message RegisterResponse { + RegistrationStatus registration_status = 1; +} + +// Request used to unregister a service +message UnregisterRequest { + string namespace = 1; + string name = 2; + string version = 3; +} + +// Response from `Unregister` +message UnregisterResponse { +} + +// Request to retrieve the metadata for a service given its service identifiers +message DiscoverServiceRequest { + string namespace = 1; + string name = 2; + string version = 3; +} + +// Response including the single service's metadata +message DiscoverServiceResponse { + ServiceMetadata service = 1; +} + +// Request to retrieve the metadata for a list of services given their namespace +message DiscoverByNamespaceRequest { + string namespace = 1; +} + +// Repsonse with all services registered for this namespace +message DiscoverByNamespaceResponse { + repeated ServiceMetadata services = 1; +} + +// Request to inspect the registry, or retrieve all registered services +message InspectRequest { +} + +// Response with a list of all registered services +message InspectResponse { + repeated ServiceMetadata services = 1; +} diff --git a/service_discovery/proto/samples/v1/hello_world_service.proto b/service_discovery/proto/samples/v1/hello_world_service.proto new file mode 100644 index 00000000..f7371b34 --- /dev/null +++ b/service_discovery/proto/samples/v1/hello_world_service.proto @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +// Hello World Service sample definition +// +// The protobuf definitions for the Hello World service sample + +syntax = "proto3"; +package hello_world; + +// The service entry point to the Hello World service. This simple service only has one method to +// show the basic flow of applications calling one another +service HelloWorld { + // Method which just logs and returns a message "Hello, {input string}" when it is called + rpc SayHello(HelloRequest) returns (HelloResponse); +} + +// Representation of a request with the "name" or string that you would like to say hello to +message HelloRequest { + // The name, or string that will be returned as part of the hello message + string name = 1; +} + +// Representation of a response which includes "Hello, " the name provided in the request +message HelloResponse { + // The message that is returned: "Hello, " the name provided in the request + string message = 1; +} \ No newline at end of file diff --git a/registry/proto/src/lib.rs b/service_discovery/proto/src/lib.rs similarity index 64% rename from registry/proto/src/lib.rs rename to service_discovery/proto/src/lib.rs index efbf6da6..1c1cb268 100644 --- a/registry/proto/src/lib.rs +++ b/service_discovery/proto/src/lib.rs @@ -7,4 +7,11 @@ pub mod chariott_registry { #![allow(clippy::derive_partial_eq_without_eq)] tonic::include_proto!("chariott_registry"); } +} + +pub mod hello_world { + pub mod v1 { + #![allow(clippy::derive_partial_eq_without_eq)] + tonic::include_proto!("hello_world"); + } } \ No newline at end of file diff --git a/registry/proto/Cargo.toml b/service_discovery/proto_build/Cargo.toml similarity index 91% rename from registry/proto/Cargo.toml rename to service_discovery/proto_build/Cargo.toml index e748776e..6d664339 100644 --- a/registry/proto/Cargo.toml +++ b/service_discovery/proto_build/Cargo.toml @@ -3,7 +3,7 @@ # SPDX-License-Identifier: MIT [package] -name = "proto-registry" +name = "proto-servicediscovery" version = "0.1.0" edition = "2021" license = "MIT" diff --git a/service_discovery/proto_build/build.rs b/service_discovery/proto_build/build.rs new file mode 100644 index 00000000..d58954c0 --- /dev/null +++ b/service_discovery/proto_build/build.rs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +fn main() -> Result<(), Box> { + tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; + tonic_build::compile_protos("../proto/samples/v1/hello_world_service.proto")?; + Ok(()) +} \ No newline at end of file diff --git a/service_discovery/proto_build/src/lib.rs b/service_discovery/proto_build/src/lib.rs new file mode 100644 index 00000000..1c1cb268 --- /dev/null +++ b/service_discovery/proto_build/src/lib.rs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +pub mod chariott_registry { + pub mod v1 { + #![allow(clippy::derive_partial_eq_without_eq)] + tonic::include_proto!("chariott_registry"); + } +} + +pub mod hello_world { + pub mod v1 { + #![allow(clippy::derive_partial_eq_without_eq)] + tonic::include_proto!("hello_world"); + } +} \ No newline at end of file diff --git a/service_discovery/samples/simple-discovery/consumer/Cargo.toml b/service_discovery/samples/simple-discovery/consumer/Cargo.toml new file mode 100644 index 00000000..4fe20f8a --- /dev/null +++ b/service_discovery/samples/simple-discovery/consumer/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "consumer" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +chariott-common = { path = "../../../../common/" } +proto-servicediscovery = { path = "../../../proto_build/"} +tokio = { workspace = true, features = ["rt-multi-thread", "time"] } +tonic = { workspace = true } +tracing = { version = "0.1" } +tracing-subscriber = { version = "0.3", features = ["env-filter"] } \ No newline at end of file diff --git a/service_discovery/samples/simple-discovery/consumer/src/main.rs b/service_discovery/samples/simple-discovery/consumer/src/main.rs new file mode 100644 index 00000000..c2579bf4 --- /dev/null +++ b/service_discovery/samples/simple-discovery/consumer/src/main.rs @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +//! A simple consumer for a sample of Chariott Service Discovery. +//! +//! This consumer "discovers" the hello world service through Chariott, and then +//! directly calls the SayHello method on it, using a known interface. This returns +//! a message containing "Hello, " followed by the string provided in the request. + +// Tells cargo to warn if a doc comment is missing and should be provided. +#![warn(missing_docs)] + +use proto_servicediscovery::chariott_registry::v1::{DiscoverServiceRequest}; +use proto_servicediscovery::chariott_registry::v1::registry_client::RegistryClient; +use proto_servicediscovery::hello_world::v1::hello_world_client::HelloWorldClient; +use proto_servicediscovery::hello_world::v1::HelloRequest; +use tonic::Request; +use tracing::info; +use tracing_subscriber::EnvFilter; +use tracing_subscriber::util::SubscriberInitExt; + +/// URL for the chariott service registry +const CHARIOTT_SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; + +#[tokio::main] +async fn main() -> Result<(), Box> { + // Set up tracing + let collector = tracing_subscriber::fmt() + .with_env_filter( + EnvFilter::builder() + .with_default_directive(tracing::Level::INFO.into()) + .from_env_lossy(), + ) + .finish(); + + collector.init(); + + // Create a registry client + let mut registry_client = RegistryClient::connect(CHARIOTT_SERVICE_REGISTRY_URL).await?; + let discover_request = Request::new(DiscoverServiceRequest { + namespace: String::from("sdv.samples"), + name: String::from("hello-world"), + version: String::from("1.0.0.0") + }); + + // Discover the simple provider service + let service_option = registry_client + .discover_service(discover_request) + .await? + .into_inner() + .service; + match service_option { + Some(service) => + { + info!("Discovered service {:?}", service); + if service.communication_kind != String::from("grpc+proto") || service.communication_reference != String::from("hello_world_service.v1.proto") { + return Err("Simple Discover Consumer does not recognize communication_kind or communication_reference of provider; cannot communicate")?; + } + + // Call the provider application directly, since we recognize the communication kind and reference + let mut provider_client = HelloWorldClient::connect(service.uri).await?; + let hello_request = Request::new(HelloRequest { + name: String::from("World") + }); + let hello_response = provider_client.say_hello(hello_request).await?.into_inner(); + info!(hello_response.message); + }, + None => info!("No service found.") + }; + Ok(()) +} diff --git a/service_discovery/samples/simple-discovery/provider/Cargo.toml b/service_discovery/samples/simple-discovery/provider/Cargo.toml new file mode 100644 index 00000000..6e047da0 --- /dev/null +++ b/service_discovery/samples/simple-discovery/provider/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "provider" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +chariott-common = { path = "../../../../common/" } +proto-servicediscovery = { path = "../../../proto_build/"} +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +tonic = { workspace = true } +tracing = { version = "0.1" } +tracing-subscriber = { version = "0.3", features = ["env-filter"] } +url = { workspace = true } \ No newline at end of file diff --git a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs new file mode 100644 index 00000000..f5fb71ad --- /dev/null +++ b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +//! Module containing gRPC service implementation based on [`proto_servicediscovery::hello_world::v1`]. +//! +//! Provides a gRPC endpoint for external services to call a hello world method. + +use proto_servicediscovery::hello_world::v1::hello_world_server::HelloWorld; +use proto_servicediscovery::hello_world::v1::{HelloRequest, HelloResponse}; +use tonic::{Request, Response, Status}; +use tracing::info; + +/// Base structure for the Hello World gRPC service. +#[derive(Default)] +pub struct HelloWorldImpl {} + +#[tonic::async_trait] +impl HelloWorld for HelloWorldImpl { + /// Says Hello, followed by the input string + /// This function returns a message which says Hello, followed by the string + /// provided in a [`HelloRequest`]. Returns a [`HelloResponse`] + /// + /// # Arguments + /// + /// * `request` - A [`HelloRequest`] wrapped by a [`tonic::Request`]. + async fn say_hello( + &self, + request: Request, + ) -> Result, Status> { + let request_inner = request.into_inner(); + let name = request_inner.name.clone(); + let message = format!("Hello, {name}"); + let hello_response = HelloResponse { + message: message.clone() + }; + info!("Sent message: {message}"); + + Ok(Response::new(hello_response)) + } +} \ No newline at end of file diff --git a/service_discovery/samples/simple-discovery/provider/src/main.rs b/service_discovery/samples/simple-discovery/provider/src/main.rs new file mode 100644 index 00000000..c5e287a4 --- /dev/null +++ b/service_discovery/samples/simple-discovery/provider/src/main.rs @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// SPDX-License-Identifier: MIT + +//! A simple provider for a sample of Chariott Service Discovery. +//! +//! This provider has one service, the hello_world service, which has one +//! method that returns a message containing "Hello, " followed by the string +//! provided in the request. The provider registers itself with Chariott. + +// Tells cargo to warn if a doc comment is missing and should be provided. +#![warn(missing_docs)] + +use chariott_common::error::Error; +use hello_world_impl::HelloWorldImpl; +use proto_servicediscovery::hello_world::v1::hello_world_server::HelloWorldServer; +use std::net::SocketAddr; +use url::Url; + +use proto_servicediscovery::chariott_registry::v1::{RegisterRequest, ServiceMetadata}; +use proto_servicediscovery::chariott_registry::v1::registry_client::RegistryClient; +use tonic::{Request}; +use tonic::transport::{Server}; +use tracing::{info}; +use tracing_subscriber::EnvFilter; +use tracing_subscriber::util::SubscriberInitExt; + +mod hello_world_impl; + +/// URL for the chariott service registry +const CHARIOTT_SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; +/// Endpoint for the hello world service, which is also a provider +const HELLO_WORLD_ENDPOINT: &str = "0.0.0.0:50064"; + +#[tokio::main] +async fn main() -> Result<(), Box> { + // Set up tracing + let collector = tracing_subscriber::fmt() + .with_env_filter( + EnvFilter::builder() + .with_default_directive(tracing::Level::INFO.into()) + .from_env_lossy(), + ) + .finish(); + + collector.init(); + + // Intitialize addresses for provider and chariott communication. + let provider_url_str = format!("http://{}", HELLO_WORLD_ENDPOINT); + let socket_address: SocketAddr = HELLO_WORLD_ENDPOINT + .clone() + .parse() + .map_err(|e| Error::from_error("error getting SocketAddr", Box::new(e)))?; + let _provider_url: Url = Url::parse(&provider_url_str) + .map_err(|e| Error::from_error("error getting Url", Box::new(e)))?; + + let service_metadata: ServiceMetadata = ServiceMetadata { + namespace: "sdv.samples".to_string(), + name: "hello-world".to_string(), + version: "1.0.0.0".to_string(), + uri: provider_url_str.clone(), + communication_kind: String::from("grpc+proto"), + communication_reference: String::from("hello_world_service.v1.proto") + }; + + let mut registry_client = RegistryClient::connect(CHARIOTT_SERVICE_REGISTRY_URL).await?; + + let register_request = Request::new(RegisterRequest { + service: Some(service_metadata) + }); + registry_client.register(register_request).await?.into_inner(); + info!("Hello World Service registered as a Chariott provider"); + + let hello_world_impl = HelloWorldImpl::default(); + // Grpc server for handling calls from clients + Server::builder() + .add_service(HelloWorldServer::new(hello_world_impl)) + .serve(socket_address) + .await?; + Ok(()) + +} From 61bd3027cb63f046e18080fd960f0073fb9143a6 Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Wed, 21 Jun 2023 12:53:17 -0400 Subject: [PATCH 03/13] Static code check fixes --- service_discovery/core/Cargo.toml | 2 +- service_discovery/core/src/registry_impl.rs | 54 ++++++++++++------- service_discovery/proto/build.rs | 2 +- .../proto/chariott/v1/chariott_registry.proto | 4 +- .../samples/v1/hello_world_service.proto | 6 +-- service_discovery/proto/src/lib.rs | 2 +- service_discovery/proto_build/Cargo.toml | 2 +- service_discovery/proto_build/build.rs | 2 +- service_discovery/proto_build/src/lib.rs | 2 +- .../simple-discovery/consumer/Cargo.toml | 2 +- .../simple-discovery/consumer/src/main.rs | 34 ++++++------ .../simple-discovery/provider/Cargo.toml | 2 +- .../provider/src/hello_world_impl.rs | 14 +++-- .../simple-discovery/provider/src/main.rs | 22 ++++---- 14 files changed, 77 insertions(+), 73 deletions(-) diff --git a/service_discovery/core/Cargo.toml b/service_discovery/core/Cargo.toml index aa799763..ef0194c4 100644 --- a/service_discovery/core/Cargo.toml +++ b/service_discovery/core/Cargo.toml @@ -15,4 +15,4 @@ parking_lot = { workspace = true } proto-servicediscovery = { path = "../proto_build/"} [build-dependencies] -tonic-build = { workspace = true } \ No newline at end of file +tonic-build = { workspace = true } diff --git a/service_discovery/core/src/registry_impl.rs b/service_discovery/core/src/registry_impl.rs index 660f8b34..fff61d2a 100644 --- a/service_discovery/core/src/registry_impl.rs +++ b/service_discovery/core/src/registry_impl.rs @@ -12,7 +12,7 @@ use proto_servicediscovery::chariott_registry::v1::registry_server::Registry; use proto_servicediscovery::chariott_registry::v1::{ DiscoverByNamespaceRequest, DiscoverByNamespaceResponse, DiscoverServiceRequest, DiscoverServiceResponse, InspectRequest, InspectResponse, RegisterRequest, RegisterResponse, - RegistrationStatus, ServiceMetadata, UnregisterRequest, UnregisterResponse + RegistrationStatus, ServiceMetadata, UnregisterRequest, UnregisterResponse, }; use std::collections::HashMap; use std::sync::Arc; @@ -92,7 +92,7 @@ impl Registry for RegistryImpl { let service_identifiers = ServiceIdentifiers { namespace: request_inner.namespace, name: request_inner.name, - version: request_inner.version + version: request_inner.version, }; info!("Received an unregister request for: {:?}", service_identifiers); @@ -102,16 +102,22 @@ impl Registry for RegistryImpl { self.registry_map.write(); match lock.remove(&service_identifiers) { Some(removed_service) => { - info!("Successfully removed service entry in Chariott service registry: {:?}", removed_service); + info!( + "Successfully removed service entry in Chariott service registry: {:?}", + removed_service + ); } None => { - let not_found_message = format!("Unable to remove service from registry: {:?}", service_identifiers); + let not_found_message = format!( + "Unable to remove service from registry: {:?}", + service_identifiers + ); warn!(not_found_message); //namespace: {0}, name: {1}, version: {2}") return Err(Status::not_found(not_found_message)); } }; } - Ok(Response::new(UnregisterResponse{})) + Ok(Response::new(UnregisterResponse {})) } /// Discovers a list of services based on the namespace, or logical grouping of services. @@ -166,7 +172,7 @@ impl Registry for RegistryImpl { let service_identifiers = ServiceIdentifiers { namespace: request_inner.namespace.clone(), name: request_inner.name.clone(), - version: request_inner.version.clone(), + version: request_inner.version, }; // This block controls the lifetime of the lock. @@ -217,7 +223,7 @@ impl Registry for RegistryImpl { /// Identifiers for a given service. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ServiceIdentifiers { - /// namespace represents a logical grouping of services + /// namespace represents a logical grouping of services namespace: String, /// the service name (without the namespace) name: String, @@ -225,7 +231,6 @@ pub struct ServiceIdentifiers { version: String, } - #[cfg(test)] mod registry_impl_test { use super::*; @@ -255,7 +260,7 @@ mod registry_impl_test { }; let registry_map = Arc::new(RwLock::new(HashMap::new())); - let registry_impl = RegistryImpl { registry_map: registry_map }; + let registry_impl = RegistryImpl { registry_map }; let request = tonic::Request::new(RegisterRequest { service: Some(service1.clone()) }); let result = registry_impl.register(request).await; assert!(result.is_ok(), "register result is not okay: {result:?}"); @@ -292,7 +297,7 @@ mod registry_impl_test { assert_eq!(updated_service_result.unwrap().uri, String::from("localhost:1001")); } } - + #[tokio::test] async fn unregister_test() { let registry_map = Arc::new(RwLock::new(HashMap::new())); @@ -318,7 +323,7 @@ mod registry_impl_test { lock.insert(service_identifiers1.clone(), service1.clone()); } - let registry_impl = RegistryImpl { registry_map: registry_map }; + let registry_impl = RegistryImpl { registry_map }; // Unregister Service let request = tonic::Request::new(UnregisterRequest { @@ -335,10 +340,13 @@ mod registry_impl_test { name: service_identifiers1.name.clone(), version: service_identifiers1.version.clone(), }); - let not_found_status = Status::not_found(format!("Unable to remove service from registry: {:?}", service_identifiers1)); + let not_found_status = Status::not_found(format!( + "Unable to remove service from registry: {:?}", + service_identifiers1 + )); let result = registry_impl.unregister(request2).await.err().unwrap(); assert_eq!(result.code(), not_found_status.code()); - assert_eq!(result.message(), not_found_status.message()); + assert_eq!(result.message(), not_found_status.message()); } #[tokio::test] @@ -366,7 +374,7 @@ mod registry_impl_test { lock.insert(service_identifiers1.clone(), service1.clone()); } - let registry_impl = RegistryImpl { registry_map: registry_map }; + let registry_impl = RegistryImpl { registry_map }; // Discover Service let request = tonic::Request::new(DiscoverServiceRequest { @@ -377,7 +385,7 @@ mod registry_impl_test { let result = registry_impl.discover_service(request).await; assert!(result.is_ok(), "DiscoverService result is not okay: {result:?}"); assert_eq!(result.unwrap().into_inner().service, Some(service1.clone())); - + // Discover by namespace let request_namespace = tonic::Request::new(DiscoverByNamespaceRequest { namespace: service_identifiers1.namespace.clone(), @@ -429,15 +437,21 @@ mod registry_impl_test { lock.insert(service_identifiers2.clone(), service2.clone()); } - let registry_impl = RegistryImpl { registry_map: registry_map }; + let registry_impl = RegistryImpl { registry_map }; - // Test that inspect returns the two services + // Test that inspect returns the two services let request = tonic::Request::new(InspectRequest {}); let result = registry_impl.inspect(request).await; assert!(result.is_ok(), "Inspect result is not okay: {result:?}"); - let result_services = result.unwrap().into_inner().services.clone(); + let result_services = result.unwrap().into_inner().services; assert_eq!(result_services.len(), 2); - assert!(result_services.contains(&service1), "Service1 not present in the inspect response"); - assert!(result_services.contains(&service2), "Service2 not present in the inspect response"); + assert!( + result_services.contains(&service1), + "Service1 not present in the inspect response" + ); + assert!( + result_services.contains(&service2), + "Service2 not present in the inspect response" + ); } } diff --git a/service_discovery/proto/build.rs b/service_discovery/proto/build.rs index d58954c0..0519d80c 100644 --- a/service_discovery/proto/build.rs +++ b/service_discovery/proto/build.rs @@ -6,4 +6,4 @@ fn main() -> Result<(), Box> { tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; tonic_build::compile_protos("../proto/samples/v1/hello_world_service.proto")?; Ok(()) -} \ No newline at end of file +} diff --git a/service_discovery/proto/chariott/v1/chariott_registry.proto b/service_discovery/proto/chariott/v1/chariott_registry.proto index e017a825..38414172 100644 --- a/service_discovery/proto/chariott/v1/chariott_registry.proto +++ b/service_discovery/proto/chariott/v1/chariott_registry.proto @@ -19,7 +19,7 @@ service Registry { // Discover, or retrieve the metadata for a single service given its fully qualified name rpc DiscoverService(DiscoverServiceRequest) returns (DiscoverServiceResponse) {} - + // Discover a list of services given their namespace rpc DiscoverByNamespace(DiscoverByNamespaceRequest) returns (DiscoverByNamespaceResponse) {} @@ -39,7 +39,7 @@ message ServiceMetadata { // Status of a registration operation enum RegistrationStatus { - // An entry did not exist in the service registry for this + // An entry did not exist in the service registry for this NEWLY_REGISTERED = 0; // An entry already existed in the service registry and it has now been updated (overwritten) UPDATED = 1; diff --git a/service_discovery/proto/samples/v1/hello_world_service.proto b/service_discovery/proto/samples/v1/hello_world_service.proto index f7371b34..37afbf5e 100644 --- a/service_discovery/proto/samples/v1/hello_world_service.proto +++ b/service_discovery/proto/samples/v1/hello_world_service.proto @@ -9,11 +9,11 @@ syntax = "proto3"; package hello_world; -// The service entry point to the Hello World service. This simple service only has one method to +// The service entry point to the Hello World service. This simple service only has one method to // show the basic flow of applications calling one another service HelloWorld { // Method which just logs and returns a message "Hello, {input string}" when it is called - rpc SayHello(HelloRequest) returns (HelloResponse); + rpc SayHello(HelloRequest) returns (HelloResponse); } // Representation of a request with the "name" or string that you would like to say hello to @@ -26,4 +26,4 @@ message HelloRequest { message HelloResponse { // The message that is returned: "Hello, " the name provided in the request string message = 1; -} \ No newline at end of file +} diff --git a/service_discovery/proto/src/lib.rs b/service_discovery/proto/src/lib.rs index 1c1cb268..e14ae8de 100644 --- a/service_discovery/proto/src/lib.rs +++ b/service_discovery/proto/src/lib.rs @@ -14,4 +14,4 @@ pub mod hello_world { #![allow(clippy::derive_partial_eq_without_eq)] tonic::include_proto!("hello_world"); } -} \ No newline at end of file +} diff --git a/service_discovery/proto_build/Cargo.toml b/service_discovery/proto_build/Cargo.toml index 6d664339..afd23f6c 100644 --- a/service_discovery/proto_build/Cargo.toml +++ b/service_discovery/proto_build/Cargo.toml @@ -14,4 +14,4 @@ tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tonic = { workspace = true } [build-dependencies] -tonic-build = { workspace = true } \ No newline at end of file +tonic-build = { workspace = true } diff --git a/service_discovery/proto_build/build.rs b/service_discovery/proto_build/build.rs index d58954c0..0519d80c 100644 --- a/service_discovery/proto_build/build.rs +++ b/service_discovery/proto_build/build.rs @@ -6,4 +6,4 @@ fn main() -> Result<(), Box> { tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; tonic_build::compile_protos("../proto/samples/v1/hello_world_service.proto")?; Ok(()) -} \ No newline at end of file +} diff --git a/service_discovery/proto_build/src/lib.rs b/service_discovery/proto_build/src/lib.rs index 1c1cb268..e14ae8de 100644 --- a/service_discovery/proto_build/src/lib.rs +++ b/service_discovery/proto_build/src/lib.rs @@ -14,4 +14,4 @@ pub mod hello_world { #![allow(clippy::derive_partial_eq_without_eq)] tonic::include_proto!("hello_world"); } -} \ No newline at end of file +} diff --git a/service_discovery/samples/simple-discovery/consumer/Cargo.toml b/service_discovery/samples/simple-discovery/consumer/Cargo.toml index 4fe20f8a..d5e30b84 100644 --- a/service_discovery/samples/simple-discovery/consumer/Cargo.toml +++ b/service_discovery/samples/simple-discovery/consumer/Cargo.toml @@ -11,4 +11,4 @@ proto-servicediscovery = { path = "../../../proto_build/"} tokio = { workspace = true, features = ["rt-multi-thread", "time"] } tonic = { workspace = true } tracing = { version = "0.1" } -tracing-subscriber = { version = "0.3", features = ["env-filter"] } \ No newline at end of file +tracing-subscriber = { version = "0.3", features = ["env-filter"] } diff --git a/service_discovery/samples/simple-discovery/consumer/src/main.rs b/service_discovery/samples/simple-discovery/consumer/src/main.rs index c2579bf4..c21e6089 100644 --- a/service_discovery/samples/simple-discovery/consumer/src/main.rs +++ b/service_discovery/samples/simple-discovery/consumer/src/main.rs @@ -5,20 +5,20 @@ //! A simple consumer for a sample of Chariott Service Discovery. //! //! This consumer "discovers" the hello world service through Chariott, and then -//! directly calls the SayHello method on it, using a known interface. This returns +//! directly calls the SayHello method on it, using a known interface. This returns //! a message containing "Hello, " followed by the string provided in the request. // Tells cargo to warn if a doc comment is missing and should be provided. #![warn(missing_docs)] -use proto_servicediscovery::chariott_registry::v1::{DiscoverServiceRequest}; use proto_servicediscovery::chariott_registry::v1::registry_client::RegistryClient; +use proto_servicediscovery::chariott_registry::v1::DiscoverServiceRequest; use proto_servicediscovery::hello_world::v1::hello_world_client::HelloWorldClient; use proto_servicediscovery::hello_world::v1::HelloRequest; use tonic::Request; use tracing::info; -use tracing_subscriber::EnvFilter; use tracing_subscriber::util::SubscriberInitExt; +use tracing_subscriber::EnvFilter; /// URL for the chariott service registry const CHARIOTT_SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; @@ -35,38 +35,34 @@ async fn main() -> Result<(), Box> { .finish(); collector.init(); - - // Create a registry client + + // Create a registry client let mut registry_client = RegistryClient::connect(CHARIOTT_SERVICE_REGISTRY_URL).await?; let discover_request = Request::new(DiscoverServiceRequest { namespace: String::from("sdv.samples"), name: String::from("hello-world"), - version: String::from("1.0.0.0") + version: String::from("1.0.0.0"), }); // Discover the simple provider service - let service_option = registry_client - .discover_service(discover_request) - .await? - .into_inner() - .service; + let service_option = + registry_client.discover_service(discover_request).await?.into_inner().service; match service_option { - Some(service) => - { + Some(service) => { info!("Discovered service {:?}", service); - if service.communication_kind != String::from("grpc+proto") || service.communication_reference != String::from("hello_world_service.v1.proto") { + if service.communication_kind != *"grpc+proto" + || service.communication_reference != *"hello_world_service.v1.proto" + { return Err("Simple Discover Consumer does not recognize communication_kind or communication_reference of provider; cannot communicate")?; } // Call the provider application directly, since we recognize the communication kind and reference let mut provider_client = HelloWorldClient::connect(service.uri).await?; - let hello_request = Request::new(HelloRequest { - name: String::from("World") - }); + let hello_request = Request::new(HelloRequest { name: String::from("World") }); let hello_response = provider_client.say_hello(hello_request).await?.into_inner(); info!(hello_response.message); - }, - None => info!("No service found.") + } + None => info!("No service found."), }; Ok(()) } diff --git a/service_discovery/samples/simple-discovery/provider/Cargo.toml b/service_discovery/samples/simple-discovery/provider/Cargo.toml index 6e047da0..c5813f9d 100644 --- a/service_discovery/samples/simple-discovery/provider/Cargo.toml +++ b/service_discovery/samples/simple-discovery/provider/Cargo.toml @@ -12,4 +12,4 @@ tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tonic = { workspace = true } tracing = { version = "0.1" } tracing-subscriber = { version = "0.3", features = ["env-filter"] } -url = { workspace = true } \ No newline at end of file +url = { workspace = true } diff --git a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs index f5fb71ad..57d0ad94 100644 --- a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs +++ b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs @@ -20,22 +20,20 @@ impl HelloWorld for HelloWorldImpl { /// Says Hello, followed by the input string /// This function returns a message which says Hello, followed by the string /// provided in a [`HelloRequest`]. Returns a [`HelloResponse`] - /// + /// /// # Arguments - /// + /// /// * `request` - A [`HelloRequest`] wrapped by a [`tonic::Request`]. async fn say_hello( &self, request: Request, ) -> Result, Status> { let request_inner = request.into_inner(); - let name = request_inner.name.clone(); + let name = request_inner.name; let message = format!("Hello, {name}"); - let hello_response = HelloResponse { - message: message.clone() - }; + let hello_response = HelloResponse { message: message.clone() }; info!("Sent message: {message}"); - + Ok(Response::new(hello_response)) } -} \ No newline at end of file +} diff --git a/service_discovery/samples/simple-discovery/provider/src/main.rs b/service_discovery/samples/simple-discovery/provider/src/main.rs index c5e287a4..12473e45 100644 --- a/service_discovery/samples/simple-discovery/provider/src/main.rs +++ b/service_discovery/samples/simple-discovery/provider/src/main.rs @@ -6,7 +6,7 @@ //! //! This provider has one service, the hello_world service, which has one //! method that returns a message containing "Hello, " followed by the string -//! provided in the request. The provider registers itself with Chariott. +//! provided in the request. The provider registers itself with Chariott. // Tells cargo to warn if a doc comment is missing and should be provided. #![warn(missing_docs)] @@ -17,13 +17,13 @@ use proto_servicediscovery::hello_world::v1::hello_world_server::HelloWorldServe use std::net::SocketAddr; use url::Url; -use proto_servicediscovery::chariott_registry::v1::{RegisterRequest, ServiceMetadata}; use proto_servicediscovery::chariott_registry::v1::registry_client::RegistryClient; -use tonic::{Request}; -use tonic::transport::{Server}; -use tracing::{info}; -use tracing_subscriber::EnvFilter; +use proto_servicediscovery::chariott_registry::v1::{RegisterRequest, ServiceMetadata}; +use tonic::transport::Server; +use tonic::Request; +use tracing::info; use tracing_subscriber::util::SubscriberInitExt; +use tracing_subscriber::EnvFilter; mod hello_world_impl; @@ -44,11 +44,10 @@ async fn main() -> Result<(), Box> { .finish(); collector.init(); - + // Intitialize addresses for provider and chariott communication. let provider_url_str = format!("http://{}", HELLO_WORLD_ENDPOINT); let socket_address: SocketAddr = HELLO_WORLD_ENDPOINT - .clone() .parse() .map_err(|e| Error::from_error("error getting SocketAddr", Box::new(e)))?; let _provider_url: Url = Url::parse(&provider_url_str) @@ -60,14 +59,12 @@ async fn main() -> Result<(), Box> { version: "1.0.0.0".to_string(), uri: provider_url_str.clone(), communication_kind: String::from("grpc+proto"), - communication_reference: String::from("hello_world_service.v1.proto") + communication_reference: String::from("hello_world_service.v1.proto"), }; let mut registry_client = RegistryClient::connect(CHARIOTT_SERVICE_REGISTRY_URL).await?; - let register_request = Request::new(RegisterRequest { - service: Some(service_metadata) - }); + let register_request = Request::new(RegisterRequest { service: Some(service_metadata) }); registry_client.register(register_request).await?.into_inner(); info!("Hello World Service registered as a Chariott provider"); @@ -78,5 +75,4 @@ async fn main() -> Result<(), Box> { .serve(socket_address) .await?; Ok(()) - } From e57c8954433b29223781759ddf643e3235b9fae2 Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Thu, 22 Jun 2023 13:22:31 -0400 Subject: [PATCH 04/13] Merge with cargo lock changes --- Cargo.lock | 72 ------------------------------------------------------ 1 file changed, 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 80092671..293f597c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -587,18 +587,6 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" -[[package]] -name = "consumer" -version = "0.1.0" -dependencies = [ - "chariott-common", - "proto-servicediscovery", - "tokio", - "tonic", - "tracing", - "tracing-subscriber", -] - [[package]] name = "core-foundation" version = "0.9.3" @@ -2036,29 +2024,6 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14f2252c834a40ed9bb5422029649578e63aa341ac401f74e719dd1afda8394e" -[[package]] -name = "parking_lot" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" -dependencies = [ - "lock_api", - "parking_lot_core", -] - -[[package]] -name = "parking_lot_core" -version = "0.9.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9069cbb9f99e3a5083476ccb29ceb1de18b9118cafa53e90c9551235de2b9521" -dependencies = [ - "cfg-if", - "libc", - "redox_syscall 0.2.16", - "smallvec", - "windows-sys 0.45.0", -] - [[package]] name = "password-hash" version = "0.4.2" @@ -2302,35 +2267,12 @@ dependencies = [ "prost", ] -[[package]] -name = "proto-servicediscovery" -version = "0.1.0" -dependencies = [ - "prost", - "tokio", - "tonic", - "tonic-build", -] - [[package]] name = "protobuf" version = "2.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf7e6d18738ecd0902d30d1ad232c9125985a3422929b16c65517b38adc14f96" -[[package]] -name = "provider" -version = "0.1.0" -dependencies = [ - "chariott-common", - "proto-servicediscovery", - "tokio", - "tonic", - "tracing", - "tracing-subscriber", - "url", -] - [[package]] name = "qoi" version = "0.4.1" @@ -2683,20 +2625,6 @@ dependencies = [ "syn 1.0.109", ] -[[package]] -name = "service_discovery" -version = "0.1.0" -dependencies = [ - "parking_lot", - "prost", - "proto-servicediscovery", - "tokio", - "tonic", - "tonic-build", - "tracing", - "tracing-subscriber", -] - [[package]] name = "sha1" version = "0.10.5" From 4e49efe0422dc4993819d0fbebe91afe0ec7750d Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Thu, 22 Jun 2023 13:46:21 -0400 Subject: [PATCH 05/13] Update cargo.lock with Service Discovery changes --- Cargo.lock | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 293f597c..54bd4002 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -587,6 +587,18 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" +[[package]] +name = "consumer" +version = "0.1.0" +dependencies = [ + "chariott-common", + "proto-servicediscovery", + "tokio", + "tonic", + "tracing", + "tracing-subscriber", +] + [[package]] name = "core-foundation" version = "0.9.3" @@ -2024,6 +2036,29 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14f2252c834a40ed9bb5422029649578e63aa341ac401f74e719dd1afda8394e" +[[package]] +name = "parking_lot" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93f00c865fe7cabf650081affecd3871070f26767e7b2070a3ffae14c654b447" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall 0.3.5", + "smallvec", + "windows-targets", +] + [[package]] name = "password-hash" version = "0.4.2" @@ -2267,12 +2302,35 @@ dependencies = [ "prost", ] +[[package]] +name = "proto-servicediscovery" +version = "0.1.0" +dependencies = [ + "prost", + "tokio", + "tonic", + "tonic-build", +] + [[package]] name = "protobuf" version = "2.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf7e6d18738ecd0902d30d1ad232c9125985a3422929b16c65517b38adc14f96" +[[package]] +name = "provider" +version = "0.1.0" +dependencies = [ + "chariott-common", + "proto-servicediscovery", + "tokio", + "tonic", + "tracing", + "tracing-subscriber", + "url", +] + [[package]] name = "qoi" version = "0.4.1" @@ -2625,6 +2683,20 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "service_discovery" +version = "0.1.0" +dependencies = [ + "parking_lot", + "prost", + "proto-servicediscovery", + "tokio", + "tonic", + "tonic-build", + "tracing", + "tracing-subscriber", +] + [[package]] name = "sha1" version = "0.10.5" @@ -3184,9 +3256,9 @@ dependencies = [ [[package]] name = "tracing-attributes" -version = "0.1.25" +version = "0.1.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8803eee176538f94ae9a14b55b2804eb7e1441f8210b1c31290b3bccdccff73b" +checksum = "5f4f31f56159e98206da9efd823404b79b6ef3143b4a7ab76e67b1751b25a4ab" dependencies = [ "proc-macro2", "quote", From 8ccb751361eb5354942372c2dcccd5b27b682c7b Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:41:50 -0400 Subject: [PATCH 06/13] First iteration comments --- Cargo.lock | 38 ++- Cargo.toml | 6 +- service_discovery/core/Cargo.toml | 11 +- service_discovery/core/src/main.rs | 18 +- ...istry_impl.rs => service_registry_impl.rs} | 299 ++++++++---------- .../v1/service_registry.proto} | 37 ++- service_discovery/proto_build/Cargo.toml | 2 +- service_discovery/proto_build/build.rs | 3 +- service_discovery/proto_build/src/lib.rs | 11 +- .../proto}/v1/hello_world_service.proto | 4 +- .../samples/proto_build/Cargo.toml | 17 + .../{proto => samples/proto_build}/build.rs | 3 +- .../{proto => samples/proto_build}/src/lib.rs | 7 - .../simple-discovery/consumer/Cargo.toml | 10 +- .../simple-discovery/consumer/src/main.rs | 26 +- .../simple-discovery/provider/Cargo.toml | 10 +- .../provider/src/hello_world_impl.rs | 6 +- .../simple-discovery/provider/src/main.rs | 26 +- 18 files changed, 276 insertions(+), 258 deletions(-) rename service_discovery/core/src/{registry_impl.rs => service_registry_impl.rs} (53%) rename service_discovery/proto/{chariott/v1/chariott_registry.proto => core/v1/service_registry.proto} (62%) rename service_discovery/{proto/samples => samples/proto}/v1/hello_world_service.proto (85%) create mode 100644 service_discovery/samples/proto_build/Cargo.toml rename service_discovery/{proto => samples/proto_build}/build.rs (51%) rename service_discovery/{proto => samples/proto_build}/src/lib.rs (61%) diff --git a/Cargo.lock b/Cargo.lock index 54bd4002..66ccc3b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -592,7 +592,8 @@ name = "consumer" version = "0.1.0" dependencies = [ "chariott-common", - "proto-servicediscovery", + "samples_proto", + "service_discovery_proto", "tokio", "tonic", "tracing", @@ -2302,16 +2303,6 @@ dependencies = [ "prost", ] -[[package]] -name = "proto-servicediscovery" -version = "0.1.0" -dependencies = [ - "prost", - "tokio", - "tonic", - "tonic-build", -] - [[package]] name = "protobuf" version = "2.27.1" @@ -2323,7 +2314,8 @@ name = "provider" version = "0.1.0" dependencies = [ "chariott-common", - "proto-servicediscovery", + "samples_proto", + "service_discovery_proto", "tokio", "tonic", "tracing", @@ -2574,6 +2566,16 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "samples_proto" +version = "0.1.0" +dependencies = [ + "prost", + "tokio", + "tonic", + "tonic-build", +] + [[package]] name = "schannel" version = "0.1.21" @@ -2689,7 +2691,7 @@ version = "0.1.0" dependencies = [ "parking_lot", "prost", - "proto-servicediscovery", + "service_discovery_proto", "tokio", "tonic", "tonic-build", @@ -2697,6 +2699,16 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "service_discovery_proto" +version = "0.1.0" +dependencies = [ + "prost", + "tokio", + "tonic", + "tonic-build", +] + [[package]] name = "sha1" version = "0.10.5" diff --git a/Cargo.toml b/Cargo.toml index 837ca0a2..6a30549e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,3 +1,7 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +# SPDX-License-Identifier: MIT + [workspace] members = [ "common", @@ -20,8 +24,6 @@ members = [ "service_discovery/samples/simple-discovery/provider" ] -exclude = [] - [package] name = "chariott" version = "0.1.0" diff --git a/service_discovery/core/Cargo.toml b/service_discovery/core/Cargo.toml index ef0194c4..2477a2e7 100644 --- a/service_discovery/core/Cargo.toml +++ b/service_discovery/core/Cargo.toml @@ -1,18 +1,21 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +# SPDX-License-Identifier: MIT + [package] name = "service_discovery" version = "0.1.0" edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +license = "MIT" [dependencies] +parking_lot = { workspace = true } prost = { workspace = true } +service_discovery_proto = { path = "../proto_build/"} tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tonic = { workspace = true } tracing = { version = "0.1" } tracing-subscriber = { version = "0.3", features = ["env-filter"] } -parking_lot = { workspace = true } -proto-servicediscovery = { path = "../proto_build/"} [build-dependencies] tonic-build = { workspace = true } diff --git a/service_discovery/core/src/main.rs b/service_discovery/core/src/main.rs index ae88a31c..a0c49dfc 100644 --- a/service_discovery/core/src/main.rs +++ b/service_discovery/core/src/main.rs @@ -12,7 +12,7 @@ #![warn(missing_docs)] use parking_lot::RwLock; -use proto_servicediscovery::chariott_registry::v1::registry_server::RegistryServer; +use service_discovery_proto::service_registry::v1::service_registry_server::ServiceRegistryServer; use std::collections::HashMap; use std::net::SocketAddr; use std::sync::Arc; @@ -21,10 +21,10 @@ use tracing::{debug, info}; use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::EnvFilter; -mod registry_impl; +mod service_registry_impl; -/// Endpoint for the chariott service registry -const CHARIOTT_SERVICE_REGISTRY_ADDR: &str = "0.0.0.0:50000"; +/// Endpoint for the service registry +const SERVICE_REGISTRY_ADDR: &str = "0.0.0.0:50000"; #[tokio::main] async fn main() -> Result<(), Box> { @@ -40,13 +40,13 @@ async fn main() -> Result<(), Box> { collector.init(); // Start up registry service - let addr: SocketAddr = CHARIOTT_SERVICE_REGISTRY_ADDR.parse()?; + let addr: SocketAddr = SERVICE_REGISTRY_ADDR.parse()?; let registry_impl = - registry_impl::RegistryImpl { registry_map: Arc::new(RwLock::new(HashMap::new())) }; - info!("Chariott Registry listening on {addr}"); + service_registry_impl::ServiceRegistryImpl::new(Arc::new(RwLock::new(HashMap::new()))); + info!("Service Registry listening on {addr}"); - Server::builder().add_service(RegistryServer::new(registry_impl)).serve(addr).await?; + Server::builder().add_service(ServiceRegistryServer::new(registry_impl)).serve(addr).await?; - debug!("The Chariott Registry has completed."); + debug!("The Service Registry has completed."); Ok(()) } diff --git a/service_discovery/core/src/registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs similarity index 53% rename from service_discovery/core/src/registry_impl.rs rename to service_discovery/core/src/service_registry_impl.rs index fff61d2a..3419954f 100644 --- a/service_discovery/core/src/registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -2,17 +2,17 @@ // Licensed under the MIT license. // SPDX-License-Identifier: MIT -//! Module containing gRPC service implementation based on [`proto_servicediscovery::chariott_registry::v1`]. +//! Module containing gRPC service implementation based on [`service_discovery_proto::service_registry::v1`]. //! //! Provides a gRPC endpoint for external services to interact with to register and discover //! services. Note: Identifiers in all Registry operations are case-sensitive. -//! -use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; -use proto_servicediscovery::chariott_registry::v1::registry_server::Registry; -use proto_servicediscovery::chariott_registry::v1::{ - DiscoverByNamespaceRequest, DiscoverByNamespaceResponse, DiscoverServiceRequest, - DiscoverServiceResponse, InspectRequest, InspectResponse, RegisterRequest, RegisterResponse, - RegistrationStatus, ServiceMetadata, UnregisterRequest, UnregisterResponse, + +use parking_lot::RwLock; +use service_discovery_proto::service_registry::v1::service_registry_server::ServiceRegistry; +use service_discovery_proto::service_registry::v1::{ + DiscoverByNamespaceRequest, DiscoverByNamespaceResponse, DiscoverRequest, DiscoverResponse, + ListRequest, ListResponse, RegisterRequest, RegisterResponse, RegistrationStatus, + ServiceMetadata, UnregisterRequest, UnregisterResponse, }; use std::collections::HashMap; use std::sync::Arc; @@ -20,15 +20,23 @@ use std::vec::Vec; use tonic::{Request, Response, Status}; use tracing::{info, warn}; -/// Base structure for the registry gRPC service +/// Base structure for the service registry gRPC service #[derive(Clone, Debug)] -pub struct RegistryImpl { - pub registry_map: Arc>>, +pub struct ServiceRegistryImpl { + registry_map: Arc>>, +} + +impl ServiceRegistryImpl { + pub fn new( + registry_map: Arc>>, + ) -> ServiceRegistryImpl { + ServiceRegistryImpl { registry_map: registry_map } + } } #[tonic::async_trait] -impl Registry for RegistryImpl { - /// Registers a service by adding it to the registry. +impl ServiceRegistry for ServiceRegistryImpl { + /// Registers a service by adding it to the service registry. /// /// This function registers a service based on a [`RegisterRequest`]. Returns a /// [`RegisterResponse`]. @@ -43,42 +51,39 @@ impl Registry for RegistryImpl { let request_inner = request.into_inner(); let service_to_register = request_inner.service.ok_or_else(|| Status::invalid_argument("Service is required"))?; - let service_identifiers = ServiceIdentifiers { + let service_identifier = ServiceIdentifier { namespace: service_to_register.namespace.clone(), name: service_to_register.name.clone(), version: service_to_register.version.clone(), }; - info!("Received a register request for: {:?}", service_identifiers); + info!("Received a register request for: {service_identifier:?}"); - let registration_status; // This block controls the lifetime of the lock. { - let mut lock: RwLockWriteGuard> = - self.registry_map.write(); - match lock.get(&service_identifiers) { - Some(_) => { - lock.insert(service_identifiers.clone(), service_to_register.clone()); - registration_status = RegistrationStatus::Updated; - info!("Updated the service entry in Chariott service registry; overwrote previous entry: {:?}", service_to_register); + let mut lock = self.registry_map.write(); + match lock.get(&service_identifier) { + Some(existing_service) => { + let error_message = format!("Register failed. A service already exists with the same service identifier: {existing_service:?}"); + warn!(error_message); + Err(Status::already_exists(error_message)) } None => { - lock.insert(service_identifiers.clone(), service_to_register.clone()); - registration_status = RegistrationStatus::NewlyRegistered; + lock.insert(service_identifier.clone(), service_to_register.clone()); info!( - "Registered new service in Chariott service registry: {:?}", - service_to_register + "Registered new service in the service registry: {service_to_register:?}" ); + let register_response = RegisterResponse { + registration_status: RegistrationStatus::NewlyRegistered as i32, + }; + Ok(Response::new(register_response)) } - }; + } } - let register_response = - RegisterResponse { registration_status: registration_status as i32 }; - Ok(Response::new(register_response)) } /// Unregisters a service by removing it from the registry. /// - /// This function registers a service based on a [`UnregisterRequest`]. Returns a + /// This function unregisters a service based on a [`UnregisterRequest`]. Returns a /// [`UnregisterResponse`]. /// /// # Arguments @@ -89,35 +94,28 @@ impl Registry for RegistryImpl { request: Request, ) -> Result, Status> { let request_inner = request.into_inner(); - let service_identifiers = ServiceIdentifiers { + let service_identifier = ServiceIdentifier { namespace: request_inner.namespace, name: request_inner.name, version: request_inner.version, }; - info!("Received an unregister request for: {:?}", service_identifiers); - - // This block controls the lifetime of the lock. - { - let mut lock: RwLockWriteGuard> = - self.registry_map.write(); - match lock.remove(&service_identifiers) { - Some(removed_service) => { - info!( - "Successfully removed service entry in Chariott service registry: {:?}", - removed_service - ); - } - None => { - let not_found_message = format!( - "Unable to remove service from registry: {:?}", - service_identifiers - ); - warn!(not_found_message); //namespace: {0}, name: {1}, version: {2}") - return Err(Status::not_found(not_found_message)); - } - }; + info!("Received an unregister request for: {service_identifier:?}"); + + let mut lock = self.registry_map.write(); + match lock.remove(&service_identifier) { + Some(removed_service) => { + info!( + "Successfully removed service entry from service registry: {removed_service:?}" + ); + Ok(Response::new(UnregisterResponse {})) + } + None => { + let not_found_message = + format!("Unable to remove service from registry: {service_identifier:?}"); + warn!(not_found_message); + Err(Status::not_found(not_found_message)) + } } - Ok(Response::new(UnregisterResponse {})) } /// Discovers a list of services based on the namespace, or logical grouping of services. @@ -133,17 +131,22 @@ impl Registry for RegistryImpl { request: Request, ) -> Result, Status> { let namespace = request.into_inner().namespace; - let mut service_list: Vec = Vec::new(); + let service_list: Vec; // This block controls the lifetime of the lock. { - let lock: RwLockReadGuard> = - self.registry_map.read(); - for (service_identifier, service_metadata) in lock.iter() { - if service_identifier.namespace == namespace { - service_list.push(service_metadata.clone()); - } - } + service_list = { + let lock = self.registry_map.read(); + lock.iter() + .filter_map(|(service_identifier, service_metadata)| { + if service_identifier.namespace == namespace { + Some(service_metadata.clone()) + } else { + None + } + }) + .collect() + }; } if service_list.is_empty() { Err(Status::not_found(format!("No registrations found for namespace {namespace}"))) @@ -157,19 +160,19 @@ impl Registry for RegistryImpl { /// Discovers a single service based on its "fully qualified name", consisting of the namespace, /// name, and version of the service. /// - /// This function discovers a service based on a [`DiscoverServiceRequest`]. Returns a - /// [`DiscoverServiceResponse`]. + /// This function discovers a service based on a [`DiscoverRequest`]. Returns a + /// [`DiscoverResponse`]. /// /// # Arguments /// - /// * `request` - A [`DiscoverServiceRequest`] wrapped by a [`tonic::Request`]. - async fn discover_service( + /// * `request` - A [`DiscoverRequest`] wrapped by a [`tonic::Request`]. + async fn discover( &self, - request: Request, - ) -> Result, Status> { + request: Request, + ) -> Result, Status> { let request_inner = request.into_inner(); - let service_identifiers = ServiceIdentifiers { + let service_identifier = ServiceIdentifier { namespace: request_inner.namespace.clone(), name: request_inner.name.clone(), version: request_inner.version, @@ -177,21 +180,19 @@ impl Registry for RegistryImpl { // This block controls the lifetime of the lock. { - let lock: RwLockReadGuard> = - self.registry_map.read(); - match lock.get(&service_identifiers) { + let lock = self.registry_map.read(); + match lock.get(&service_identifier) { Some(service) => { - info!("Read service in DiscoverService {:?}", service); - let discover_service_response = - DiscoverServiceResponse { service: Some(service.clone()) }; - Ok(Response::new(discover_service_response)) + info!("Read service in Discover {service:?}"); + let discover_response = DiscoverResponse { service: Some(service.clone()) }; + Ok(Response::new(discover_response)) } None => { let not_found_message = format!( "No service found for namespace: {0}, name: {1}, version: {2}", - service_identifiers.namespace, - service_identifiers.name, - service_identifiers.version + service_identifier.namespace, + service_identifier.name, + service_identifier.version ); warn!(not_found_message); Err(Status::not_found(not_found_message)) @@ -200,29 +201,25 @@ impl Registry for RegistryImpl { } } - /// Inspects the contents of the service registry. + /// Lists the contents of the service registry. /// - /// This function retrieves all services currently registered based on an [`InspectRequest`]. Returns a - /// [`InspectResponse`]. + /// This function retrieves all services currently registered based on an [`ListRequest`]. Returns a + /// [`ListResponse`]. /// /// # Arguments /// - /// * `request` - A [`InspectRequest`] wrapped by a [`tonic::Request`]. - async fn inspect( - &self, - _request: Request, - ) -> Result, Status> { - let lock: RwLockReadGuard> = - self.registry_map.read(); + /// * `request` - A [`ListRequest`] wrapped by a [`tonic::Request`]. + async fn list(&self, _request: Request) -> Result, Status> { + let lock = self.registry_map.read(); let services_list = lock.values().cloned().collect(); - let inspect_response = InspectResponse { services: services_list }; - Ok(Response::new(inspect_response)) + let list_response = ListResponse { services: services_list }; + Ok(Response::new(list_response)) } } /// Identifiers for a given service. #[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct ServiceIdentifiers { +pub struct ServiceIdentifier { /// namespace represents a logical grouping of services namespace: String, /// the service name (without the namespace) @@ -236,13 +233,12 @@ mod registry_impl_test { use super::*; fn has_service( - registry_map: Arc>>, - key: &ServiceIdentifiers, + registry_map: Arc>>, + key: &ServiceIdentifier, ) -> bool { // This block controls the lifetime of the lock. { - let lock: RwLockReadGuard> = - registry_map.read(); + let lock = registry_map.read(); lock.contains_key(key) } } @@ -260,7 +256,7 @@ mod registry_impl_test { }; let registry_map = Arc::new(RwLock::new(HashMap::new())); - let registry_impl = RegistryImpl { registry_map }; + let registry_impl = ServiceRegistryImpl { registry_map }; let request = tonic::Request::new(RegisterRequest { service: Some(service1.clone()) }); let result = registry_impl.register(request).await; assert!(result.is_ok(), "register result is not okay: {result:?}"); @@ -268,34 +264,27 @@ mod registry_impl_test { result.unwrap().into_inner().registration_status.clone(), RegistrationStatus::NewlyRegistered as i32 ); - let service_identifiers = ServiceIdentifiers { + let service_identifier = ServiceIdentifier { namespace: service1.namespace.clone(), name: service1.name.clone(), version: service1.version.clone(), }; assert!( - has_service(registry_impl.registry_map.clone(), &service_identifiers), + has_service(registry_impl.registry_map.clone(), &service_identifier), "service not present in registry" ); - // Test updating a registration + // Test adding a registration with same identifier fails service1.uri = String::from("localhost:1001"); - let update_request = + let existing_service_request = tonic::Request::new(RegisterRequest { service: Some(service1.clone()) }); - let updated_result = registry_impl.register(update_request).await; + let existing_service_result = registry_impl.register(existing_service_request).await; - assert!(updated_result.is_ok(), "register result is not okay: {updated_result:?}"); - assert_eq!( - updated_result.unwrap().into_inner().registration_status.clone(), - RegistrationStatus::Updated as i32 + assert!( + existing_service_result.is_err(), + "Registering an existing service should fail: {existing_service_result:?}" ); - // This block controls the lifetime of the lock. - { - let lock: RwLockReadGuard> = - registry_impl.registry_map.read(); - let updated_service_result = lock.get(&service_identifiers); - assert_eq!(updated_service_result.unwrap().uri, String::from("localhost:1001")); - } + assert_eq!(existing_service_result.unwrap_err().code(), Status::already_exists("").code()); } #[tokio::test] @@ -310,7 +299,7 @@ mod registry_impl_test { communication_kind: String::from("grpc+proto"), communication_reference: String::from("sdv.test.test_service.v1.proto"), }; - let service_identifiers1 = ServiceIdentifiers { + let service_identifier1 = ServiceIdentifier { namespace: service1.namespace.clone(), name: service1.name.clone(), version: service1.version.clone(), @@ -318,31 +307,29 @@ mod registry_impl_test { // This block controls the lifetime of the lock. { - let mut lock: RwLockWriteGuard> = - registry_map.write(); - lock.insert(service_identifiers1.clone(), service1.clone()); + let mut lock = registry_map.write(); + lock.insert(service_identifier1.clone(), service1.clone()); } - let registry_impl = RegistryImpl { registry_map }; + let registry_impl = ServiceRegistryImpl { registry_map }; // Unregister Service let request = tonic::Request::new(UnregisterRequest { - namespace: service_identifiers1.namespace.clone(), - name: service_identifiers1.name.clone(), - version: service_identifiers1.version.clone(), + namespace: service_identifier1.namespace.clone(), + name: service_identifier1.name.clone(), + version: service_identifier1.version.clone(), }); let result = registry_impl.unregister(request).await; assert!(result.is_ok(), "Unregister result is not okay: {result:?}"); // Unregister Service that doesn't exist let request2 = tonic::Request::new(UnregisterRequest { - namespace: service_identifiers1.namespace.clone(), - name: service_identifiers1.name.clone(), - version: service_identifiers1.version.clone(), + namespace: service_identifier1.namespace.clone(), + name: service_identifier1.name.clone(), + version: service_identifier1.version.clone(), }); let not_found_status = Status::not_found(format!( - "Unable to remove service from registry: {:?}", - service_identifiers1 + "Unable to remove service from registry: {service_identifier1:?}" )); let result = registry_impl.unregister(request2).await.err().unwrap(); assert_eq!(result.code(), not_found_status.code()); @@ -361,7 +348,7 @@ mod registry_impl_test { communication_kind: String::from("grpc+proto"), communication_reference: String::from("sdv.test.test_service.v1.proto"), }; - let service_identifiers1 = ServiceIdentifiers { + let service_identifier1 = ServiceIdentifier { namespace: service1.namespace.clone(), name: service1.name.clone(), version: service1.version.clone(), @@ -369,26 +356,25 @@ mod registry_impl_test { // This block controls the lifetime of the lock. { - let mut lock: RwLockWriteGuard> = - registry_map.write(); - lock.insert(service_identifiers1.clone(), service1.clone()); + let mut lock = registry_map.write(); + lock.insert(service_identifier1.clone(), service1.clone()); } - let registry_impl = RegistryImpl { registry_map }; + let registry_impl = ServiceRegistryImpl { registry_map }; // Discover Service - let request = tonic::Request::new(DiscoverServiceRequest { - namespace: service_identifiers1.namespace.clone(), - name: service_identifiers1.name.clone(), - version: service_identifiers1.version.clone(), + let request = tonic::Request::new(DiscoverRequest { + namespace: service_identifier1.namespace.clone(), + name: service_identifier1.name.clone(), + version: service_identifier1.version.clone(), }); - let result = registry_impl.discover_service(request).await; - assert!(result.is_ok(), "DiscoverService result is not okay: {result:?}"); + let result = registry_impl.discover(request).await; + assert!(result.is_ok(), "Discover result is not okay: {result:?}"); assert_eq!(result.unwrap().into_inner().service, Some(service1.clone())); // Discover by namespace let request_namespace = tonic::Request::new(DiscoverByNamespaceRequest { - namespace: service_identifiers1.namespace.clone(), + namespace: service_identifier1.namespace.clone(), }); let result_namespace = registry_impl.discover_by_namespace(request_namespace).await; assert!( @@ -399,7 +385,7 @@ mod registry_impl_test { } #[tokio::test] - async fn inspect_test() { + async fn list_test() { let registry_map = Arc::new(RwLock::new(HashMap::new())); let service1 = ServiceMetadata { @@ -418,12 +404,12 @@ mod registry_impl_test { communication_kind: String::from("grpc+proto"), communication_reference: String::from("sdv.test.test_service.v2.proto"), }; - let service_identifiers1 = ServiceIdentifiers { + let service_identifier1 = ServiceIdentifier { namespace: service1.namespace.clone(), name: service1.name.clone(), version: service1.version.clone(), }; - let service_identifiers2 = ServiceIdentifiers { + let service_identifier2 = ServiceIdentifier { namespace: service2.namespace.clone(), name: service2.name.clone(), version: service2.version.clone(), @@ -431,27 +417,20 @@ mod registry_impl_test { // This block controls the lifetime of the lock. { - let mut lock: RwLockWriteGuard> = - registry_map.write(); - lock.insert(service_identifiers1.clone(), service1.clone()); - lock.insert(service_identifiers2.clone(), service2.clone()); + let mut lock = registry_map.write(); + lock.insert(service_identifier1.clone(), service1.clone()); + lock.insert(service_identifier2.clone(), service2.clone()); } - let registry_impl = RegistryImpl { registry_map }; + let registry_impl = ServiceRegistryImpl { registry_map }; - // Test that inspect returns the two services - let request = tonic::Request::new(InspectRequest {}); - let result = registry_impl.inspect(request).await; - assert!(result.is_ok(), "Inspect result is not okay: {result:?}"); + // Test that list returns the two services + let request = tonic::Request::new(ListRequest {}); + let result = registry_impl.list(request).await; + assert!(result.is_ok(), "List result is not okay: {result:?}"); let result_services = result.unwrap().into_inner().services; assert_eq!(result_services.len(), 2); - assert!( - result_services.contains(&service1), - "Service1 not present in the inspect response" - ); - assert!( - result_services.contains(&service2), - "Service2 not present in the inspect response" - ); + assert!(result_services.contains(&service1), "Service1 not present in the list response"); + assert!(result_services.contains(&service2), "Service2 not present in the list response"); } } diff --git a/service_discovery/proto/chariott/v1/chariott_registry.proto b/service_discovery/proto/core/v1/service_registry.proto similarity index 62% rename from service_discovery/proto/chariott/v1/chariott_registry.proto rename to service_discovery/proto/core/v1/service_registry.proto index 38414172..458117b6 100644 --- a/service_discovery/proto/chariott/v1/chariott_registry.proto +++ b/service_discovery/proto/core/v1/service_registry.proto @@ -3,14 +3,14 @@ // SPDX-License-Identifier: MIT syntax = "proto3"; -package chariott_registry; +package service_registry; // Service Registry definition // // The protobuf definitions for Chariott's service registry // The entry point for the Registry gRPC Service. -service Registry { +service ServiceRegistry { // Register, or add a service to the registry rpc Register(RegisterRequest) returns (RegisterResponse) {} @@ -18,31 +18,40 @@ service Registry { rpc Unregister(UnregisterRequest) returns (UnregisterResponse) {} // Discover, or retrieve the metadata for a single service given its fully qualified name - rpc DiscoverService(DiscoverServiceRequest) returns (DiscoverServiceResponse) {} + rpc Discover(DiscoverRequest) returns (DiscoverResponse) {} // Discover a list of services given their namespace rpc DiscoverByNamespace(DiscoverByNamespaceRequest) returns (DiscoverByNamespaceResponse) {} - // Inspect, or retrieve all contents of the service registry - rpc Inspect(InspectRequest) returns (InspectResponse) {} + // List, or retrieve all contents of the service registry + rpc List(ListRequest) returns (ListResponse) {} } -// Represenation of a service including all of its metadata that the registry stores +// Representation of a service including all of its metadata that the registry stores message ServiceMetadata { + // Logical grouping of services. Multiple services can share the same namespace. + // example: sdv.samples string namespace = 1; + // Unique name of the service. string name = 2; + // The combination of namespace, name, and version uniquely identify a service string version = 3; + // The uri that clients can use to communicate with this service string uri = 4; + // Used by clients to validate that they can communicate with this service. A short description of + // the communication kind, potentially including the network protocol and api specification type. + // example: grpc+proto string communication_kind = 5; + // Used by clients to validate that they can communicate with this service. Communication communication_reference + // can be a reference to the api specification + // example: hello_world_service.v1.proto string communication_reference = 6; } // Status of a registration operation enum RegistrationStatus { - // An entry did not exist in the service registry for this + // An entry did not previously exist in the service registry for this service NEWLY_REGISTERED = 0; - // An entry already existed in the service registry and it has now been updated (overwritten) - UPDATED = 1; } // Request used to register a service, including all of its metadata @@ -67,14 +76,14 @@ message UnregisterResponse { } // Request to retrieve the metadata for a service given its service identifiers -message DiscoverServiceRequest { +message DiscoverRequest { string namespace = 1; string name = 2; string version = 3; } // Response including the single service's metadata -message DiscoverServiceResponse { +message DiscoverResponse { ServiceMetadata service = 1; } @@ -88,11 +97,11 @@ message DiscoverByNamespaceResponse { repeated ServiceMetadata services = 1; } -// Request to inspect the registry, or retrieve all registered services -message InspectRequest { +// Request to list the registry, or retrieve all registered services +message ListRequest { } // Response with a list of all registered services -message InspectResponse { +message ListResponse { repeated ServiceMetadata services = 1; } diff --git a/service_discovery/proto_build/Cargo.toml b/service_discovery/proto_build/Cargo.toml index afd23f6c..62a9d52f 100644 --- a/service_discovery/proto_build/Cargo.toml +++ b/service_discovery/proto_build/Cargo.toml @@ -3,7 +3,7 @@ # SPDX-License-Identifier: MIT [package] -name = "proto-servicediscovery" +name = "service_discovery_proto" version = "0.1.0" edition = "2021" license = "MIT" diff --git a/service_discovery/proto_build/build.rs b/service_discovery/proto_build/build.rs index 0519d80c..0c627312 100644 --- a/service_discovery/proto_build/build.rs +++ b/service_discovery/proto_build/build.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: MIT fn main() -> Result<(), Box> { - tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; - tonic_build::compile_protos("../proto/samples/v1/hello_world_service.proto")?; + tonic_build::compile_protos("../proto/core/v1/service_registry.proto")?; Ok(()) } diff --git a/service_discovery/proto_build/src/lib.rs b/service_discovery/proto_build/src/lib.rs index e14ae8de..28d53436 100644 --- a/service_discovery/proto_build/src/lib.rs +++ b/service_discovery/proto_build/src/lib.rs @@ -2,16 +2,9 @@ // Licensed under the MIT license. // SPDX-License-Identifier: MIT -pub mod chariott_registry { +pub mod service_registry { pub mod v1 { #![allow(clippy::derive_partial_eq_without_eq)] - tonic::include_proto!("chariott_registry"); - } -} - -pub mod hello_world { - pub mod v1 { - #![allow(clippy::derive_partial_eq_without_eq)] - tonic::include_proto!("hello_world"); + tonic::include_proto!("service_registry"); } } diff --git a/service_discovery/proto/samples/v1/hello_world_service.proto b/service_discovery/samples/proto/v1/hello_world_service.proto similarity index 85% rename from service_discovery/proto/samples/v1/hello_world_service.proto rename to service_discovery/samples/proto/v1/hello_world_service.proto index 37afbf5e..7db4ecf6 100644 --- a/service_discovery/proto/samples/v1/hello_world_service.proto +++ b/service_discovery/samples/proto/v1/hello_world_service.proto @@ -12,8 +12,8 @@ package hello_world; // The service entry point to the Hello World service. This simple service only has one method to // show the basic flow of applications calling one another service HelloWorld { - // Method which just logs and returns a message "Hello, {input string}" when it is called - rpc SayHello(HelloRequest) returns (HelloResponse); + // Method which just logs and returns a message "Hello, {input string}" when it is called + rpc SayHello(HelloRequest) returns (HelloResponse); } // Representation of a request with the "name" or string that you would like to say hello to diff --git a/service_discovery/samples/proto_build/Cargo.toml b/service_discovery/samples/proto_build/Cargo.toml new file mode 100644 index 00000000..8151721a --- /dev/null +++ b/service_discovery/samples/proto_build/Cargo.toml @@ -0,0 +1,17 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +# SPDX-License-Identifier: MIT + +[package] +name = "samples_proto" +version = "0.1.0" +edition = "2021" +license = "MIT" + +[dependencies] +prost = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +tonic = { workspace = true } + +[build-dependencies] +tonic-build = { workspace = true } diff --git a/service_discovery/proto/build.rs b/service_discovery/samples/proto_build/build.rs similarity index 51% rename from service_discovery/proto/build.rs rename to service_discovery/samples/proto_build/build.rs index 0519d80c..b2775980 100644 --- a/service_discovery/proto/build.rs +++ b/service_discovery/samples/proto_build/build.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: MIT fn main() -> Result<(), Box> { - tonic_build::compile_protos("../proto/chariott/v1/chariott_registry.proto")?; - tonic_build::compile_protos("../proto/samples/v1/hello_world_service.proto")?; + tonic_build::compile_protos("../proto/v1/hello_world_service.proto")?; Ok(()) } diff --git a/service_discovery/proto/src/lib.rs b/service_discovery/samples/proto_build/src/lib.rs similarity index 61% rename from service_discovery/proto/src/lib.rs rename to service_discovery/samples/proto_build/src/lib.rs index e14ae8de..130a5883 100644 --- a/service_discovery/proto/src/lib.rs +++ b/service_discovery/samples/proto_build/src/lib.rs @@ -2,13 +2,6 @@ // Licensed under the MIT license. // SPDX-License-Identifier: MIT -pub mod chariott_registry { - pub mod v1 { - #![allow(clippy::derive_partial_eq_without_eq)] - tonic::include_proto!("chariott_registry"); - } -} - pub mod hello_world { pub mod v1 { #![allow(clippy::derive_partial_eq_without_eq)] diff --git a/service_discovery/samples/simple-discovery/consumer/Cargo.toml b/service_discovery/samples/simple-discovery/consumer/Cargo.toml index d5e30b84..f9bd6669 100644 --- a/service_discovery/samples/simple-discovery/consumer/Cargo.toml +++ b/service_discovery/samples/simple-discovery/consumer/Cargo.toml @@ -1,13 +1,17 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +# SPDX-License-Identifier: MIT + [package] name = "consumer" version = "0.1.0" edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +license = "MIT" [dependencies] chariott-common = { path = "../../../../common/" } -proto-servicediscovery = { path = "../../../proto_build/"} +samples_proto = { path = "../../proto_build/" } +service_discovery_proto = { path = "../../../proto_build/"} tokio = { workspace = true, features = ["rt-multi-thread", "time"] } tonic = { workspace = true } tracing = { version = "0.1" } diff --git a/service_discovery/samples/simple-discovery/consumer/src/main.rs b/service_discovery/samples/simple-discovery/consumer/src/main.rs index c21e6089..12b1e85a 100644 --- a/service_discovery/samples/simple-discovery/consumer/src/main.rs +++ b/service_discovery/samples/simple-discovery/consumer/src/main.rs @@ -4,24 +4,26 @@ //! A simple consumer for a sample of Chariott Service Discovery. //! -//! This consumer "discovers" the hello world service through Chariott, and then +//! This consumer "discovers" the hello world service through the registry, and then //! directly calls the SayHello method on it, using a known interface. This returns //! a message containing "Hello, " followed by the string provided in the request. // Tells cargo to warn if a doc comment is missing and should be provided. #![warn(missing_docs)] -use proto_servicediscovery::chariott_registry::v1::registry_client::RegistryClient; -use proto_servicediscovery::chariott_registry::v1::DiscoverServiceRequest; -use proto_servicediscovery::hello_world::v1::hello_world_client::HelloWorldClient; -use proto_servicediscovery::hello_world::v1::HelloRequest; +use samples_proto::hello_world::v1::hello_world_client::HelloWorldClient; +use samples_proto::hello_world::v1::HelloRequest; +use service_discovery_proto::service_registry::v1::service_registry_client::ServiceRegistryClient; +use service_discovery_proto::service_registry::v1::DiscoverRequest; use tonic::Request; use tracing::info; use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::EnvFilter; -/// URL for the chariott service registry -const CHARIOTT_SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; +/// URL for the service registry +const SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; +/// Expected provider communication kind. Validate against this to ensure we can communicate with the service that the service registry returns +const EXPECTED_COMMUNICATION_KIND: &str = "grpc+proto"; #[tokio::main] async fn main() -> Result<(), Box> { @@ -37,8 +39,8 @@ async fn main() -> Result<(), Box> { collector.init(); // Create a registry client - let mut registry_client = RegistryClient::connect(CHARIOTT_SERVICE_REGISTRY_URL).await?; - let discover_request = Request::new(DiscoverServiceRequest { + let mut service_registry_client = ServiceRegistryClient::connect(SERVICE_REGISTRY_URL).await?; + let discover_request = Request::new(DiscoverRequest { namespace: String::from("sdv.samples"), name: String::from("hello-world"), version: String::from("1.0.0.0"), @@ -46,11 +48,11 @@ async fn main() -> Result<(), Box> { // Discover the simple provider service let service_option = - registry_client.discover_service(discover_request).await?.into_inner().service; + service_registry_client.discover(discover_request).await?.into_inner().service; match service_option { Some(service) => { - info!("Discovered service {:?}", service); - if service.communication_kind != *"grpc+proto" + info!("Discovered service {service:?}"); + if service.communication_kind != EXPECTED_COMMUNICATION_KIND || service.communication_reference != *"hello_world_service.v1.proto" { return Err("Simple Discover Consumer does not recognize communication_kind or communication_reference of provider; cannot communicate")?; diff --git a/service_discovery/samples/simple-discovery/provider/Cargo.toml b/service_discovery/samples/simple-discovery/provider/Cargo.toml index c5813f9d..704ff27c 100644 --- a/service_discovery/samples/simple-discovery/provider/Cargo.toml +++ b/service_discovery/samples/simple-discovery/provider/Cargo.toml @@ -1,13 +1,17 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. +# SPDX-License-Identifier: MIT + [package] name = "provider" version = "0.1.0" edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +license = "MIT" [dependencies] chariott-common = { path = "../../../../common/" } -proto-servicediscovery = { path = "../../../proto_build/"} +samples_proto = { path = "../../proto_build/" } +service_discovery_proto = { path = "../../../proto_build" } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } tonic = { workspace = true } tracing = { version = "0.1" } diff --git a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs index 57d0ad94..98f590be 100644 --- a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs +++ b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs @@ -2,12 +2,12 @@ // Licensed under the MIT license. // SPDX-License-Identifier: MIT -//! Module containing gRPC service implementation based on [`proto_servicediscovery::hello_world::v1`]. +//! Module containing gRPC service implementation based on [`samples_proto::hello_world::v1`]. //! //! Provides a gRPC endpoint for external services to call a hello world method. -use proto_servicediscovery::hello_world::v1::hello_world_server::HelloWorld; -use proto_servicediscovery::hello_world::v1::{HelloRequest, HelloResponse}; +use samples_proto::hello_world::v1::hello_world_server::HelloWorld; +use samples_proto::hello_world::v1::{HelloRequest, HelloResponse}; use tonic::{Request, Response, Status}; use tracing::info; diff --git a/service_discovery/samples/simple-discovery/provider/src/main.rs b/service_discovery/samples/simple-discovery/provider/src/main.rs index 12473e45..a526a6de 100644 --- a/service_discovery/samples/simple-discovery/provider/src/main.rs +++ b/service_discovery/samples/simple-discovery/provider/src/main.rs @@ -6,19 +6,19 @@ //! //! This provider has one service, the hello_world service, which has one //! method that returns a message containing "Hello, " followed by the string -//! provided in the request. The provider registers itself with Chariott. +//! provided in the request. The provider registers itself in the registry. // Tells cargo to warn if a doc comment is missing and should be provided. #![warn(missing_docs)] use chariott_common::error::Error; use hello_world_impl::HelloWorldImpl; -use proto_servicediscovery::hello_world::v1::hello_world_server::HelloWorldServer; +use samples_proto::hello_world::v1::hello_world_server::HelloWorldServer; use std::net::SocketAddr; use url::Url; -use proto_servicediscovery::chariott_registry::v1::registry_client::RegistryClient; -use proto_servicediscovery::chariott_registry::v1::{RegisterRequest, ServiceMetadata}; +use service_discovery_proto::service_registry::v1::service_registry_client::ServiceRegistryClient; +use service_discovery_proto::service_registry::v1::{RegisterRequest, ServiceMetadata}; use tonic::transport::Server; use tonic::Request; use tracing::info; @@ -27,10 +27,12 @@ use tracing_subscriber::EnvFilter; mod hello_world_impl; -/// URL for the chariott service registry -const CHARIOTT_SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; +/// URL for the service registry +const SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; /// Endpoint for the hello world service, which is also a provider const HELLO_WORLD_ENDPOINT: &str = "0.0.0.0:50064"; +/// communication kind for this service +const COMMUNICATION_KIND: &str = "grpc+proto"; #[tokio::main] async fn main() -> Result<(), Box> { @@ -45,8 +47,8 @@ async fn main() -> Result<(), Box> { collector.init(); - // Intitialize addresses for provider and chariott communication. - let provider_url_str = format!("http://{}", HELLO_WORLD_ENDPOINT); + // Intitialize addresses for provider communication. + let provider_url_str = format!("http://{HELLO_WORLD_ENDPOINT}"); let socket_address: SocketAddr = HELLO_WORLD_ENDPOINT .parse() .map_err(|e| Error::from_error("error getting SocketAddr", Box::new(e)))?; @@ -58,15 +60,15 @@ async fn main() -> Result<(), Box> { name: "hello-world".to_string(), version: "1.0.0.0".to_string(), uri: provider_url_str.clone(), - communication_kind: String::from("grpc+proto"), + communication_kind: COMMUNICATION_KIND.to_owned(), communication_reference: String::from("hello_world_service.v1.proto"), }; - let mut registry_client = RegistryClient::connect(CHARIOTT_SERVICE_REGISTRY_URL).await?; + let mut service_registry_client = ServiceRegistryClient::connect(SERVICE_REGISTRY_URL).await?; let register_request = Request::new(RegisterRequest { service: Some(service_metadata) }); - registry_client.register(register_request).await?.into_inner(); - info!("Hello World Service registered as a Chariott provider"); + service_registry_client.register(register_request).await?.into_inner(); + info!("Hello World Service registered as a provider"); let hello_world_impl = HelloWorldImpl::default(); // Grpc server for handling calls from clients From 2b72aa283dc1e3a8671e54040367b7f236da2a5d Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:52:01 -0400 Subject: [PATCH 07/13] Fix whitespace --- service_discovery/core/src/service_registry_impl.rs | 2 +- service_discovery/proto/core/v1/service_registry.proto | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/service_discovery/core/src/service_registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs index 3419954f..5cee8344 100644 --- a/service_discovery/core/src/service_registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -30,7 +30,7 @@ impl ServiceRegistryImpl { pub fn new( registry_map: Arc>>, ) -> ServiceRegistryImpl { - ServiceRegistryImpl { registry_map: registry_map } + ServiceRegistryImpl { registry_map } } } diff --git a/service_discovery/proto/core/v1/service_registry.proto b/service_discovery/proto/core/v1/service_registry.proto index 458117b6..cdd57a5a 100644 --- a/service_discovery/proto/core/v1/service_registry.proto +++ b/service_discovery/proto/core/v1/service_registry.proto @@ -29,7 +29,7 @@ service ServiceRegistry { // Representation of a service including all of its metadata that the registry stores message ServiceMetadata { - // Logical grouping of services. Multiple services can share the same namespace. + // Logical grouping of services. Multiple services can share the same namespace. // example: sdv.samples string namespace = 1; // Unique name of the service. @@ -39,12 +39,12 @@ message ServiceMetadata { // The uri that clients can use to communicate with this service string uri = 4; // Used by clients to validate that they can communicate with this service. A short description of - // the communication kind, potentially including the network protocol and api specification type. + // the communication kind, potentially including the network protocol and api specification type. // example: grpc+proto string communication_kind = 5; // Used by clients to validate that they can communicate with this service. Communication communication_reference // can be a reference to the api specification - // example: hello_world_service.v1.proto + // example: hello_world_service.v1.proto string communication_reference = 6; } From 4b37478ef047b05bf9957b150ad260aae44f4c1d Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Mon, 26 Jun 2023 17:17:11 -0400 Subject: [PATCH 08/13] Make method comments less verbose --- .../core/src/service_registry_impl.rs | 27 +++++-------------- .../provider/src/hello_world_impl.rs | 5 ++-- 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/service_discovery/core/src/service_registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs index 5cee8344..de8462e0 100644 --- a/service_discovery/core/src/service_registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -38,12 +38,9 @@ impl ServiceRegistryImpl { impl ServiceRegistry for ServiceRegistryImpl { /// Registers a service by adding it to the service registry. /// - /// This function registers a service based on a [`RegisterRequest`]. Returns a - /// [`RegisterResponse`]. - /// /// # Arguments /// - /// * `request` - A [`RegisterRequest`] wrapped by a [`tonic::Request`]. + /// * `request` - Contains the necessary metadata for the service to be registered async fn register( &self, request: Request, @@ -83,12 +80,9 @@ impl ServiceRegistry for ServiceRegistryImpl { /// Unregisters a service by removing it from the registry. /// - /// This function unregisters a service based on a [`UnregisterRequest`]. Returns a - /// [`UnregisterResponse`]. - /// /// # Arguments /// - /// * `request` - A [`UnregisterRequest`] wrapped by a [`tonic::Request`]. + /// * `request` - Contains the service identification information for the service to unregister async fn unregister( &self, request: Request, @@ -120,12 +114,9 @@ impl ServiceRegistry for ServiceRegistryImpl { /// Discovers a list of services based on the namespace, or logical grouping of services. /// - /// This function discovers a list of services based on a [`DiscoverByNamespaceRequest`]. Returns a - /// [`DiscoverByNamespaceResponse`]. - /// /// # Arguments /// - /// * `request` - A [`DiscoverByNamespaceRequest`] wrapped by a [`tonic::Request`]. + /// * `request` - Contains the namespace of the services to be discovered async fn discover_by_namespace( &self, request: Request, @@ -160,12 +151,9 @@ impl ServiceRegistry for ServiceRegistryImpl { /// Discovers a single service based on its "fully qualified name", consisting of the namespace, /// name, and version of the service. /// - /// This function discovers a service based on a [`DiscoverRequest`]. Returns a - /// [`DiscoverResponse`]. - /// /// # Arguments /// - /// * `request` - A [`DiscoverRequest`] wrapped by a [`tonic::Request`]. + /// * `request` - Contains the service identification information for the service to discover async fn discover( &self, request: Request, @@ -201,14 +189,11 @@ impl ServiceRegistry for ServiceRegistryImpl { } } - /// Lists the contents of the service registry. - /// - /// This function retrieves all services currently registered based on an [`ListRequest`]. Returns a - /// [`ListResponse`]. + /// Lists all services currently registered with the service registry. /// /// # Arguments /// - /// * `request` - A [`ListRequest`] wrapped by a [`tonic::Request`]. + /// * `request` - Empty ListRequest async fn list(&self, _request: Request) -> Result, Status> { let lock = self.registry_map.read(); let services_list = lock.values().cloned().collect(); diff --git a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs index 98f590be..a0de583b 100644 --- a/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs +++ b/service_discovery/samples/simple-discovery/provider/src/hello_world_impl.rs @@ -17,13 +17,12 @@ pub struct HelloWorldImpl {} #[tonic::async_trait] impl HelloWorld for HelloWorldImpl { - /// Says Hello, followed by the input string /// This function returns a message which says Hello, followed by the string - /// provided in a [`HelloRequest`]. Returns a [`HelloResponse`] + /// provided in the request. /// /// # Arguments /// - /// * `request` - A [`HelloRequest`] wrapped by a [`tonic::Request`]. + /// * `request` - Contains the input string async fn say_hello( &self, request: Request, From 5406195ebd8b2181af3709d0088fa8608e0a60ae Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Tue, 27 Jun 2023 11:25:58 -0400 Subject: [PATCH 09/13] a few locking changes --- .../core/src/service_registry_impl.rs | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/service_discovery/core/src/service_registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs index de8462e0..376ca8e5 100644 --- a/service_discovery/core/src/service_registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -122,23 +122,20 @@ impl ServiceRegistry for ServiceRegistryImpl { request: Request, ) -> Result, Status> { let namespace = request.into_inner().namespace; - let service_list: Vec; // This block controls the lifetime of the lock. - { - service_list = { - let lock = self.registry_map.read(); - lock.iter() - .filter_map(|(service_identifier, service_metadata)| { - if service_identifier.namespace == namespace { - Some(service_metadata.clone()) - } else { - None - } - }) - .collect() - }; - } + let service_list: Vec = { + let lock = self.registry_map.read(); + lock.iter() + .filter_map(|(service_identifier, service_metadata)| { + if service_identifier.namespace == namespace { + Some(service_metadata.clone()) + } else { + None + } + }) + .collect() + }; if service_list.is_empty() { Err(Status::not_found(format!("No registrations found for namespace {namespace}"))) } else { @@ -167,24 +164,26 @@ impl ServiceRegistry for ServiceRegistryImpl { }; // This block controls the lifetime of the lock. - { + let service_option = { let lock = self.registry_map.read(); - match lock.get(&service_identifier) { - Some(service) => { - info!("Read service in Discover {service:?}"); - let discover_response = DiscoverResponse { service: Some(service.clone()) }; - Ok(Response::new(discover_response)) - } - None => { - let not_found_message = format!( - "No service found for namespace: {0}, name: {1}, version: {2}", - service_identifier.namespace, - service_identifier.name, - service_identifier.version - ); - warn!(not_found_message); - Err(Status::not_found(not_found_message)) - } + lock.get(&service_identifier).cloned() + }; + + match service_option { + Some(service) => { + info!("Read service in Discover {service:?}"); + let discover_response = DiscoverResponse { service: Some(service.clone()) }; + Ok(Response::new(discover_response)) + } + None => { + let not_found_message = format!( + "No service found for namespace: {0}, name: {1}, version: {2}", + service_identifier.namespace, + service_identifier.name, + service_identifier.version + ); + warn!(not_found_message); + Err(Status::not_found(not_found_message)) } } } From c537fbd9bc39acc3a26421c13c572d36be90621c Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Tue, 27 Jun 2023 11:33:28 -0400 Subject: [PATCH 10/13] Remove unnecessary clone --- service_discovery/core/src/service_registry_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service_discovery/core/src/service_registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs index 376ca8e5..0e4d9027 100644 --- a/service_discovery/core/src/service_registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -172,7 +172,7 @@ impl ServiceRegistry for ServiceRegistryImpl { match service_option { Some(service) => { info!("Read service in Discover {service:?}"); - let discover_response = DiscoverResponse { service: Some(service.clone()) }; + let discover_response = DiscoverResponse { service: Some(service) }; Ok(Response::new(discover_response)) } None => { From 4b7bfd20d8f42abd3c101cb065a59affef0762a1 Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Tue, 27 Jun 2023 11:44:12 -0400 Subject: [PATCH 11/13] Add constant --- .../samples/simple-discovery/consumer/src/main.rs | 4 +++- .../samples/simple-discovery/provider/src/main.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/service_discovery/samples/simple-discovery/consumer/src/main.rs b/service_discovery/samples/simple-discovery/consumer/src/main.rs index 12b1e85a..e4c4225d 100644 --- a/service_discovery/samples/simple-discovery/consumer/src/main.rs +++ b/service_discovery/samples/simple-discovery/consumer/src/main.rs @@ -24,6 +24,8 @@ use tracing_subscriber::EnvFilter; const SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; /// Expected provider communication kind. Validate against this to ensure we can communicate with the service that the service registry returns const EXPECTED_COMMUNICATION_KIND: &str = "grpc+proto"; +/// Expected provider communication reference. Validate against this to ensure we can communicate with the service that the service registry returns +const EXPECTED_COMMUNICATION_REFERENCE: &str = "hello_world_service.v1.proto"; #[tokio::main] async fn main() -> Result<(), Box> { @@ -53,7 +55,7 @@ async fn main() -> Result<(), Box> { Some(service) => { info!("Discovered service {service:?}"); if service.communication_kind != EXPECTED_COMMUNICATION_KIND - || service.communication_reference != *"hello_world_service.v1.proto" + || service.communication_reference != EXPECTED_COMMUNICATION_REFERENCE { return Err("Simple Discover Consumer does not recognize communication_kind or communication_reference of provider; cannot communicate")?; } diff --git a/service_discovery/samples/simple-discovery/provider/src/main.rs b/service_discovery/samples/simple-discovery/provider/src/main.rs index a526a6de..7927d36a 100644 --- a/service_discovery/samples/simple-discovery/provider/src/main.rs +++ b/service_discovery/samples/simple-discovery/provider/src/main.rs @@ -33,6 +33,8 @@ const SERVICE_REGISTRY_URL: &str = "http://0.0.0.0:50000"; const HELLO_WORLD_ENDPOINT: &str = "0.0.0.0:50064"; /// communication kind for this service const COMMUNICATION_KIND: &str = "grpc+proto"; +/// communication reference for this service +const COMMUNICATION_REFERENCE: &str = "hello_world_service.v1.proto"; #[tokio::main] async fn main() -> Result<(), Box> { @@ -61,7 +63,7 @@ async fn main() -> Result<(), Box> { version: "1.0.0.0".to_string(), uri: provider_url_str.clone(), communication_kind: COMMUNICATION_KIND.to_owned(), - communication_reference: String::from("hello_world_service.v1.proto"), + communication_reference: COMMUNICATION_REFERENCE.to_owned(), }; let mut service_registry_client = ServiceRegistryClient::connect(SERVICE_REGISTRY_URL).await?; From f4875231de7c4dad0e3bdb0fcb5336107fff2138 Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Wed, 28 Jun 2023 16:23:24 -0400 Subject: [PATCH 12/13] Removing some unnecessary changes --- .../core/src/service_registry_impl.rs | 34 +++++++++---------- .../proto/core/v1/service_registry.proto | 9 +---- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/service_discovery/core/src/service_registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs index 0e4d9027..684f9909 100644 --- a/service_discovery/core/src/service_registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -11,8 +11,8 @@ use parking_lot::RwLock; use service_discovery_proto::service_registry::v1::service_registry_server::ServiceRegistry; use service_discovery_proto::service_registry::v1::{ DiscoverByNamespaceRequest, DiscoverByNamespaceResponse, DiscoverRequest, DiscoverResponse, - ListRequest, ListResponse, RegisterRequest, RegisterResponse, RegistrationStatus, - ServiceMetadata, UnregisterRequest, UnregisterResponse, + ListRequest, ListResponse, RegisterRequest, RegisterResponse, ServiceMetadata, + UnregisterRequest, UnregisterResponse, }; use std::collections::HashMap; use std::sync::Arc; @@ -69,10 +69,7 @@ impl ServiceRegistry for ServiceRegistryImpl { info!( "Registered new service in the service registry: {service_to_register:?}" ); - let register_response = RegisterResponse { - registration_status: RegistrationStatus::NewlyRegistered as i32, - }; - Ok(Response::new(register_response)) + Ok(Response::new(RegisterResponse {})) } } } @@ -136,13 +133,8 @@ impl ServiceRegistry for ServiceRegistryImpl { }) .collect() }; - if service_list.is_empty() { - Err(Status::not_found(format!("No registrations found for namespace {namespace}"))) - } else { - let discover_by_namespace_response = - DiscoverByNamespaceResponse { services: service_list }; - Ok(Response::new(discover_by_namespace_response)) - } + let discover_by_namespace_response = DiscoverByNamespaceResponse { services: service_list }; + Ok(Response::new(discover_by_namespace_response)) } /// Discovers a single service based on its "fully qualified name", consisting of the namespace, @@ -244,10 +236,7 @@ mod registry_impl_test { let request = tonic::Request::new(RegisterRequest { service: Some(service1.clone()) }); let result = registry_impl.register(request).await; assert!(result.is_ok(), "register result is not okay: {result:?}"); - assert_eq!( - result.unwrap().into_inner().registration_status.clone(), - RegistrationStatus::NewlyRegistered as i32 - ); + let service_identifier = ServiceIdentifier { namespace: service1.namespace.clone(), name: service1.name.clone(), @@ -366,6 +355,17 @@ mod registry_impl_test { "DiscoverByNamespace result is not okay: {result_namespace:?}" ); assert_eq!(result_namespace.unwrap().into_inner().services[0], service1.clone()); + + // Discover by namespace that has no services + let request_namespace_none = + tonic::Request::new(DiscoverByNamespaceRequest { namespace: String::from("sdv.none") }); + let result_namespace_none = + registry_impl.discover_by_namespace(request_namespace_none).await; + assert!( + result_namespace_none.is_ok(), + "DiscoverByNamespace with no services result is not okay: {result_namespace_none:?}" + ); + assert!(result_namespace_none.unwrap().into_inner().services.is_empty()); } #[tokio::test] diff --git a/service_discovery/proto/core/v1/service_registry.proto b/service_discovery/proto/core/v1/service_registry.proto index cdd57a5a..7babda98 100644 --- a/service_discovery/proto/core/v1/service_registry.proto +++ b/service_discovery/proto/core/v1/service_registry.proto @@ -48,20 +48,13 @@ message ServiceMetadata { string communication_reference = 6; } -// Status of a registration operation -enum RegistrationStatus { - // An entry did not previously exist in the service registry for this service - NEWLY_REGISTERED = 0; -} - // Request used to register a service, including all of its metadata message RegisterRequest { ServiceMetadata service = 1; } -// Response from `Register` which shows the status of the register operation +// Response from `Register` message RegisterResponse { - RegistrationStatus registration_status = 1; } // Request used to unregister a service From 7346e666146b1967655b2adaec6cdfad2b3816fa Mon Sep 17 00:00:00 2001 From: Lauren Datz <105828115+ladatz@users.noreply.github.com> Date: Thu, 29 Jun 2023 09:59:58 -0400 Subject: [PATCH 13/13] Use then instead of if statement --- service_discovery/core/src/service_registry_impl.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/service_discovery/core/src/service_registry_impl.rs b/service_discovery/core/src/service_registry_impl.rs index 684f9909..af3236d8 100644 --- a/service_discovery/core/src/service_registry_impl.rs +++ b/service_discovery/core/src/service_registry_impl.rs @@ -125,11 +125,7 @@ impl ServiceRegistry for ServiceRegistryImpl { let lock = self.registry_map.read(); lock.iter() .filter_map(|(service_identifier, service_metadata)| { - if service_identifier.namespace == namespace { - Some(service_metadata.clone()) - } else { - None - } + (service_identifier.namespace == namespace).then(|| service_metadata.clone()) }) .collect() };