You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/03/20 11:35:13 UTC

[GitHub] [skywalking] mrproliu opened a new pull request #4546: Provide HTTP parameter in the profiling contet

mrproliu opened a new pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546
 
 
   
   Please answer these questions before submitting pull request
   
   - Why submit this pull request?
   - [ ] Bug fix
   - [x] New feature provided
   - [ ] Improve performance
   
   - Related issues
   #4542 
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050736
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
+     */
+    NONE,
+
+    /**
+     * Prepare to profile, when {@link ProfileTask#getMinDurationThreshold()} is reached,
+     * the status will be changed to profiling, and the thread snapshot will be started
+     */
+    PENDING,
+
+    /**
+     * Profile operation has been started, and dump thread snapshot will continue
 
 Review comment:
   ```suggestion
        * Profile operation has been started.
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395635058
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/AbstractTracerContext.java
 ##########
 @@ -115,4 +116,10 @@
      */
     void asyncStop(AsyncSpan span);
 
+    /**
+     * Get current context profiler
+     * @return null if not profiling
+     */
+    ThreadProfiler profiler();
 
 Review comment:
   We should not open this heavy object for this. Right now, only true/false should be opened. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050572
 
 

 ##########
 File path: apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java
 ##########
 @@ -83,32 +83,26 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
         SpanLayer.asHttp(span);
 
         if (Config.Plugin.Tomcat.COLLECT_HTTP_PARAMS) {
-            final Map<String, String[]> parameterMap = new HashMap<>();
-            final org.apache.coyote.Request coyoteRequest = request.getCoyoteRequest();
-            final Parameters parameters = coyoteRequest.getParameters();
-            for (final Enumeration<String> names = parameters.getParameterNames(); names.hasMoreElements(); ) {
-                final String name = names.nextElement();
-                parameterMap.put(name, parameters.getParameterValues(name));
-            }
-
-            if (!parameterMap.isEmpty()) {
-                String tagValue = CollectionUtil.toString(parameterMap);
-                tagValue = Config.Plugin.Http.HTTP_PARAMS_LENGTH_THRESHOLD > 0 ? StringUtil.cut(tagValue, Config.Plugin.Http.HTTP_PARAMS_LENGTH_THRESHOLD) : tagValue;
-                Tags.HTTP.PARAMS.set(span, tagValue);
-            }
+            collectHttpParam(request, span);
         }
     }
 
     @Override
     public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
         Object ret) throws Throwable {
+        Request request = (Request) allArguments[0];
         HttpServletResponse response = (HttpServletResponse) allArguments[1];
 
         AbstractSpan span = ContextManager.activeSpan();
         if (IS_SERVLET_GET_STATUS_METHOD_EXIST && response.getStatus() >= 400) {
             span.errorOccurred();
             Tags.STATUS_CODE.set(span, Integer.toString(response.getStatus()));
         }
+        // Active HTTP parameter collection automatically in the profiling context.
+        // https://github.com/apache/skywalking/issues/4542
 
 Review comment:
   Don't refer to issue, unless there is something special, because if don't follow this, we will have thousands of issue links in the codes.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395992943
 
 

 ##########
 File path: apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
 ##########
 @@ -185,6 +180,12 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
                 ContextManager.getRuntimeContext().remove(CONTROLLER_METHOD_STACK_DEPTH);
             }
 
+            // Active HTTP parameter collection automatically in the profiling context.
+            // https://github.com/apache/skywalking/issues/4542
+            if (!Config.Plugin.Tomcat.COLLECT_HTTP_PARAMS && span.isProfiling()) {
 
 Review comment:
   `Config.Plugin.SpringMVC.COLLECT_HTTP_PARAMS`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395692162
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##########
 @@ -106,9 +107,9 @@
     private final long createTime;
 
     /**
-     * profiling status
+     * profiling data
      */
-    private volatile boolean profiling;
+    private volatile ThreadProfiler profiler;
 
 Review comment:
   Or you should refactor this as an enum, having value, NONE, PENDING and PROFILING. Also, this enum should have a method called `isBeingWatched`. PENDING and PROFILING should return yes.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050769
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatusReference.java
 ##########
 @@ -0,0 +1,66 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Wrapper {@link ProfileStatus}, make sure {@link org.apache.skywalking.apm.agent.core.context.TracingContext} with {@link ThreadProfiler} have same reference with {@link ProfileStatus},
+ * And only the profile module could change the status
+ */
+public class ProfileStatusReference {
+
+    private volatile ProfileStatus status;
+
+    private ProfileStatusReference(ProfileStatus status) {
+        this.status = status;
+    }
+
+    /**
+     * Create with not watching
+     */
+    public static ProfileStatusReference createWithNone() {
+        return new ProfileStatusReference(ProfileStatus.NONE);
+    }
+
+    /**
+     * Create with padding to profile
+     */
+    public static ProfileStatusReference createWithPadding() {
+        return new ProfileStatusReference(ProfileStatus.PENDING);
+    }
+
+    public ProfileStatus get() {
+        return this.status;
+    }
+
+    public boolean isBeingWatched() {
 
 Review comment:
   ```suggestion
       /**
        * The profile monitoring is watching, wait for some profile conditions.
        */
       public boolean isBeingWatched() {
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395635849
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ContextManager.java
 ##########
 @@ -224,4 +226,17 @@ public static RuntimeContext getRuntimeContext() {
 
         return runtimeContext;
     }
+
+    /**
+     * Get current context profiling status
+     * @return
+     */
+    public static ProfilingStatus getProfilingStatus() {
+        final AbstractTracerContext context = get();
 
 Review comment:
   This will cost unexpected resource. We should use `span` to delegate. Such as `span#isProfiling`.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io commented on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601670784
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=h1) Report
   > Merging [#4546](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/5251cc7239d184d3b72069373cd83a3cb84e7482?src=pr&el=desc) will **decrease** coverage by `0.03%`.
   > The diff coverage is `13.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4546/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4546      +/-   ##
   ==========================================
   - Coverage   25.55%   25.52%   -0.04%     
   ==========================================
     Files        1245     1245              
     Lines       28961    28977      +16     
     Branches     3973     3977       +4     
   ==========================================
   - Hits         7402     7397       -5     
   - Misses      20869    20889      +20     
   - Partials      690      691       +1
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...gent/core/profile/ProfileTaskExecutionContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvbkNvbnRleHQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...commons/interceptor/AbstractMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vc3ByaW5nLXBsdWdpbnMvbXZjLWFubm90YXRpb24tY29tbW9ucy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9hcG0vcGx1Z2luL3NwcmluZy9tdmMvY29tbW9ucy9pbnRlcmNlcHRvci9BYnN0cmFjdE1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
   | [...g/apm/agent/core/context/IgnoredTracerContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9JZ25vcmVkVHJhY2VyQ29udGV4dC5qYXZh) | `68.96% <0%> (-2.47%)` | :arrow_down: |
   | [...walking/apm/agent/core/context/ContextManager.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9Db250ZXh0TWFuYWdlci5qYXZh) | `49.43% <0%> (-2.95%)` | :arrow_down: |
   | [...walking/apm/agent/core/context/TracingContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9UcmFjaW5nQ29udGV4dC5qYXZh) | `61.44% <100%> (+0.16%)` | :arrow_up: |
   | [.../apm/plugin/tomcat78x/TomcatInvokeInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vdG9tY2F0LTcueC04LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdG9tY2F0Nzh4L1RvbWNhdEludm9rZUludGVyY2VwdG9yLmphdmE=) | `55.81% <6.25%> (-4.72%)` | :arrow_down: |
   | [...gent/core/profile/ProfileTaskExecutionService.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvblNlcnZpY2UuamF2YQ==) | `26.66% <66.66%> (ø)` | :arrow_up: |
   | [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `0% <0%> (-100%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=footer). Last update [5251cc7...f020112](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng merged pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395991846
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
+     */
+    NONE,
+
+    /**
+     * Prepare to profile, when {@link ProfileTask#getMinDurationThreshold()} is reached,
+     * the status will be changed to profiling, and the thread snapshot will be started
+     */
+    PADDING,
 
 Review comment:
   `PADDING`? do you mean `PENDING`? 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396049971
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
+     */
+    NONE,
+
+    /**
+     * Prepare to profile, when {@link ProfileTask#getMinDurationThreshold()} is reached,
 
 Review comment:
   ```suggestion
        * Prepare to profile, until {@link ProfileTask#getMinDurationThreshold()} reached,
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601670784
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=h1) Report
   > Merging [#4546](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/2e9f0016332ec4d3d79393897c7985eda2376fd6&el=desc) will **increase** coverage by `0.00%`.
   > The diff coverage is `21.05%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4546/graphs/tree.svg?width=650&height=150&src=pr&token=qrILxY5yA8)](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #4546   +/-   ##
   =======================================
     Coverage   25.55%   25.56%           
   =======================================
     Files        1245     1246    +1     
     Lines       28961    28981   +20     
     Branches     3973     3977    +4     
   =======================================
   + Hits         7402     7410    +8     
   - Misses      20869    20879   +10     
   - Partials      690      692    +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...walking/apm/agent/core/context/ContextManager.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9Db250ZXh0TWFuYWdlci5qYXZh) | `52.38% <ø> (ø)` | |
   | [.../agent/core/context/trace/AbstractTracingSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9BYnN0cmFjdFRyYWNpbmdTcGFuLmphdmE=) | `59.82% <0.00%> (-0.54%)` | :arrow_down: |
   | [...walking/apm/agent/core/context/trace/NoopSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9Ob29wU3Bhbi5qYXZh) | `0.00% <ø> (ø)` | |
   | [...gent/core/profile/ProfileTaskExecutionContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvbkNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...ywalking/apm/agent/core/profile/ProfileThread.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGhyZWFkLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...walking/apm/agent/core/profile/ThreadProfiler.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9UaHJlYWRQcm9maWxlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...commons/interceptor/AbstractMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vc3ByaW5nLXBsdWdpbnMvbXZjLWFubm90YXRpb24tY29tbW9ucy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9hcG0vcGx1Z2luL3NwcmluZy9tdmMvY29tbW9ucy9pbnRlcmNlcHRvci9BYnN0cmFjdE1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../apm/plugin/tomcat78x/TomcatInvokeInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vdG9tY2F0LTcueC04LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdG9tY2F0Nzh4L1RvbWNhdEludm9rZUludGVyY2VwdG9yLmphdmE=) | `55.81% <6.25%> (-4.72%)` | :arrow_down: |
   | [...apm/agent/core/profile/ProfileStatusReference.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlU3RhdHVzUmVmZXJlbmNlLmphdmE=) | `16.66% <16.66%> (ø)` | |
   | [...gent/core/profile/ProfileTaskExecutionService.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvblNlcnZpY2UuamF2YQ==) | `26.31% <50.00%> (-0.36%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=footer). Last update [2e9f001...aa82ad6](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601670784
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=h1) Report
   > Merging [#4546](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/2e9f0016332ec4d3d79393897c7985eda2376fd6?src=pr&el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `21.05%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4546/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4546      +/-   ##
   ==========================================
   - Coverage   25.55%   25.54%   -0.02%     
   ==========================================
     Files        1245     1246       +1     
     Lines       28961    28981      +20     
     Branches     3973     3977       +4     
   ==========================================
   + Hits         7402     7403       +1     
   - Misses      20869    20886      +17     
   - Partials      690      692       +2
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...walking/apm/agent/core/context/trace/NoopSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9Ob29wU3Bhbi5qYXZh) | `0% <ø> (ø)` | :arrow_up: |
   | [...walking/apm/agent/core/context/ContextManager.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9Db250ZXh0TWFuYWdlci5qYXZh) | `52.38% <ø> (ø)` | :arrow_up: |
   | [...ywalking/apm/agent/core/profile/ProfileThread.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGhyZWFkLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
   | [...gent/core/profile/ProfileTaskExecutionContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvbkNvbnRleHQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [.../agent/core/context/trace/AbstractTracingSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9BYnN0cmFjdFRyYWNpbmdTcGFuLmphdmE=) | `59.82% <0%> (-0.54%)` | :arrow_down: |
   | [...commons/interceptor/AbstractMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vc3ByaW5nLXBsdWdpbnMvbXZjLWFubm90YXRpb24tY29tbW9ucy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9hcG0vcGx1Z2luL3NwcmluZy9tdmMvY29tbW9ucy9pbnRlcmNlcHRvci9BYnN0cmFjdE1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
   | [...walking/apm/agent/core/profile/ThreadProfiler.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9UaHJlYWRQcm9maWxlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...walking/apm/agent/core/context/TracingContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9UcmFjaW5nQ29udGV4dC5qYXZh) | `61.44% <100%> (+0.16%)` | :arrow_up: |
   | [...ywalking/apm/agent/core/profile/ProfileStatus.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlU3RhdHVzLmphdmE=) | `100% <100%> (ø)` | |
   | [...apm/agent/core/profile/ProfileStatusReference.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlU3RhdHVzUmVmZXJlbmNlLmphdmE=) | `16.66% <16.66%> (ø)` | |
   | ... and [5 more](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=footer). Last update [2e9f001...aa82ad6](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395991749
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/trace/AbstractSpan.java
 ##########
 @@ -126,4 +126,11 @@
     AbstractSpan start(long startTime);
 
     AbstractSpan setPeer(String remotePeer);
+
+    /**
+     * Check current context has profiling
+     *
+     * @return
+     */
 
 Review comment:
   either remove this or add descriptive comments

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601670784
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=h1) Report
   > Merging [#4546](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/2e9f0016332ec4d3d79393897c7985eda2376fd6&el=desc) will **decrease** coverage by `0.03%`.
   > The diff coverage is `13.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4546/graphs/tree.svg?width=650&height=150&src=pr&token=qrILxY5yA8)](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4546      +/-   ##
   ==========================================
   - Coverage   25.55%   25.52%   -0.04%     
   ==========================================
     Files        1245     1245              
     Lines       28961    28977      +16     
     Branches     3973     3977       +4     
   ==========================================
   - Hits         7402     7397       -5     
   - Misses      20869    20889      +20     
   - Partials      690      691       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...walking/apm/agent/core/context/ContextManager.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9Db250ZXh0TWFuYWdlci5qYXZh) | `49.43% <0.00%> (-2.95%)` | :arrow_down: |
   | [...g/apm/agent/core/context/IgnoredTracerContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9JZ25vcmVkVHJhY2VyQ29udGV4dC5qYXZh) | `68.96% <0.00%> (-2.47%)` | :arrow_down: |
   | [...gent/core/profile/ProfileTaskExecutionContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvbkNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...commons/interceptor/AbstractMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vc3ByaW5nLXBsdWdpbnMvbXZjLWFubm90YXRpb24tY29tbW9ucy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9hcG0vcGx1Z2luL3NwcmluZy9tdmMvY29tbW9ucy9pbnRlcmNlcHRvci9BYnN0cmFjdE1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../apm/plugin/tomcat78x/TomcatInvokeInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vdG9tY2F0LTcueC04LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdG9tY2F0Nzh4L1RvbWNhdEludm9rZUludGVyY2VwdG9yLmphdmE=) | `55.81% <6.25%> (-4.72%)` | :arrow_down: |
   | [...gent/core/profile/ProfileTaskExecutionService.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvblNlcnZpY2UuamF2YQ==) | `26.66% <66.66%> (ø)` | |
   | [...walking/apm/agent/core/context/TracingContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9UcmFjaW5nQ29udGV4dC5qYXZh) | `61.44% <100.00%> (+0.16%)` | :arrow_up: |
   | [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=footer). Last update [2e9f001...3ef76fa](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395668307
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##########
 @@ -106,9 +107,9 @@
     private final long createTime;
 
     /**
-     * profiling status
+     * profiling data
      */
-    private volatile boolean profiling;
+    private volatile ThreadProfiler profiler;
 
 Review comment:
   What do you mean ready? The parameter collection only happens after the profile activated.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] mrproliu commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395673591
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##########
 @@ -106,9 +107,9 @@
     private final long createTime;
 
     /**
-     * profiling status
+     * profiling data
      */
-    private volatile boolean profiling;
+    private volatile ThreadProfiler profiler;
 
 Review comment:
   The profile has `minDurationThreshold` to explain only start profiling when bigger this parameter, before this time, it was ready status. I think on the ready status, we don't need to start the HTTP parameter happens.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050256
 
 

 ##########
 File path: apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
 ##########
 @@ -185,6 +180,12 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
                 ContextManager.getRuntimeContext().remove(CONTROLLER_METHOD_STACK_DEPTH);
             }
 
+            // Active HTTP parameter collection automatically in the profiling context.
+            // https://github.com/apache/skywalking/issues/4542
 
 Review comment:
   Why refer an issue here? Anything special?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050026
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
+     */
+    NONE,
+
+    /**
+     * Prepare to profile, when {@link ProfileTask#getMinDurationThreshold()} is reached,
+     * the status will be changed to profiling, and the thread snapshot will be started
 
 Review comment:
   ```suggestion
        * Once the status changed to profiling, the thread snapshot is officially started
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050851
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/trace/AbstractSpan.java
 ##########
 @@ -126,4 +126,9 @@
     AbstractSpan start(long startTime);
 
     AbstractSpan setPeer(String remotePeer);
+
+    /**
+     * Check current context has profiling
 
 Review comment:
   ```suggestion
        * @return true if the span's owner(tracing context main thread) is been profiled.
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] mrproliu commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
mrproliu commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395642921
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##########
 @@ -106,9 +107,9 @@
     private final long createTime;
 
     /**
-     * profiling status
+     * profiling data
      */
-    private volatile boolean profiling;
+    private volatile ThreadProfiler profiler;
 
 Review comment:
   Maybe I need to change it to `ProfilingStatus` because we need the status to check the current context is profiling or ready.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395991969
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatusReference.java
 ##########
 @@ -0,0 +1,68 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Wrapper {@link ProfileStatus}, make sure {@link org.apache.skywalking.apm.agent.core.context.TracingContext} with {@link ThreadProfiler} have same reference with {@link ProfileStatus},
+ * And only the profile module could change the status
+ */
+public class ProfileStatusReference {
+
+    private volatile ProfileStatus status;
+
+    private ProfileStatusReference(ProfileStatus status) {
+        this.status = status;
+    }
+
+    /**
+     * Create with not watching
+     * @return
+     */
+    public static ProfileStatusReference createWithNone() {
+        return new ProfileStatusReference(ProfileStatus.NONE);
+    }
+
+    /**
+     * Create with padding to profile
+     * @return
 
 Review comment:
   ditto

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601670784
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=h1) Report
   > Merging [#4546](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/a9d34e3b6121351dc86dc7148398a83c5a96d4b0&el=desc) will **increase** coverage by `0.03%`.
   > The diff coverage is `21.05%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4546/graphs/tree.svg?width=650&height=150&src=pr&token=qrILxY5yA8)](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4546      +/-   ##
   ==========================================
   + Coverage   25.53%   25.57%   +0.03%     
   ==========================================
     Files        1245     1246       +1     
     Lines       28955    28975      +20     
     Branches     3970     3974       +4     
   ==========================================
   + Hits         7395     7410      +15     
   - Misses      20870    20873       +3     
   - Partials      690      692       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...walking/apm/agent/core/context/ContextManager.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9Db250ZXh0TWFuYWdlci5qYXZh) | `52.38% <ø> (ø)` | |
   | [.../agent/core/context/trace/AbstractTracingSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9BYnN0cmFjdFRyYWNpbmdTcGFuLmphdmE=) | `59.82% <0.00%> (-0.54%)` | :arrow_down: |
   | [...walking/apm/agent/core/context/trace/NoopSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9Ob29wU3Bhbi5qYXZh) | `0.00% <ø> (ø)` | |
   | [...gent/core/profile/ProfileTaskExecutionContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvbkNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...ywalking/apm/agent/core/profile/ProfileThread.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGhyZWFkLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...walking/apm/agent/core/profile/ThreadProfiler.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9UaHJlYWRQcm9maWxlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...commons/interceptor/AbstractMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vc3ByaW5nLXBsdWdpbnMvbXZjLWFubm90YXRpb24tY29tbW9ucy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9hcG0vcGx1Z2luL3NwcmluZy9tdmMvY29tbW9ucy9pbnRlcmNlcHRvci9BYnN0cmFjdE1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../apm/plugin/tomcat78x/TomcatInvokeInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vdG9tY2F0LTcueC04LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdG9tY2F0Nzh4L1RvbWNhdEludm9rZUludGVyY2VwdG9yLmphdmE=) | `55.81% <6.25%> (-4.72%)` | :arrow_down: |
   | [...apm/agent/core/profile/ProfileStatusReference.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlU3RhdHVzUmVmZXJlbmNlLmphdmE=) | `16.66% <16.66%> (ø)` | |
   | [...gent/core/profile/ProfileTaskExecutionService.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvblNlcnZpY2UuamF2YQ==) | `26.31% <50.00%> (-0.36%)` | :arrow_down: |
   | ... and [5 more](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=footer). Last update [a9d34e3...d9ebc31](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] mrproliu commented on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601998469
 
 
   @kezhenxu94 I seem java version e2e test failure, I do a local run, cannot find `adoptopenjdk/openjdk9:alpine-slim` this docker image. Do I miss anything?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050169
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatusReference.java
 ##########
 @@ -0,0 +1,66 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Wrapper {@link ProfileStatus}, make sure {@link org.apache.skywalking.apm.agent.core.context.TracingContext} with {@link ThreadProfiler} have same reference with {@link ProfileStatus},
+ * And only the profile module could change the status
+ */
+public class ProfileStatusReference {
+
+    private volatile ProfileStatus status;
+
+    private ProfileStatusReference(ProfileStatus status) {
+        this.status = status;
+    }
+
+    /**
+     * Create with not watching
+     */
+    public static ProfileStatusReference createWithNone() {
+        return new ProfileStatusReference(ProfileStatus.NONE);
+    }
+
+    /**
+     * Create with padding to profile
+     */
+    public static ProfileStatusReference createWithPadding() {
 
 Review comment:
   I think there is another typo, pending

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050716
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
 
 Review comment:
   ```suggestion
        * No profile
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050104
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
+     */
+    NONE,
+
+    /**
+     * Prepare to profile, when {@link ProfileTask#getMinDurationThreshold()} is reached,
+     * the status will be changed to profiling, and the thread snapshot will be started
+     */
+    PENDING,
+
+    /**
+     * Profile operation has been started, and dump thread snapshot will continue
+     */
+    PROFILING,
+
+    /**
+     * The current {@link org.apache.skywalking.apm.agent.core.context.TracingContext} has finished executing,
+     * or the current thread cannot take a thread snapshot operation again
 
 Review comment:
   ```suggestion
        * or the current thread isn't available.
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395991926
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatusReference.java
 ##########
 @@ -0,0 +1,68 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Wrapper {@link ProfileStatus}, make sure {@link org.apache.skywalking.apm.agent.core.context.TracingContext} with {@link ThreadProfiler} have same reference with {@link ProfileStatus},
+ * And only the profile module could change the status
+ */
+public class ProfileStatusReference {
+
+    private volatile ProfileStatus status;
+
+    private ProfileStatusReference(ProfileStatus status) {
+        this.status = status;
+    }
+
+    /**
+     * Create with not watching
+     * @return
 
 Review comment:
   either remove this or add descriptive comments
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395690273
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##########
 @@ -106,9 +107,9 @@
     private final long createTime;
 
     /**
-     * profiling status
+     * profiling data
      */
-    private volatile boolean profiling;
+    private volatile ThreadProfiler profiler;
 
 Review comment:
   I think we have a naming issue here. The current `profiling` should rename to `pendingProfile`, and we need a real `profiling` to indicate the thread dump started status.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r396050111
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/profile/ProfileStatus.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * 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.agent.core.profile;
+
+/**
+ * Profile status, include entire profile cycle
+ */
+public enum ProfileStatus {
+    /**
+     * Profile not required
+     */
+    NONE,
+
+    /**
+     * Prepare to profile, when {@link ProfileTask#getMinDurationThreshold()} is reached,
+     * the status will be changed to profiling, and the thread snapshot will be started
+     */
+    PENDING,
+
+    /**
+     * Profile operation has been started, and dump thread snapshot will continue
+     */
+    PROFILING,
+
+    /**
+     * The current {@link org.apache.skywalking.apm.agent.core.context.TracingContext} has finished executing,
 
 Review comment:
   ```suggestion
        * The current {@link org.apache.skywalking.apm.agent.core.context.TracingContext} has finished,
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#issuecomment-601670784
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=h1) Report
   > Merging [#4546](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/2e9f0016332ec4d3d79393897c7985eda2376fd6&el=desc) will **increase** coverage by `0.00%`.
   > The diff coverage is `21.05%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4546/graphs/tree.svg?width=650&height=150&src=pr&token=qrILxY5yA8)](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #4546   +/-   ##
   =======================================
     Coverage   25.55%   25.56%           
   =======================================
     Files        1245     1246    +1     
     Lines       28961    28981   +20     
     Branches     3973     3977    +4     
   =======================================
   + Hits         7402     7410    +8     
   - Misses      20869    20879   +10     
   - Partials      690      692    +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...walking/apm/agent/core/context/ContextManager.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC9Db250ZXh0TWFuYWdlci5qYXZh) | `52.38% <ø> (ø)` | |
   | [.../agent/core/context/trace/AbstractTracingSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9BYnN0cmFjdFRyYWNpbmdTcGFuLmphdmE=) | `59.82% <0.00%> (-0.54%)` | :arrow_down: |
   | [...walking/apm/agent/core/context/trace/NoopSpan.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvY29udGV4dC90cmFjZS9Ob29wU3Bhbi5qYXZh) | `0.00% <ø> (ø)` | |
   | [...gent/core/profile/ProfileTaskExecutionContext.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvbkNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...ywalking/apm/agent/core/profile/ProfileThread.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGhyZWFkLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...walking/apm/agent/core/profile/ThreadProfiler.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9UaHJlYWRQcm9maWxlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...commons/interceptor/AbstractMethodInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vc3ByaW5nLXBsdWdpbnMvbXZjLWFubm90YXRpb24tY29tbW9ucy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9hcG0vcGx1Z2luL3NwcmluZy9tdmMvY29tbW9ucy9pbnRlcmNlcHRvci9BYnN0cmFjdE1ldGhvZEludGVyY2VwdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../apm/plugin/tomcat78x/TomcatInvokeInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vdG9tY2F0LTcueC04LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdG9tY2F0Nzh4L1RvbWNhdEludm9rZUludGVyY2VwdG9yLmphdmE=) | `55.81% <6.25%> (-4.72%)` | :arrow_down: |
   | [...apm/agent/core/profile/ProfileStatusReference.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlU3RhdHVzUmVmZXJlbmNlLmphdmE=) | `16.66% <16.66%> (ø)` | |
   | [...gent/core/profile/ProfileTaskExecutionService.java](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0V4ZWN1dGlvblNlcnZpY2UuamF2YQ==) | `26.31% <50.00%> (-0.36%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/skywalking/pull/4546/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=footer). Last update [2e9f001...b68d43d](https://codecov.io/gh/apache/skywalking/pull/4546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4546: Provide HTTP parameter in the profiling contet
URL: https://github.com/apache/skywalking/pull/4546#discussion_r395636182
 
 

 ##########
 File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/TracingContext.java
 ##########
 @@ -106,9 +107,9 @@
     private final long createTime;
 
     /**
-     * profiling status
+     * profiling data
      */
-    private volatile boolean profiling;
+    private volatile ThreadProfiler profiler;
 
 Review comment:
   I don't think we should open this.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services