Dynamically update turbine cluster names and monitors

[spencergibb]

See https://github.com/spring-cloud/spring-cloud-consul/issues/216

---

We have requirement that we should not everytime start our turbine application to register new services from consul , it should dynamically register those services as monitor and show their aggregated view.

But as per default implementation , it is registering monitor at the start of turbine application as mentioned in

    @Override
    public void initClusterMonitors() {
        for (String clusterName : getClusterNames()) {
            ClusterMonitor<AggDataFromCluster> clusterMonitor = (ClusterMonitor<AggDataFromCluster>) findOrRegisterAggregateMonitor(clusterName);
            clusterMonitor.registerListenertoClusterMonitor(this.StaticListener);
            try {
                clusterMonitor.startMonitor();
            }
            catch (Exception ex) {
                log.warn("Could not init cluster monitor for: " + clusterName);
                clusterMonitor.stopMonitor();
                clusterMonitor.getDispatcher().stopDispatcher();
            }
        }
    }

but it is also reading instance list from instance discovery at periodically

    private final TimerTask producer = new TimerTask() {

        @Override
        public void run() {
            if(observers == null || observers.size() == 0) {
                logger.info("No observers for InstanceObservable, will try again later");
                return; 
            }

            // get the refreshed cluster
            List<Instance> newList = null;
            try {
                newList = getInstanceList();

                logger.info("Retrieved hosts from InstanceDiscovery: " + newList.size());
                if(newList.size() < 10) {
                    logger.debug("Retrieved hosts from InstanceDiscovery: " + newList);
                }

                // ...
            } catch (Throwable t) {
                logger.info("Failed to fetch instance info, will continue to run and will try again later", t);
                return;
            }
            return;
    };

My question is , will it be dynamically register new services as well??

[JagmohanSharma]

Actually as per default implementation, we are reading cluster config from turbine.aggregator.clusterConfig property. It means, to make this dynamic in nature we should be reading this dynamically without reading from turbine.aggregator.clusterConfig.

As in our implementation we are reading service instances from consul using consulDiscoveryClient and using there serviceId as cluster name in respective AggregatorFactory.initClusterMonitors().

And for making this dynamic in nature , we have used Spring task scheduler which executes this in a particular configured period of time.

Can we also make use of the same in default implementation so that I can raise pull request?

                              @Scheduled(initialDelayString = "${turbine.discovery.pollDelayMillis:60000}",                      
                                  fixedDelayString = "${turbine.discovery.pollDelayMillis:60000}")
                            public void clusterMonitorScheduler() {
                                   initClusterMonitors();
                                 }

                                     @Override
                                  public void initClusterMonitors() {
    logger.info("Initialising ClusterMonitors at frequency: " + pollDelayMillis.get() + " millis");
    Collection<TurbineDataMonitor<AggDataFromCluster>> listRegisteredClusterMonitors = AggregatorClusterMonitorConsole.getAllMonitors();
    if (CollectionUtils.isNotEmpty(listRegisteredClusterMonitors)) {
        for (TurbineDataMonitor<AggDataFromCluster> monitor : listRegisteredClusterMonitors) {
            monitor.stopMonitor();
            monitor.getDispatcher().stopDispatcher();
        }
    }
    for (String clusterName : getClusterNames()) {
        ClusterMonitor<AggDataFromCluster> clusterMonitor = (ClusterMonitor<AggDataFromCluster>) findOrRegisterAggregateMonitor(clusterName);
        clusterMonitor.registerListenertoClusterMonitor(StaticListener);
        try {
            clusterMonitor.startMonitor();
        } catch (Exception ex) {
            logger.warn("Could not init cluster monitor for: " + clusterName);
            clusterMonitor.stopMonitor();
            clusterMonitor.getDispatcher().stopDispatcher();
        }
    }
}

                                               private List<String> getClusterNames() {

    List<String> clusters = new ArrayList<String>();
    List<ServiceInstance> serviceInstances = discoveryClient.getAllInstances();
    if (serviceInstances == null || serviceInstances.size() == 0) {
        logger.info("No apps configured, returning an empty instance list");
        clusters.add("default");
    } else {
        for (ServiceInstance serviceInstance : serviceInstances) {
            clusters.add(serviceInstance.getServiceId());
        }
    }

    return clusters;
}

[spencergibb]

So far, there aren't many (just you?) asking for this. Not sure a PR is warranted. I'd like to see if there are more plus ones.

[IsNull]

I wonder if my use case is related to this one:

In our case we have a single (real) cluster. Now turbine uses the term cluster in a more abstract way, if I understand it correctly, as an aggregate of n-hystrix streams.

What you usually want (IMHO) is to have your instance replica hystrix streams aggregated, but not mixing different services. So this yields the strange setup:

my-microservice-A -> instances(10) -> CLUSTER my-microservice-A my-microservice-B -> instances(13) -> CLUSTER my-microservice-B

This way you have a "cluster" for each service, which aggregates the data of all instances of it.

If my reasoning so far is correct, then the requirement to manually configure turbine with config lists of services is really strange, especally since it has eureka integration.

turbine:
  aggregator:
    clusterConfig: ${CLUSTER_LIST}
  appConfig: ${SERVICE_LIST}

So my idea is to use the eureka client to fetch all existing services and periodically update the appConfig and dynamically create a "cluster" for each serviceId.

Does this make any sense?

[ghost]

+1

[fcunha-ciandt]

+1

[JagmohanSharma]

@spencergibb we have more such requests for this feature. is it now good to have and warranted. :)

[northhead]

+1111111

[gillius]

I am just getting started with Consul and Spring cloud and I expected it to work this way. I'm surprised it doesn't, so maybe I'm missing something but I can't think of any other way it's supposed to work? Thinking about @spencergibb's comment, what is the way this is "supposed" to be used if not this way? Even in the documentation it says to create one cluster per service, but you must do it in a config file, meaning you have to redeploy to add a new service. It makes no sense because we're supposed to be doing service discovery. I wrote my own TurbineClustersProvider that just returns a filtered form of DiscoveryClient.getServices, but as @JagmohanSharma discovered, it's only called on startup.

[spencergibb]

This module has entered maintenance mode. This means that the Spring Cloud team will no longer be adding new features to the module. We will fix blocker bugs and security issues, and we will also consider and review small pull requests from the community.