Skip to content

Commit

Permalink
fix: monitor arg cause npe (#86)
Browse files Browse the repository at this point in the history
* fix(): 解决metaFreshTime和configServiceUrl的NPE以及时间格式问题

* fix(): 消除external-type在启用monitor下但又未配置时的无用日志打印

* fix(): PrometheusApolloClientMetricsExporter下错误的logger配置

* fix(): ConfigMoniotr 方法命名与API不统一问题

* fix(): 优化代码以及增加DateUtil测试类
  • Loading branch information
Rawven authored Dec 6, 2024
1 parent d4b76f8 commit e64c35b
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 42 deletions.
2 changes: 1 addition & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Apollo Java 2.4.0
* [Add more observability in apollo config client](https://github.com/apolloconfig/apollo-java/pull/74)
* [Feature Support Kubernetes ConfigMap cache for Apollo java, golang client](https://github.com/apolloconfig/apollo-java/pull/79)
* [Feature support pulling configuration information from multiple AppIds](https://github.com/apolloconfig/apollo-java/pull/70)

* [Fix monitor arg cause npe](https://github.com/apolloconfig/apollo-java/pull/86)

------------------
All issues and pull requests are [here](https://github.com/apolloconfig/apollo-java/milestone/4?closed=1)
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.ctrip.framework.apollo.build.ApolloInjector;
import com.ctrip.framework.apollo.core.utils.ClassLoaderUtil;
import com.ctrip.framework.apollo.core.utils.StringUtils;
import com.ctrip.framework.apollo.monitor.internal.exporter.AbstractApolloClientMetricsExporter;
import com.ctrip.framework.apollo.monitor.internal.exporter.ApolloClientMetricsExporter;
import com.ctrip.framework.apollo.monitor.internal.jmx.ApolloClientJmxMBeanRegister;
Expand Down Expand Up @@ -102,10 +103,13 @@ private static void initializeMetricsEventListener() {

private static void initializeMetricsExporter(
) {
if (StringUtils.isBlank(m_configUtil.getMonitorExternalType())) {
return;
}
ApolloClientMetricsExporterFactory exporterFactory = ApolloInjector.getInstance(
ApolloClientMetricsExporterFactory.class);
ApolloClientMetricsExporterFactory.class);
ApolloClientMetricsExporter metricsReporter = exporterFactory.getMetricsReporter(
MONITOR_CONTEXT.getApolloClientMonitorEventListeners());
MONITOR_CONTEXT.getApolloClientMonitorEventListeners());
if (metricsReporter != null) {
MONITOR_CONTEXT.setApolloClientMetricsExporter(metricsReporter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public interface ConfigMonitor {
/**
* get running params monitor api
*/
ApolloClientBootstrapArgsMonitorApi getRunningParamsMonitorApi();
ApolloClientBootstrapArgsMonitorApi getBootstrapArgsMonitorApi();

/**
* get monitor external system data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public ApolloClientNamespaceMonitorApi getNamespaceMonitorApi() {
}

@Override
public ApolloClientBootstrapArgsMonitorApi getRunningParamsMonitorApi() {
public ApolloClientBootstrapArgsMonitorApi getBootstrapArgsMonitorApi() {
return apolloClientMonitorContext.getBootstrapArgsApi();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@

import com.ctrip.framework.apollo.Apollo;
import com.ctrip.framework.apollo.core.utils.DeferredLoggerFactory;
import com.ctrip.framework.apollo.core.utils.StringUtils;
import com.ctrip.framework.apollo.monitor.api.ApolloClientBootstrapArgsMonitorApi;
import com.ctrip.framework.apollo.monitor.internal.jmx.mbean.ApolloClientJmxBootstrapArgsMBean;
import com.ctrip.framework.apollo.monitor.internal.listener.AbstractApolloClientMonitorEventListener;
import com.ctrip.framework.apollo.monitor.internal.event.ApolloClientMonitorEvent;
import com.ctrip.framework.apollo.util.ConfigUtil;
import com.ctrip.framework.apollo.util.date.DateUtil;
import com.google.common.collect.Maps;

import java.time.LocalDateTime;
import java.util.Map;
import java.util.Optional;

import org.slf4j.Logger;

/**
Expand All @@ -46,50 +52,55 @@ public class DefaultApolloClientBootstrapArgsApi extends

public DefaultApolloClientBootstrapArgsApi(ConfigUtil configUtil) {
super(TAG_BOOTSTRAP);
bootstrapArgs.put(APOLLO_ACCESS_KEY_SECRET, configUtil.getAccessKeySecret());
bootstrapArgs.put(APOLLO_AUTO_UPDATE_INJECTED_SPRING_PROPERTIES,
putAttachmentValue(APOLLO_ACCESS_KEY_SECRET, configUtil.getAccessKeySecret());
putAttachmentValue(APOLLO_AUTO_UPDATE_INJECTED_SPRING_PROPERTIES,
configUtil.isAutoUpdateInjectedSpringPropertiesEnabled());
bootstrapArgs.put(APOLLO_BOOTSTRAP_ENABLED,
putAttachmentValue(APOLLO_BOOTSTRAP_ENABLED,
Boolean.parseBoolean(System.getProperty(APOLLO_BOOTSTRAP_ENABLED)));
bootstrapArgs.put(APOLLO_BOOTSTRAP_NAMESPACES,
putAttachmentValue(APOLLO_BOOTSTRAP_NAMESPACES,
System.getProperty(APOLLO_BOOTSTRAP_NAMESPACES));
bootstrapArgs.put(APOLLO_BOOTSTRAP_EAGER_LOAD_ENABLED,
putAttachmentValue(APOLLO_BOOTSTRAP_EAGER_LOAD_ENABLED,
Boolean.parseBoolean(System.getProperty(APOLLO_BOOTSTRAP_EAGER_LOAD_ENABLED)));
bootstrapArgs.put(APOLLO_OVERRIDE_SYSTEM_PROPERTIES, configUtil.isOverrideSystemProperties());
bootstrapArgs.put(APOLLO_CACHE_DIR, configUtil.getDefaultLocalCacheDir());
bootstrapArgs.put(APOLLO_CLUSTER, configUtil.getCluster());
bootstrapArgs.put(APOLLO_CONFIG_SERVICE,
putAttachmentValue(APOLLO_OVERRIDE_SYSTEM_PROPERTIES, configUtil.isOverrideSystemProperties());
putAttachmentValue(APOLLO_CACHE_DIR, configUtil.getDefaultLocalCacheDir());
putAttachmentValue(APOLLO_CLUSTER, configUtil.getCluster());
putAttachmentValue(APOLLO_CONFIG_SERVICE,
System.getProperty(APOLLO_CONFIG_SERVICE));
bootstrapArgs.put(APOLLO_CLIENT_MONITOR_EXTERNAL_TYPE, configUtil.getMonitorExternalType());
bootstrapArgs.put(APOLLO_CLIENT_MONITOR_ENABLED, configUtil.isClientMonitorEnabled());
bootstrapArgs.put(APOLLO_CLIENT_MONITOR_EXTERNAL_EXPORT_PERIOD,
putAttachmentValue(APOLLO_CLIENT_MONITOR_EXTERNAL_TYPE, configUtil.getMonitorExternalType());
putAttachmentValue(APOLLO_CLIENT_MONITOR_ENABLED, configUtil.isClientMonitorEnabled());
putAttachmentValue(APOLLO_CLIENT_MONITOR_EXTERNAL_EXPORT_PERIOD,
configUtil.getMonitorExternalExportPeriod());
bootstrapArgs.put(APOLLO_META, configUtil.getMetaServerDomainName());
bootstrapArgs.put(APOLLO_PROPERTY_NAMES_CACHE_ENABLE, configUtil.isPropertyNamesCacheEnabled());
bootstrapArgs.put(APOLLO_PROPERTY_ORDER_ENABLE, configUtil.isPropertiesOrderEnabled());
bootstrapArgs.put(APOLLO_CLIENT_MONITOR_JMX_ENABLED, configUtil.isClientMonitorJmxEnabled());
bootstrapArgs.put(APOLLO_CLIENT_MONITOR_EXCEPTION_QUEUE_SIZE,
putAttachmentValue(APOLLO_META, configUtil.getMetaServerDomainName());
putAttachmentValue(APOLLO_PROPERTY_NAMES_CACHE_ENABLE, configUtil.isPropertyNamesCacheEnabled());
putAttachmentValue(APOLLO_PROPERTY_ORDER_ENABLE, configUtil.isPropertiesOrderEnabled());
putAttachmentValue(APOLLO_CLIENT_MONITOR_JMX_ENABLED, configUtil.isClientMonitorJmxEnabled());
putAttachmentValue(APOLLO_CLIENT_MONITOR_EXCEPTION_QUEUE_SIZE,
configUtil.getMonitorExceptionQueueSize());
bootstrapArgs.put(APP_ID, configUtil.getAppId());
bootstrapArgs.put(ENV, configUtil.getApolloEnv());
bootstrapArgs.put(VERSION, Apollo.VERSION);
bootstrapArgs.forEach((key, value) -> {
if (value != null) {
bootstrapArgsString.put(key, value.toString());
}
});

putAttachmentValue(APP_ID, configUtil.getAppId());
putAttachmentValue(ENV, configUtil.getApolloEnv());
putAttachmentValue(VERSION, Apollo.VERSION);
DateUtil.formatLocalDateTime(LocalDateTime.now())
.ifPresent(s -> putAttachmentValue(META_FRESH, s));
putAttachmentValue(CONFIG_SERVICE_URL,"");
}

@Override
public void collect0(ApolloClientMonitorEvent event) {
String argName = event.getName();
if (bootstrapArgs.containsKey(argName)) {
bootstrapArgs.put(argName, event.getAttachmentValue(argName));
putAttachmentValue(argName, event.getAttachmentValue(argName));
} else {
logger.warn("Unhandled event name: {}", argName);
}
}

private void putAttachmentValue(String argName, Object value) {
if(StringUtils.isBlank(argName) || value == null) {
return;
}
bootstrapArgs.put(argName, value);
bootstrapArgsString.put(argName, String.valueOf(value));
}

@Override
public boolean isMetricsSampleUpdated() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@
import com.ctrip.framework.apollo.monitor.internal.ApolloClientMonitorConstant;
import com.ctrip.framework.apollo.monitor.internal.listener.AbstractApolloClientMonitorEventListener;
import com.ctrip.framework.apollo.monitor.internal.event.ApolloClientMonitorEvent;
import com.ctrip.framework.apollo.util.date.DateUtil;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.slf4j.Logger;

Expand Down Expand Up @@ -182,7 +184,8 @@ public Map<String, NamespaceMetricsString> getNamespaceMetricsString() {
namespaces.forEach((namespace, metrics) -> {
NamespaceMetricsString namespaceMetricsString = new NamespaceMetricsString();
namespaceMetricsString.setFirstLoadTimeSpendInMs(metrics.getFirstLoadTimeSpendInMs());
namespaceMetricsString.setLatestUpdateTime(metrics.getLatestUpdateTime().toString());
DateUtil.formatLocalDateTime(metrics.getLatestUpdateTime())
.ifPresent(namespaceMetricsString::setLatestUpdateTime);
namespaceMetricsString.setUsageCount(metrics.getUsageCount());
namespaceMetricsString.setReleaseKey(metrics.getReleaseKey());
namespaceMetricsStringMap.put(namespace, namespaceMetricsString);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@
import com.ctrip.framework.apollo.monitor.internal.event.ApolloClientMonitorEventPublisher;
import com.ctrip.framework.apollo.tracer.spi.MessageProducer;
import com.ctrip.framework.apollo.tracer.spi.Transaction;
import com.ctrip.framework.apollo.util.date.DateUtil;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

/**
* @author Rawven
Expand Down Expand Up @@ -97,7 +100,7 @@ private void handleTaggedEvent(String type, String name) {
publishNamespaceNotFoundEvent(name);
break;
case APOLLO_CLIENT_CONFIGMETA:
// 不需要收集
// No need to collect
break;
default:
break;
Expand All @@ -122,9 +125,9 @@ private void publishConfigChangeEvent(String name) {

private void publishMetaServiceEvent() {
ApolloClientMonitorEventPublisher.publish(
ApolloClientMonitorEventFactory.getInstance().createEvent(META_FRESH)
.withTag(TAG_BOOTSTRAP)
.putAttachment(META_FRESH, LocalDate.now().toString()));
ApolloClientMonitorEventFactory.getInstance().createEvent(META_FRESH)
.withTag(TAG_BOOTSTRAP)
.putAttachment(META_FRESH, DateUtil.formatLocalDateTime(LocalDateTime.now()).orElse("")));
}

private void publishConfigServiceEvent(String name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public class ConfigUtil {
private boolean propertyKubernetesCacheEnabled = false;
private boolean clientMonitorEnabled = false;
private boolean clientMonitorJmxEnabled = false;
private String monitorExternalType = "NONE";
private String monitorExternalType = "";
private long monitorExternalExportPeriod = 10;
private int monitorExceptionQueueSize = 25;

Expand Down Expand Up @@ -556,7 +556,7 @@ private void initClientMonitorExternalType() {
monitorExternalType = System.getProperty(ApolloClientSystemConsts.APOLLO_CLIENT_MONITOR_EXTERNAL_TYPE);
if (Strings.isNullOrEmpty(monitorExternalType)) {
monitorExternalType = Foundation.app()
.getProperty(ApolloClientSystemConsts.APOLLO_CLIENT_MONITOR_EXTERNAL_TYPE, "NONE");
.getProperty(ApolloClientSystemConsts.APOLLO_CLIENT_MONITOR_EXTERNAL_TYPE, "");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2022 Apollo Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package com.ctrip.framework.apollo.util.date;

import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Optional;

/**
* @author Rawven
* @date 2024/10/19
*/
public class DateUtil {
public static final DateTimeFormatter MEDIUM_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");

/**
* Formats a LocalDateTime object to a string using the MEDIUM_FORMATTER.
*
* @param localDateTime the LocalDateTime to format, can be null
* @return the formatted date-time string, or null if the input is null
*/
public static Optional<String> formatLocalDateTime(LocalDateTime localDateTime) {
return Optional.ofNullable(localDateTime)
.map(dt -> dt.format(MEDIUM_FORMATTER));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

import com.ctrip.framework.apollo.build.ApolloInjector;
import com.ctrip.framework.apollo.build.MockInjector;
import com.ctrip.framework.apollo.monitor.api.ApolloClientBootstrapArgsMonitorApi;
import com.ctrip.framework.apollo.monitor.api.ApolloClientExceptionMonitorApi;
Expand Down Expand Up @@ -65,7 +64,7 @@ public void setUp(){
public void testApis(){
assertSame(exceptionMonitorApi, configMonitor.getExceptionMonitorApi());
assertSame(namespaceMonitorApi, configMonitor.getNamespaceMonitorApi());
assertSame(bootstrapArgsMonitorApi, configMonitor.getRunningParamsMonitorApi());
assertSame(bootstrapArgsMonitorApi, configMonitor.getBootstrapArgsMonitorApi());
assertSame(threadPoolMonitorApi, configMonitor.getThreadPoolMonitorApi());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2022 Apollo Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package com.ctrip.framework.apollo.util.date;

import org.junit.Test;
import static org.junit.Assert.*;
import java.time.LocalDateTime;
import java.util.Optional;

public class DateUtilTest {

@Test
public void testFormatLocalDateTime_validDate() {
LocalDateTime dateTime = LocalDateTime.of(2024, 12, 1, 10, 30, 0);

Optional<String> formattedDate = DateUtil.formatLocalDateTime(dateTime);

assertTrue(formattedDate.isPresent());
assertEquals("2024-12-01 10:30:00", formattedDate.get());
}

@Test
public void testFormatLocalDateTime_nullDate() {
Optional<String> result = DateUtil.formatLocalDateTime(null);

assertFalse(result.isPresent());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class PrometheusApolloClientMetricsExporter extends

private static final String PROMETHEUS = "prometheus";
private final Logger logger = DeferredLoggerFactory.getLogger(
DefaultApolloClientNamespaceApi.class);
PrometheusApolloClientMetricsExporter.class);
protected CollectorRegistry registry;
protected Map<String, Collector.Describable> map;

Expand Down

0 comments on commit e64c35b

Please sign in to comment.