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