You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "AlbumenJ (via GitHub)" <gi...@apache.org> on 2023/07/15 02:24:48 UTC
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #12582: Support multi registries metrics key
AlbumenJ commented on code in PR #12582:
URL: https://github.com/apache/dubbo/pull/12582#discussion_r1264290630
##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java:
##########
@@ -899,12 +900,16 @@ private DynamicConfiguration getDynamicConfiguration(URL connectionURL) {
private void registerServiceInstance() {
try {
registered = true;
- MetricsEventBus.post(RegistryEvent.toRegisterEvent(applicationModel),
- () -> {
- ServiceInstanceMetadataUtils.registerMetadataAndInstance(applicationModel);
- return null;
- }
- );
+ List<ServiceDiscovery> serviceDiscoveries = ServiceInstanceMetadataUtils.getServiceDiscoveries(applicationModel);
+ if (serviceDiscoveries.size() > 0) {
+ MetricsEventBus.post(RegistryEvent.toRegisterEvent(applicationModel, ServiceInstanceMetadataUtils.getServiceDiscoveryNames(serviceDiscoveries)),
Review Comment:
Move this `MetricsEventBus` into `ServiceInstanceMetadataUtils.registerMetadataAndInstance` would be better
##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java:
##########
@@ -513,8 +515,9 @@ private void doExportUrls(RegisterTypeEnum registerType) {
repository.registerProvider(providerModel);
List<URL> registryURLs = ConfigValidationUtils.loadRegistries(this, true);
-
- MetricsEventBus.post(RegistryEvent.toRsEvent(getApplicationModel(), getUniqueServiceName(), protocols.size() * registryURLs.size()), () -> {
+ List<String> serviceDiscoveryNames = registryURLs.stream().map(url-> url.getParameter(RegistryConstants.REGISTRY_CLUSTER_KEY)).distinct().collect(Collectors.toList());
+ MetricsEventBus.post(RegistryEvent.toRsEvent(getApplicationModel(), getUniqueServiceName(), protocols.size(), serviceDiscoveryNames),
+ () -> {
Review Comment:
It is not a good idea to working on some service discovery related in `ServiceConfig`
##########
dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/model/MetricsSupport.java:
##########
@@ -61,23 +62,42 @@ public class MetricsSupport {
private static final String version = Version.getVersion();
private static final String commitId = Version.getLastCommitId();
- public static Map<String, String> applicationTags(ApplicationModel applicationModel) {
+ public static Map<String, String> applicationTags(ApplicationModel applicationModel, @Nullable Map<String, String> extraInfo) {
Review Comment:
Add a overwrite method `Map<String, String> applicationTags(ApplicationModel applicationModel)` to prevent use `null` for caller
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org