Merge pull request #111 from nobodyiam/log-cat-refactor

Refactor client log and cat

apollo-biz/src/main/java/com/ctrip/apollo/biz/service/ConfigService.java

@@ -42,7 +42,7 @@ public class ConfigService {
       return null;
     }
     ApolloConfig config = new ApolloConfig(release.getAppId(), release.getClusterName(),
-        namespaceName, release.getId());
+        namespaceName, String.valueOf(release.getId()));
     config.setConfigurations(transformConfigurationToMap(release.getConfigurations()));
     return config;
   }

apollo-client/src/main/java/com/ctrip/apollo/internals/AbstractConfigRepository.java

@@ -22,7 +22,7 @@ public abstract class AbstractConfigRepository implements ConfigRepository {
       sync();
     } catch (Throwable ex) {
       Cat.logError(ex);
-      logger.error("Sync config failed with repository {}", this.getClass(), ex);
+      logger.warn("Sync config failed with repository {}, reason: {}", this.getClass(), ex);
     }
   }
 

apollo-client/src/main/java/com/ctrip/apollo/internals/ConfigServiceLocator.java

@@ -29,6 +29,9 @@ public class ConfigServiceLocator {
   private AtomicReference<List<ServiceDTO>> m_configServices;
   private Type m_responseType;
 
+  /**
+   * Create a config service locator
+   */
   public ConfigServiceLocator() {
     List<ServiceDTO> initial = Lists.newArrayList();
     m_configServices = new AtomicReference<>(initial);
@@ -64,7 +67,7 @@ public class ConfigServiceLocator {
       try {
         HttpResponse<List<ServiceDTO>> response = m_httpUtil.doGet(request, m_responseType);
         m_configServices.set(response.getBody());
-        Cat.logEvent("Apollo.Config.Services", response.getBody().toString());
+        logConfigServicesToCat(response.getBody());
         transaction.setStatus(Message.SUCCESS);
         return;
       } catch (Throwable ex) {
@@ -77,11 +80,17 @@ public class ConfigServiceLocator {
 
       try {
         TimeUnit.SECONDS.sleep(1);
-      } catch (InterruptedException e) {
+      } catch (InterruptedException ex) {
         //ignore
       }
     }
 
     throw new RuntimeException("Get config services failed", exception);
   }
+
+  private void logConfigServicesToCat(List<ServiceDTO> serviceDTOs) {
+    for (ServiceDTO serviceDTO : serviceDTOs) {
+      Cat.logEvent("Apollo.Config.Services", serviceDTO.getHomepageUrl());
+    }
+  }
 }

apollo-client/src/main/java/com/ctrip/apollo/internals/DefaultConfig.java

@@ -48,10 +48,9 @@ public class DefaultConfig extends AbstractConfig implements RepositoryChangeLis
       m_configProperties.set(m_configRepository.getConfig());
       m_configRepository.addChangeListener(this);
     } catch (Throwable ex) {
-      String message = String.format("Init Apollo Local Config failed - namespace: %s",
-          m_namespace);
       Cat.logError(ex);
-      logger.error(message, ex);
+      logger.warn("Init Apollo Local Config failed - namespace: {}, reason: {}.",
+          m_namespace, ex);
     }
   }
 

apollo-client/src/main/java/com/ctrip/apollo/internals/LocalFileConfigRepository.java

@@ -4,6 +4,8 @@ import com.google.common.base.Preconditions;
 
 import com.ctrip.apollo.util.ConfigUtil;
 import com.dianping.cat.Cat;
+import com.dianping.cat.message.Message;
+import com.dianping.cat.message.Transaction;
 
 import org.codehaus.plexus.PlexusContainer;
 import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
@@ -85,12 +87,19 @@ public class LocalFileConfigRepository extends AbstractConfigRepository
 
   @Override
   protected void sync() {
+    Transaction transaction = Cat.newTransaction("Apollo.ConfigService", "queryLocalConfigFile");
+    Throwable exception = null;
     try {
+      transaction.addData("Basedir", m_baseDir.getAbsolutePath());
       m_fileProperties = this.loadFromLocalCacheFile(m_baseDir, m_namespace);
+      transaction.setStatus(Message.SUCCESS);
     } catch (Throwable ex) {
       Cat.logError(ex);
-      ex.printStackTrace();
+      transaction.setStatus(ex);
+      exception = ex;
       //ignore
+    } finally {
+      transaction.complete();
     }
 
     //sync with fallback immediately
@@ -98,7 +107,7 @@ public class LocalFileConfigRepository extends AbstractConfigRepository
 
     if (m_fileProperties == null) {
       throw new RuntimeException(
-          "Load config from local config failed!");
+          "Load config from local config failed!", exception);
     }
   }
 
@@ -111,7 +120,7 @@ public class LocalFileConfigRepository extends AbstractConfigRepository
       updateFileProperties(properties);
     } catch (Throwable ex) {
       Cat.logError(ex);
-      logger.warn("Sync config from fallback repository {} failed", m_fallback.getClass(), ex);
+      logger.warn("Sync config from fallback repository {} failed, reason: {}", m_fallback.getClass(), ex);
     }
   }
 
@@ -166,12 +175,16 @@ public class LocalFileConfigRepository extends AbstractConfigRepository
 
     OutputStream out = null;
 
+    Transaction transaction = Cat.newTransaction("Apollo.ConfigService", "persistLocalConfigFile");
+    transaction.addData("LocalConfigFile", file.getAbsolutePath());
     try {
       out = new FileOutputStream(file);
       m_fileProperties.store(out, "Persisted by DefaultConfig");
+      transaction.setStatus(Message.SUCCESS);
     } catch (IOException ex) {
       Cat.logError(ex);
-      logger.error("Persist local cache file {} failed", file.getAbsolutePath(), ex);
+      transaction.setStatus(ex);
+      logger.warn("Persist local cache file {} failed, reason: {}.", file.getAbsolutePath(), ex);
     } finally {
       if (out != null) {
         try {
@@ -180,6 +193,7 @@ public class LocalFileConfigRepository extends AbstractConfigRepository
           //ignore
         }
       }
+      transaction.complete();
     }
   }
 

apollo-client/src/main/java/com/ctrip/apollo/internals/RemoteConfigRepository.java

@@ -78,7 +78,7 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
   }
 
   private void schedulePeriodicRefresh() {
-    logger.info("Schedule periodic refresh with interval: {} {}",
+    logger.debug("Schedule periodic refresh with interval: {} {}",
         m_configUtil.getRefreshInterval(), m_configUtil.getRefreshTimeUnit());
     this.m_executorService.scheduleAtFixedRate(
         new Runnable() {
@@ -100,7 +100,7 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
       return;
     }
 
-    logger.info("Remote Config changes!");
+    logger.debug("Remote Config refreshed!");
 
     m_configCache.set(current);
 
@@ -162,20 +162,19 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
 
       try {
         TimeUnit.SECONDS.sleep(1);
-      } catch (InterruptedException e) {
+      } catch (InterruptedException ex) {
         //ignore
       }
     }
     String message = String.format(
         "Load Apollo Config failed - appId: %s, cluster: %s, namespace: %s, services: %s",
         appId, cluster, m_namespace, configServices);
-    logger.error(message, exception);
     throw new RuntimeException(message, exception);
   }
 
   private String assembleUrl(String uri, String appId, String cluster, String namespace,
                              ApolloConfig previousConfig) {
-    String path = "config/%s/%s";
+    String path = "configs/%s/%s";
     List<String> params = Lists.newArrayList(appId, cluster);
 
     if (!Strings.isNullOrEmpty(namespace)) {

apollo-client/src/main/java/com/ctrip/apollo/internals/SimpleConfig.java

@@ -25,7 +25,8 @@ public class SimpleConfig extends AbstractConfig implements RepositoryChangeList
 
   /**
    * Constructor.
-   * @param namespace the namespace for this config instance
+   *
+   * @param namespace        the namespace for this config instance
    * @param configRepository the config repository for this config instance
    */
   public SimpleConfig(String namespace, ConfigRepository configRepository) {
@@ -39,10 +40,8 @@ public class SimpleConfig extends AbstractConfig implements RepositoryChangeList
       m_configProperties = m_configRepository.getConfig();
       m_configRepository.addChangeListener(this);
     } catch (Throwable ex) {
-      String message = String.format("Init Apollo Simple Config failed - namespace: %s",
-          m_namespace);
-      Cat.logError(message, ex);
-      logger.error(message, ex);
+      Cat.logError(ex);
+      logger.warn("Init Apollo Simple Config failed - namespace: {}, reason: {}", m_namespace, ex);
     }
   }
 

apollo-client/src/main/java/com/ctrip/apollo/spi/DefaultConfigFactory.java

@@ -6,6 +6,8 @@ import com.ctrip.apollo.internals.DefaultConfig;
 import com.ctrip.apollo.internals.LocalFileConfigRepository;
 import com.ctrip.apollo.internals.RemoteConfigRepository;
 import com.dianping.cat.Cat;
+import com.dianping.cat.message.Message;
+import com.dianping.cat.message.Transaction;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,11 +38,19 @@ public class DefaultConfigFactory implements ConfigFactory {
     if (baseDir.exists()) {
       return;
     }
+    Transaction transaction = Cat.newTransaction("Apollo.ConfigService", "createLocalConfigDir");
+    transaction.addData("BaseDir", baseDir.getAbsolutePath());
     try {
       Files.createDirectory(baseDir.toPath());
+      transaction.setStatus(Message.SUCCESS);
     } catch (IOException ex) {
       Cat.logError(ex);
-      logger.error("Unable to create directory: " + baseDir, ex);
+      transaction.setStatus(ex);
+      logger.warn(
+          "Unable to create local config cache directory {}, reason: {}. Will not able to cache config file.",
+          baseDir, ex);
+    } finally {
+      transaction.complete();
     }
   }
 

apollo-client/src/main/java/com/ctrip/apollo/util/http/HttpUtil.java

@@ -31,7 +31,7 @@ public class HttpUtil {
   public HttpUtil() {
     gson = new Gson();
     try {
-      basicAuth =  "Basic " + BaseEncoding.base64().encode("".getBytes("UTF-8"));
+      basicAuth =  "Basic " + BaseEncoding.base64().encode("user:".getBytes("UTF-8"));
     } catch (UnsupportedEncodingException e) {
       e.printStackTrace();
     }

apollo-client/src/test/java/com/ctrip/apollo/integration/ConfigIntegrationTest.java

@@ -41,7 +41,7 @@ import static org.junit.Assert.assertThat;
  * @author Jason Song(song_s@ctrip.com)
  */
 public class ConfigIntegrationTest extends BaseIntegrationTest {
-  private long someReleaseId;
+  private String someReleaseId;
   private File configDir;
   private String someNamespace;
 
@@ -50,7 +50,7 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     super.setUp();
 
     someNamespace = ConfigConsts.NAMESPACE_APPLICATION;
-    someReleaseId = 1;
+    someReleaseId = "1";
     configDir = new File(ClassLoaderUtil.getClassPath() + "config-cache");
     configDir.mkdirs();
   }
@@ -195,8 +195,13 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     final List<ConfigChangeEvent> changeEvents = Lists.newArrayList();
 
     config.addChangeListener(new ConfigChangeListener() {
+      AtomicInteger counter = new AtomicInteger(0);
       @Override
       public void onChange(ConfigChangeEvent changeEvent) {
+        //only need to assert once
+        if (counter.incrementAndGet() > 1) {
+          return;
+        }
         assertEquals(1, changeEvent.getChanges().size());
         assertEquals(someValue, changeEvent.getChange(someKey).getOldValue());
         assertEquals(anotherValue, changeEvent.getChange(someKey).getNewValue());
@@ -207,7 +212,7 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
 
     apolloConfig.getConfigurations().put(someKey, anotherValue);
 
-    Thread.sleep(someRefreshTimeUnit.toMillis(someRefreshInterval * 2));
+    someRefreshTimeUnit.sleep(someRefreshInterval * 2);
 
     assertThat(
         "Change event's size should equal to one or there must be some assertion failed in change listener",
@@ -217,7 +222,7 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
 
   private ContextHandler mockConfigServerHandler(final int statusCode, final ApolloConfig result,
                                                  final boolean failedAtFirstTime) {
-    ContextHandler context = new ContextHandler("/config/*");
+    ContextHandler context = new ContextHandler("/configs/*");
     context.setHandler(new AbstractHandler() {
       AtomicInteger counter = new AtomicInteger(0);
 

apollo-client/src/test/java/com/ctrip/apollo/internals/RemoteConfigRepositoryTest.java

@@ -107,7 +107,7 @@ public class RemoteConfigRepositoryTest extends ComponentTestCase {
   private ApolloConfig assembleApolloConfig(Map<String, String> configurations) {
     String someAppId = "appId";
     String someClusterName = "cluster";
-    long someReleaseId = 1;
+    String someReleaseId = "1";
     ApolloConfig apolloConfig =
             new ApolloConfig(someAppId, someClusterName, someNamespace, someReleaseId);
 

apollo-configservice/src/main/java/com/ctrip/apollo/configservice/controller/ConfigController.java

@@ -20,25 +20,28 @@ import javax.servlet.http.HttpServletResponse;
  * @author Jason Song(song_s@ctrip.com)
  */
 @RestController
-@RequestMapping("/config")
+@RequestMapping("/configs")
 public class ConfigController {
   @Autowired
   private ConfigService configService;
 
   @RequestMapping(value = "/{appId}/{clusterName}", method = RequestMethod.GET)
   public ApolloConfig queryConfig(@PathVariable String appId, @PathVariable String clusterName,
-                                  @RequestParam(value = "releaseId", defaultValue = "-1") long clientSideReleaseId,
+                                  @RequestParam(value = "releaseId", defaultValue = "-1") String clientSideReleaseId,
                                   HttpServletResponse response) throws IOException {
-    return this.queryConfig(appId, clusterName, ConfigConsts.NAMESPACE_APPLICATION, clientSideReleaseId,
+    return this
+        .queryConfig(appId, clusterName, ConfigConsts.NAMESPACE_APPLICATION, clientSideReleaseId,
             response);
   }
 
   @RequestMapping(value = "/{appId}/{clusterName}/{namespace}", method = RequestMethod.GET)
   public ApolloConfig queryConfig(@PathVariable String appId, @PathVariable String clusterName,
                                   @PathVariable String namespace,
-                                  @RequestParam(value = "releaseId", defaultValue = "-1") long clientSideReleaseId,
+                                  @RequestParam(value = "releaseId", defaultValue = "-1") String clientSideReleaseId,
                                   HttpServletResponse response) throws IOException {
     Release release = configService.findRelease(appId, clusterName, namespace);
+    //TODO if namespace != application, should also query config by namespace and DC?
+    //And if found, should merge config, as well as releaseId -> make releaseId a string?
     if (release == null) {
       response.sendError(HttpServletResponse.SC_NOT_FOUND,
           String.format(
@@ -46,7 +49,7 @@ public class ConfigController {
               appId, clusterName, namespace));
       return null;
     }
-    if (release.getId() == clientSideReleaseId) {
+    if (String.valueOf(release.getId()).equals(clientSideReleaseId)) {
       // Client side configuration is the same with server side, return 304
       response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
       return null;

apollo-configservice/src/test/java/com/ctrip/apollo/configservice/controller/ConfigControllerTest.java

@@ -57,7 +57,7 @@ public class ConfigControllerTest {
     when(configService.loadConfig(someRelease, someNamespaceName)).thenReturn(someApolloConfig);
 
     ApolloConfig result = configController.queryConfig(someAppId, someClusterName,
-        someNamespaceName, someClientSideReleaseId, someResponse);
+        someNamespaceName, String.valueOf(someClientSideReleaseId), someResponse);
 
     assertEquals(someApolloConfig, result);
     verify(configService, times(1)).findRelease(someAppId, someClusterName, someNamespaceName);
@@ -76,7 +76,7 @@ public class ConfigControllerTest {
     when(configService.findRelease(someAppId, someClusterName, someNamespaceName)).thenReturn(null);
 
     ApolloConfig result = configController.queryConfig(someAppId, someClusterName,
-        someNamespaceName, someClientSideReleaseId, someResponse);
+        someNamespaceName, String.valueOf(someClientSideReleaseId), someResponse);
 
     assertNull(result);
     verify(someResponse, times(1)).sendError(eq(HttpServletResponse.SC_NOT_FOUND), anyString());
@@ -98,7 +98,7 @@ public class ConfigControllerTest {
     when(configService.loadConfig(someRelease, someNamespaceName)).thenReturn(null);
 
     ApolloConfig result = configController.queryConfig(someAppId, someClusterName,
-        someNamespaceName, someClientSideReleaseId, someResponse);
+        someNamespaceName, String.valueOf(someClientSideReleaseId), someResponse);
 
     assertNull(result);
     verify(someResponse, times(1)).sendError(eq(HttpServletResponse.SC_NOT_FOUND), anyString());
@@ -119,7 +119,7 @@ public class ConfigControllerTest {
     when(someRelease.getId()).thenReturn(someServerSideReleaseId);
 
     ApolloConfig result = configController.queryConfig(someAppId, someClusterName, someNamespaceName,
-        someClientSideReleaseId, someResponse);
+        String.valueOf(someClientSideReleaseId), someResponse);
 
     assertNull(result);
     verify(someResponse, times(1)).setStatus(HttpServletResponse.SC_NOT_MODIFIED);

apollo-core/src/main/java/com/ctrip/apollo/core/dto/ApolloConfig.java

@@ -17,12 +17,12 @@ public class ApolloConfig {
 
   private Map<String, String> configurations;
 
-  private long releaseId;
+  private String releaseId;
 
   public ApolloConfig(String appId,
                       String cluster,
                       String namespace,
-                      long releaseId) {
+                      String releaseId) {
     super();
     this.appId = appId;
     this.cluster = cluster;
@@ -42,7 +42,7 @@ public class ApolloConfig {
     return namespace;
   }
 
-  public long getReleaseId() {
+  public String getReleaseId() {
     return releaseId;
   }
 

apollo-core/src/main/java/com/ctrip/apollo/core/dto/ApolloConfigNotification.java

+package com.ctrip.apollo.core.dto;
+
+/**
+ * @author Jason Song(song_s@ctrip.com)
+ */
+public class ApolloConfigNotification {
+  private final String appId;
+  private final String cluster;
+  private final String namespace;
+
+  public ApolloConfigNotification(String appId, String cluster, String namespace) {
+    this.appId = appId;
+    this.cluster = cluster;
+    this.namespace = namespace;
+  }
+
+  public String getAppId() {
+    return appId;
+  }
+
+  public String getCluster() {
+    return cluster;
+  }
+
+  public String getNamespace() {
+    return namespace;
+  }
+}