Skip to content

Commit d52ae67

Browse files
crossoverJietomsun28
crossoverJie
and
tomsun28
authored
[improve][HIP] HIP-01: Implement refactoring AbstractCollect (apache#1966)
Co-authored-by: tomsun28 <[email protected]>
1 parent 1a3c41f commit d52ae67

38 files changed

+361
-390
lines changed

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/AbstractCollect.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@
2626
*/
2727
public abstract class AbstractCollect {
2828

29+
/**
30+
* Pre-check metrics
31+
* @param metrics metric configuration
32+
* @throws IllegalArgumentException when validation failed
33+
*/
34+
public abstract void preCheck(Metrics metrics) throws IllegalArgumentException;
35+
36+
2937
/**
3038
* Real acquisition implementation interface
3139
* @param builder response builder

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollect.java‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,23 @@ public JdbcCommonCollect(){
6363
connectionCommonCache = new ConnectionCommonCache<>();
6464
}
6565

66+
@Override
67+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
68+
if (metrics == null || metrics.getJdbc() == null) {
69+
throw new IllegalArgumentException("Database collect must has jdbc params");
70+
}
71+
if (StringUtils.hasText(metrics.getJdbc().getUrl())) {
72+
for (String keyword : VULNERABLE_KEYWORDS) {
73+
if (metrics.getJdbc().getUrl().contains(keyword)) {
74+
throw new IllegalArgumentException("Jdbc url prohibit contains vulnerable param " + keyword);
75+
}
76+
}
77+
}
78+
}
79+
6680
@Override
6781
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
6882
long startTime = System.currentTimeMillis();
69-
// check the params
70-
try {
71-
validateParams(metrics);
72-
} catch (Exception e) {
73-
builder.setCode(CollectRep.Code.FAIL);
74-
builder.setMsg(e.getMessage());
75-
return;
76-
}
7783
JdbcProtocol jdbcProtocol = metrics.getJdbc();
7884
String databaseUrl = constructDatabaseUrl(jdbcProtocol);
7985
int timeout = CollectUtil.getTimeout(jdbcProtocol.getTimeout());
@@ -311,17 +317,4 @@ private String constructDatabaseUrl(JdbcProtocol jdbcProtocol) {
311317
default -> throw new IllegalArgumentException("Not support database platform: " + jdbcProtocol.getPlatform());
312318
};
313319
}
314-
315-
private void validateParams(Metrics metrics) throws IllegalArgumentException {
316-
if (metrics == null || metrics.getJdbc() == null) {
317-
throw new IllegalArgumentException("Database collect must has jdbc params");
318-
}
319-
if (StringUtils.hasText(metrics.getJdbc().getUrl())) {
320-
for (String keyword : VULNERABLE_KEYWORDS) {
321-
if (metrics.getJdbc().getUrl().contains(keyword)) {
322-
throw new IllegalArgumentException("Jdbc url prohibit contains vulnerable param " + keyword);
323-
}
324-
}
325-
}
326-
}
327320
}

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/dns/DnsCollectImpl.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,19 @@ public class DnsCollectImpl extends AbstractCollect {
9292

9393

9494
@Override
95-
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
95+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
9696
// compatible with monitoring template configurations of older versions
9797
if (StringUtils.isBlank(metrics.getDns().getQueryClass())) {
9898
metrics.getDns().setQueryClass(DClass.string(DClass.IN));
9999
}
100100
// check params
101101
if (checkDnsProtocolFailed(metrics.getDns())) {
102-
builder.setCode(CollectRep.Code.FAIL);
103-
builder.setMsg("DNS collect must have a valid DNS protocol param! ");
104-
return;
102+
throw new IllegalArgumentException("DNS collect must have a valid DNS protocol param! ");
105103
}
104+
}
105+
106+
@Override
107+
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
106108

107109
DnsResolveResult dnsResolveResult;
108110
try {

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/ftp/FtpCollectImpl.java‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,25 @@ public class FtpCollectImpl extends AbstractCollect {
4141
private static final String ANONYMOUS = "anonymous";
4242
private static final String PASSWORD = "password";
4343

44+
/**
45+
* preCheck params
46+
*/
47+
public void preCheck(Metrics metrics) throws IllegalArgumentException{
48+
if (metrics == null || metrics.getFtp() == null) {
49+
throw new IllegalArgumentException("Ftp collect must has ftp params.");
50+
}
51+
FtpProtocol ftpProtocol = metrics.getFtp();
52+
Assert.hasText(ftpProtocol.getHost(), "Ftp Protocol host is required.");
53+
Assert.hasText(ftpProtocol.getPort(), "Ftp Protocol port is required.");
54+
Assert.hasText(ftpProtocol.getDirection(), "Ftp Protocol direction is required.");
55+
Assert.hasText(ftpProtocol.getTimeout(), "Ftp Protocol timeout is required.");
56+
}
57+
4458

4559
@Override
4660
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
4761
FTPClient ftpClient = new FTPClient();
4862
FtpProtocol ftpProtocol = metrics.getFtp();
49-
// Judge whether the basic information is wrong
50-
try {
51-
preCheck(metrics);
52-
} catch (Exception e) {
53-
log.info("[FtpProtocol] error: {}", CommonUtil.getMessageFromThrowable(e), e);
54-
builder.setCode(CollectRep.Code.UN_CONNECTABLE);
55-
builder.setMsg(e.getMessage());
56-
return;
57-
}
5863
// Set timeout
5964
ftpClient.setControlKeepAliveReplyTimeout(Integer.parseInt(ftpProtocol.getTimeout()));
6065

@@ -141,20 +146,6 @@ private void connect(FTPClient ftpClient, FtpProtocol ftpProtocol) {
141146
}
142147
}
143148

144-
/**
145-
* preCheck params
146-
*/
147-
private void preCheck(Metrics metrics) {
148-
if (metrics == null || metrics.getFtp() == null) {
149-
throw new IllegalArgumentException("Ftp collect must has ftp params.");
150-
}
151-
FtpProtocol ftpProtocol = metrics.getFtp();
152-
Assert.hasText(ftpProtocol.getHost(), "Ftp Protocol host is required.");
153-
Assert.hasText(ftpProtocol.getPort(), "Ftp Protocol port is required.");
154-
Assert.hasText(ftpProtocol.getDirection(), "Ftp Protocol direction is required.");
155-
Assert.hasText(ftpProtocol.getTimeout(), "Ftp Protocol timeout is required.");
156-
}
157-
158149
@Override
159150
public String supportProtocol() {
160151
return DispatchConstants.PROTOCOL_FTP;

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -105,18 +105,28 @@ public class HttpCollectImpl extends AbstractCollect {
105105

106106
public HttpCollectImpl() {
107107
}
108-
108+
109+
@Override
110+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
111+
if (metrics == null || metrics.getHttp() == null) {
112+
throw new IllegalArgumentException("Http/Https collect must has http params");
113+
}
114+
}
115+
109116
@Override
110117
public void collect(CollectRep.MetricsData.Builder builder,
111118
long monitorId, String app, Metrics metrics) {
112119
long startTime = System.currentTimeMillis();
113-
try {
114-
validateParams(metrics);
115-
} catch (Exception e) {
116-
builder.setCode(CollectRep.Code.FAIL);
117-
builder.setMsg(e.getMessage());
118-
return;
120+
121+
HttpProtocol httpProtocol = metrics.getHttp();
122+
String url = httpProtocol.getUrl();
123+
if (!StringUtils.hasText(url) || !url.startsWith(RIGHT_DASH)) {
124+
httpProtocol.setUrl(StringUtils.hasText(url) ? RIGHT_DASH + url.trim() : RIGHT_DASH);
125+
}
126+
if (CollectionUtils.isEmpty(httpProtocol.getSuccessCodes())) {
127+
httpProtocol.setSuccessCodes(List.of("200"));
119128
}
129+
120130
HttpContext httpContext = createHttpContext(metrics.getHttp());
121131
HttpUriRequest request = createHttpRequest(metrics.getHttp());
122132
try (CloseableHttpResponse response = CommonHttpClient.getHttpClient().execute(request, httpContext)) {
@@ -198,23 +208,6 @@ public String supportProtocol() {
198208
return DispatchConstants.PROTOCOL_HTTP;
199209
}
200210

201-
private void validateParams(Metrics metrics) throws Exception {
202-
if (metrics == null || metrics.getHttp() == null) {
203-
throw new Exception("Http/Https collect must has http params");
204-
}
205-
206-
HttpProtocol httpProtocol = metrics.getHttp();
207-
String url = httpProtocol.getUrl();
208-
209-
if (!StringUtils.hasText(url) || !url.startsWith(RIGHT_DASH)) {
210-
httpProtocol.setUrl(StringUtils.hasText(url) ? RIGHT_DASH + url.trim() : RIGHT_DASH);
211-
}
212-
213-
if (CollectionUtils.isEmpty(httpProtocol.getSuccessCodes())) {
214-
httpProtocol.setSuccessCodes(List.of("200"));
215-
}
216-
}
217-
218211
private void parseResponseByWebsite(String resp, List<String> aliasFields, HttpProtocol http,
219212
CollectRep.MetricsData.Builder builder, Long responseTime) {
220213
CollectRep.ValueRow.Builder valueRowBuilder = CollectRep.ValueRow.newBuilder();

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/http/SslCertificateCollectImpl.java‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,24 @@ public class SslCertificateCollectImpl extends AbstractCollect {
5656

5757
public SslCertificateCollectImpl() {}
5858

59+
@Override
60+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
61+
if (metrics == null || metrics.getHttp() == null) {
62+
throw new IllegalArgumentException("Http/Https collect must has http params");
63+
}
64+
}
65+
5966
@Override
6067
public void collect(CollectRep.MetricsData.Builder builder,
6168
long monitorId, String app, Metrics metrics) {
6269
long startTime = System.currentTimeMillis();
63-
try {
64-
validateParams(metrics);
65-
} catch (Exception e) {
66-
builder.setCode(CollectRep.Code.FAIL);
67-
builder.setMsg(e.getMessage());
68-
return;
69-
}
70+
7071
HttpProtocol httpProtocol = metrics.getHttp();
72+
String url = httpProtocol.getUrl();
73+
if (!StringUtils.hasText(url) || !url.startsWith(RIGHT_DASH)) {
74+
httpProtocol.setUrl(StringUtils.hasText(url) ? RIGHT_DASH + url.trim() : RIGHT_DASH);
75+
}
76+
7177
HttpsURLConnection urlConnection = null;
7278
try {
7379
String uri = "";
@@ -153,14 +159,7 @@ public String supportProtocol() {
153159
return DispatchConstants.PROTOCOL_SSL_CERT;
154160
}
155161

156-
private void validateParams(Metrics metrics) throws Exception {
157-
if (metrics == null || metrics.getHttp() == null) {
158-
throw new Exception("Http/Https collect must has http params");
159-
}
160-
HttpProtocol httpProtocol = metrics.getHttp();
161-
String url = httpProtocol.getUrl();
162-
if (!StringUtils.hasText(url) || !url.startsWith(RIGHT_DASH)) {
163-
httpProtocol.setUrl(StringUtils.hasText(url) ? RIGHT_DASH + url.trim() : RIGHT_DASH);
164-
}
162+
private void validateParams(Metrics metrics) {
163+
165164
}
166165
}

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/httpsd/HttpsdImpl.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,16 @@ public class HttpsdImpl extends AbstractCollect {
5050
private DiscoveryClientManagement discoveryClientManagement = new DiscoveryClientManagement();
5151

5252
@Override
53-
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
53+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
5454
HttpsdProtocol httpsdProtocol = metrics.getHttpsd();
55-
// check params
56-
if (checkParamsFailed(httpsdProtocol)) {
57-
builder.setCode(CollectRep.Code.FAIL);
58-
builder.setMsg("http_sd collect must have a valid http_sd protocol param! ");
59-
return;
55+
if (Objects.isNull(httpsdProtocol) || httpsdProtocol.isInvalid()){
56+
throw new IllegalArgumentException("http_sd collect must have a valid http_sd protocol param! ");
6057
}
58+
}
59+
60+
@Override
61+
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
62+
HttpsdProtocol httpsdProtocol = metrics.getHttpsd();
6163

6264
try (DiscoveryClient discoveryClient = discoveryClientManagement.getClient(httpsdProtocol)) {
6365
collectMetrics(builder, metrics, discoveryClient);

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/icmp/IcmpCollectImpl.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,16 @@ public class IcmpCollectImpl extends AbstractCollect {
3939
public IcmpCollectImpl(){}
4040

4141
@Override
42-
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
43-
long startTime = System.currentTimeMillis();
44-
// Simple validation requires mandatory parameters
42+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
4543
if (metrics == null || metrics.getIcmp() == null) {
46-
builder.setCode(CollectRep.Code.FAIL);
47-
builder.setMsg("ICMP collect must has icmp params");
48-
return;
44+
throw new IllegalArgumentException("ICMP collect must has icmp params");
4945
}
46+
}
47+
48+
@Override
49+
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
50+
long startTime = System.currentTimeMillis();
51+
5052
IcmpProtocol icmp = metrics.getIcmp();
5153
// The default timeout is 6000 milliseconds
5254
int timeout = 6000;

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/jmx/JmxCollectImpl.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,20 @@ public JmxCollectImpl() {
7575
connectionCommonCache = new ConnectionCommonCache<>();
7676
}
7777

78+
@Override
79+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
80+
Assert.isTrue(metrics != null && metrics.getJmx() != null, "JMX collect must have JMX params");
81+
82+
String url = metrics.getJmx().getUrl();
83+
if (StringUtils.hasText(url)) {
84+
Assert.doesNotContain(url, IGNORED_STUB, "JMX url prohibit contains stub, please check");
85+
}
86+
}
87+
7888
@Override
7989
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
8090

8191
try {
82-
validateParams(metrics);
8392
JmxProtocol jmxProtocol = metrics.getJmx();
8493

8594
// Create a jndi remote connection
@@ -164,15 +173,6 @@ private Map<String, String> extractAttributeValue(AttributeList attributeList) {
164173
return attributeValueMap;
165174
}
166175

167-
private void validateParams(Metrics metrics) {
168-
Assert.isTrue(metrics != null && metrics.getJmx() != null, "JMX collect must have JMX params");
169-
170-
String url = metrics.getJmx().getUrl();
171-
if (StringUtils.hasText(url)) {
172-
Assert.doesNotContain(url, IGNORED_STUB, "JMX url prohibit contains stub, please check");
173-
}
174-
}
175-
176176
private JMXConnector getConnectSession(JmxProtocol jmxProtocol) throws IOException {
177177
CacheIdentifier identifier = CacheIdentifier.builder().ip(jmxProtocol.getHost())
178178
.port(jmxProtocol.getPort()).username(jmxProtocol.getUsername())

‎collector/src/main/java/org/apache/hertzbeat/collector/collect/memcached/MemcachedCollectImpl.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,16 @@ public MemcachedCollectImpl() {
5555
private static final String STATS_END_RSP = "END";
5656

5757
@Override
58-
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
59-
long startTime = System.currentTimeMillis();
58+
public void preCheck(Metrics metrics) throws IllegalArgumentException {
6059
if (metrics == null || metrics.getMemcached() == null) {
61-
builder.setCode(CollectRep.Code.FAIL);
62-
builder.setMsg("Memcached collect must has Memcached params");
63-
return;
60+
throw new IllegalArgumentException("Memcached collect must has Memcached params");
6461
}
62+
}
63+
64+
@Override
65+
public void collect(CollectRep.MetricsData.Builder builder, long monitorId, String app, Metrics metrics) {
66+
long startTime = System.currentTimeMillis();
67+
6568
MemcachedProtocol memcachedProtocol = metrics.getMemcached();
6669
String memcachedHost = memcachedProtocol.getHost();
6770
String memcachedPort = memcachedProtocol.getPort();

0 commit comments

Comments
 (0)