[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r475110197
##
File path: test/plugin/scenarios/spring-scheduled-scenario/pom.xml
##
@@ -0,0 +1,129 @@
+
+
+http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+
+org.apache.skywalking.apm.testcase
+spring-scheduled-scenario
+1.0.0
+war
+
+4.0.0
+
+skywalking-spring-scheduled-scenario
+
+
+UTF-8
+1.8
+1.18.10
+
+3.1.0.RELEASE
+spring-scheduled
+
+
+
+
+javax.servlet
+javax.servlet-api
+3.1.0
+provided
+
+
+org.apache.logging.log4j
+log4j-api
+2.8.1
+
+
+org.apache.logging.log4j
+log4j-core
+2.8.1
+
+
+org.projectlombok
+lombok
+${lombok.version}
+provided
+
+
+
+org.springframework
+spring-context
+${test.framework.version}
+
+
+org.springframework
+spring-web
+${test.framework.version}
+
+
+org.springframework
+spring-webmvc
+${test.framework.version}
+
+
+cglib
+cglib
+2.2
+
+
+
+com.squareup.okhttp3
+okhttp
+3.0.0
+
+
+
+
+spring-scheduled-scenario
+
+
+maven-compiler-plugin
+
+${compiler.version}
+${compiler.version}
+${project.build.sourceEncoding}
+
+
+
+
+org.apache.tomcat.maven
+tomcat7-maven-plugin
+2.1
+
+8080
+/spring-scheduled-scenario
+UTF-8
+tomcat7
+
+
+
+
+
+
+
+spring-snapshots
+http://repo.spring.io/snapshot
+
+
+spring-milestones
+http://repo.spring.io/milestone
Review comment:
I have removed the unused code
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r474607626
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
verify failed. the master code seems to have changes
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473523128
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> You could have two, but only `SpringScheduled` as an OP name(also will
be an endpoint) is not a good idea, you could use `SpringScheduled-` + `full
class name` + `method name` as a better operation name. Because from the
analysis perspective you need a meaningful name.
Yes, I agree with you.
So, are these the final conclusions?
1. spring-scheduled-annotation-plugin is not an optional plugin
2. use `SpringScheduled- + full class name + method name` as a better
operation name.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios.
Maybe we should keep the status quo and use `SpringScheduled` as
`operationName`.
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios.
Maybe we should keep the status quo and use `SpringScheduled` as a
operationName.
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios.
Maybe we should keep the status quo and use'SpringScheduled' as
`operationName`.
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios
e.g.
## no added spring-annotation-plugin
## added spring-annotation-plugin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using `@Component and @Async`
generate two spans?
It seems that `@Component, @Scheduled` and `@Component, @Async` are similar
scenarios
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133823
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> Do you think there is a problem with using @component and @async
generate two spans?
>
> It seems that `@Component`, `@Scheduled` and `@Component`, `@Async` are
similar scenarios
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473133273
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Do you think there is a problem with using @Component and @Async
generate two spans?
It seems that @Component, @Scheduled and @Component, @Async are similar
scenarios
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473092819
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
I need to set it's an optional plugin? spring-annotation-plugin is
already in optional list
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473086195
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
OK，i have a try
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473086195
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
OK，I'll try
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473068943
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> > > @configuration and @scheduled can be used together
> >
> >
> > Yes, but with `component` is the right usage?
>
> The document indicates that `@Component` and `@Scheduled` can also be used
together
Duplicate tracing occurs when `@Component` and `@Scheduled` are used together
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473066417
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> > @configuration and @scheduled can be used together
>
> Yes, but with `component` is the right usage?
The document indicates that `@Component` and `@Scheduled` can also be used
together
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473061473
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> According to the official doc,
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/context/annotation/Configuration.html
>
> `@Configuration` should only be used for `@Bean`. Is there abuse? I am not
working on Spring for a long time, could you explain more about the use case.
The document describes ` @Configuration` and `@Scheduled` can be used
together
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/scheduling/annotation/EnableScheduling.html
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473061473
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> According to the official doc,
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/context/annotation/Configuration.html
>
> `@Configuration` should only be used for `@Bean`. Is there abuse? I am not
working on Spring for a long time, could you explain more about the use case.
The document describes @Configuration and @Scheduled can be used together
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/scheduling/annotation/EnableScheduling.html
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473036932
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
> The first one is easy to explain, we don't watch the configuration.
The second one is not clear, why `Duplicate tracing occurs when I use
@component`? Who will trace twice?
spring-scheduled-annotation-plugin trace once
then spring-annotation-plugin(optional-plugin) trace it again
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473004194
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Maybe the @Scheduled method should be filtered out in
spring-annotation-plugi.
1. @Scheduled is not an optional plugin
2. spring-scheduler can be started through [xml configuration
files](https://github.com/spring-projects/spring-framework/blob/master/spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTasksBeanDefinitionParser.java#L103)
instead of just annotations
```xml
```
How do you think about it ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473004194
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Maybe the @Scheduled method should be filtered out in
spring-annotation-plugi.
1. @Scheduled is not an optional plugin
2. spring-scheduler can be started through [configuration
files](https://github.com/spring-projects/spring-framework/blob/master/spring-context/src/main/java/org/springframework/scheduling/config/ScheduledTasksBeanDefinitionParser.java#L103)
instead of just annotations
How do you think about it ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473004194
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Maybe the @Scheduled method should be filtered out in
spring-annotation-plugi.
1. @Scheduled is not an optional plugin
2. spring-scheduler can be started through configuration files instead of
just annotations
How do you think about it ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473004194
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
Maybe the @Scheduled method should be filtered out in
spring-annotation-plugi.
1. @Scheduled is not an optional plugin
2. spring-scheduler can be started through configuration files instead of
just annotations
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r473000375
##
File path:
test/plugin/scenarios/spring-scheduled-scenario/src/main/java/org/apache/skywalking/apm/testcase/spring/scheduled/job/SchedulingJob.java
##
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.spring.scheduled.job;
+
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.springframework.scheduling.annotation.EnableScheduling;
+import org.springframework.scheduling.annotation.Scheduled;
+import org.springframework.stereotype.Component;
+
+import java.io.IOException;
+
+@Component
+@EnableScheduling
+public class SchedulingJob {
+
+private static final Logger logger =
LogManager.getLogger(SchedulingJob.class);
+
+private static final OkHttpClient client = new
OkHttpClient.Builder().build();
+
+@Scheduled(fixedDelay = 5000)
Review comment:
I found some bugs... @wu-sheng
Expected to be normal when I use @Configuration
---
Duplicate tracing occurs when I use @Component
---
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r472650011
##
File path:
apm-sniffer/apm-sdk-plugin/spring-plugins/scheduled-annotation-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/scheduled/ScheduledMethodInterceptor.java
##
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.spring.scheduled;
+
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.tag.Tags;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
+
+import java.lang.reflect.Method;
+
+public class ScheduledMethodInterceptor implements
InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor {
+
+private static final String DEFAULT_OPERATION_NAME = "SpringScheduled";
+private static final String DEFAULT_LOGIC_ENDPOINT_CONTENT =
"{\"logic-span\":true}";
Review comment:
ok
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r472592209
##
File path:
apm-sniffer/apm-sdk-plugin/spring-plugins/scheduled-annotation-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/scheduled/ScheduledMethodInterceptor.java
##
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.spring.scheduled;
+
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.tag.Tags;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
+
+import java.lang.reflect.Method;
+
+public class ScheduledMethodInterceptor implements
InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor {
+
+private static final String DEFAULT_OPERATION_NAME = "SpringScheduled";
+private static final String DEFAULT_LOGIC_ENDPOINT_CONTENT =
"{\"logic-span\":true}";
Review comment:
Add logic endpoint into others plugin？I could try
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339:
URL: https://github.com/apache/skywalking/pull/5339#discussion_r471928247
##
File path:
apm-sniffer/apm-sdk-plugin/spring-plugins/scheduled-annotation-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/scheduled/ScheduledMethodInterceptor.java
##
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.spring.scheduled;
+
+import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
+
+import java.lang.reflect.Method;
+
+public class ScheduledMethodInterceptor implements
InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor {
+
+private static final String DEFAULT_OPERATION_NAME = "SpringScheduled";
+
+@Override
+public void beforeMethod(EnhancedInstance objInst, Method method, Object[]
allArguments, Class[] argumentsTypes, MethodInterceptResult result) throws
Throwable {
+String targetMethodName = (String) objInst.getSkyWalkingDynamicField();
+String operationName = targetMethodName != null ? targetMethodName :
DEFAULT_OPERATION_NAME;
+
+AbstractSpan span = ContextManager.createEntrySpan(operationName, new
ContextCarrier());
Review comment:
Thank you, I will read the plugin dev document.
I referenced [elasticjob
plugin](https://github.com/apache/skywalking/blob/master/apm-sniffer/apm-sdk-plugin/elastic-job-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/elasticjob/ElasticJobExecutorInterceptor.java#L39)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [skywalking] hailin0 commented on a change in pull request #5339: Add support for spring @Scheduled
hailin0 commented on a change in pull request #5339: URL: https://github.com/apache/skywalking/pull/5339#discussion_r471922334 ## File path: test/plugin/scenarios/spring-scheduled-scenario/config/expectedData.yaml ## @@ -0,0 +1,48 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +segmentItems: + - serviceName: spring-scheduled-scenario +segmentSize: ge 2 +segments: Review comment: Yes, it's on [line 37](https://github.com/apache/skywalking/pull/5339/files/7f6bba6a27a0ba0ccb41c639f0002e5276b7de3a#diff-f1c217f8303ef2da8c91cd14aaa67300R37). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
