[GitHub] [skywalking] codecov-io commented on issue #4363: Fix non-atomic operations on volatile variables
codecov-io commented on issue #4363: Fix non-atomic operations on volatile variables URL: https://github.com/apache/skywalking/pull/4363#issuecomment-586558394 # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4363?src=pr&el=h1) Report > Merging [#4363](https://codecov.io/gh/apache/skywalking/pull/4363?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/5fed153ee2c51584a855e474ea97e97c6b8ec9ca?src=pr&el=desc) will **decrease** coverage by `0.17%`. > The diff coverage is `60%`. [](https://codecov.io/gh/apache/skywalking/pull/4363?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4363 +/- ## == - Coverage 26.14% 25.97% -0.18% == Files1182 1181 -1 Lines 2692226841 -81 Branches 3714 3702 -12 == - Hits 7040 6972 -68 + Misses 1924819239 -9 + Partials 634 630 -4 ``` | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4363?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...atacarrier/partition/SimpleRollingPartitioner.java](https://codecov.io/gh/apache/skywalking/pull/4363/diff?src=pr&el=tree#diff-YXBtLWNvbW1vbnMvYXBtLWRhdGFjYXJyaWVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9jb21tb25zL2RhdGFjYXJyaWVyL3BhcnRpdGlvbi9TaW1wbGVSb2xsaW5nUGFydGl0aW9uZXIuamF2YQ==) | `75% <100%> (+25%)` | :arrow_up: | | [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4363/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <50%> (ø)` | :arrow_up: | | [...datacarrier/consumer/MultipleChannelsConsumer.java](https://codecov.io/gh/apache/skywalking/pull/4363/diff?src=pr&el=tree#diff-YXBtLWNvbW1vbnMvYXBtLWRhdGFjYXJyaWVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9jb21tb25zL2RhdGFjYXJyaWVyL2NvbnN1bWVyL011bHRpcGxlQ2hhbm5lbHNDb25zdW1lci5qYXZh) | `10.63% <50%> (+1.94%)` | :arrow_up: | | [.../agent/core/profile/ProfileTaskChannelService.java](https://codecov.io/gh/apache/skywalking/pull/4363/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0NoYW5uZWxTZXJ2aWNlLmphdmE=) | `26.88% <0%> (-1.08%)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4363?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4363?src=pr&el=footer). Last update [5fed153...784d53f](https://codecov.io/gh/apache/skywalking/pull/4363?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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 closed pull request #4363: Fix non-atomic operations on volatile variables
kezhenxu94 closed pull request #4363: Fix non-atomic operations on volatile variables URL: https://github.com/apache/skywalking/pull/4363 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4363: Fix non-atomic operations on volatile variables
wu-sheng commented on a change in pull request #4363: Fix non-atomic operations
on volatile variables
URL: https://github.com/apache/skywalking/pull/4363#discussion_r379748470
##
File path:
apm-commons/apm-datacarrier/src/main/java/org/apache/skywalking/apm/commons/datacarrier/consumer/MultipleChannelsConsumer.java
##
@@ -30,12 +31,13 @@
public class MultipleChannelsConsumer extends Thread {
private volatile boolean running;
private volatile ArrayList consumeTargets;
-private volatile long size;
+private final AtomicLong size;
Review comment:
This is just for changed value accessable from other consumer thread. No
requirement for AtomicLong.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4363: Fix non-atomic operations on volatile variables
wu-sheng commented on a change in pull request #4363: Fix non-atomic operations on volatile variables URL: https://github.com/apache/skywalking/pull/4363#discussion_r379748997 ## File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/remote/GRPCChannelManager.java ## @@ -48,7 +49,7 @@ private final List listeners = Collections.synchronizedList(new LinkedList<>()); private volatile List grpcServers; private volatile int selectedIdx = -1; -private volatile int reconnectCount = 0; +private AtomicInteger reconnectCount = new AtomicInteger(0); Review comment: Why AtomicInteger? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4363: Fix non-atomic operations on volatile variables
wu-sheng commented on a change in pull request #4363: Fix non-atomic operations
on volatile variables
URL: https://github.com/apache/skywalking/pull/4363#discussion_r379749532
##
File path:
apm-commons/apm-datacarrier/src/main/java/org/apache/skywalking/apm/commons/datacarrier/partition/SimpleRollingPartitioner.java
##
@@ -18,15 +18,17 @@
package org.apache.skywalking.apm.commons.datacarrier.partition;
+import java.util.concurrent.atomic.AtomicInteger;
+
/**
* use normal int to rolling.
*/
public class SimpleRollingPartitioner implements IDataPartitioner {
-private volatile int i = 0;
+private AtomicInteger i = new AtomicInteger(0);
Review comment:
This will cause performance cost without much meaning. Rolling is for
switching different channels, there is no need to be strict rule.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 opened a new pull request #4363: Fix non-atomic operations on volatile variables
kezhenxu94 opened a new pull request #4363: Fix non-atomic operations on volatile variables URL: https://github.com/apache/skywalking/pull/4363 ### Motivation: Fix non-atomic operations on `volatile` variables. ### Modifications: Replace `int`, `long` with `AtomicInteger` and `AtomicLong`. ### Result: No potential threading visibility issues caused by non-atomic operations on `volatile` variables. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[skywalking] branch master updated (ae33ac8 -> 5fed153)
This is an automated email from the ASF dual-hosted git repository. kezhenxu94 pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/skywalking.git. from ae33ac8 Adjust Armeria plugin to adapt 0.98.0 (#4362) add 5fed153 Add Armeria 0.98.0 to the supported version list (#4328) No new revisions were added by this update. Summary of changes: docs/en/setup/service-agent/java-agent/Supported-list.md | 2 +- test/plugin/scenarios/armeria-0.96plus-scenario/support-version.list | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
[GitHub] [skywalking] kezhenxu94 merged pull request #4328: Add Armeria 0.98.0 to the supported version list
kezhenxu94 merged pull request #4328: Add Armeria 0.98.0 to the supported version list URL: https://github.com/apache/skywalking/pull/4328 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at
OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379715955
##
File path:
test/e2e/e2e-base/src/main/java/org/apache/skywalking/e2e/metrics/MetricsMatcher.java
##
@@ -64,15 +72,23 @@ public static void verifyMetrics(SimpleQueryClient
queryClient, String metricNam
}
public static void verifyPercentileMetrics(SimpleQueryClient queryClient,
String metricName, String id,
-final LocalDateTime minutesAgo, long retryInterval, Runnable
generateTraffic) throws Exception {
+ final LocalDateTime minutesAgo,
long retryInterval, Runnable generateTraffic) throws Exception {
+verifyPercentileMetrics(queryClient, metricName, id, minutesAgo,
LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1), retryInterval,
generateTraffic);
Review comment:
I get it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 merged pull request #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 merged pull request #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[skywalking] branch master updated (ae442e3 -> ae33ac8)
This is an automated email from the ASF dual-hosted git repository.
kezhenxu94 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git.
from ae442e3 More comments for important classes (#4361)
add ae33ac8 Adjust Armeria plugin to adapt 0.98.0 (#4362)
No new revisions were added by this update.
Summary of changes:
.../apm-sdk-plugin/armeria-0.85.x-plugin/pom.xml | 2 +-
.../armeria/Armeria085ClientInterceptor.java | 14 --
.../armeria/Armeria085ServerInterceptor.java | 10 +-
.../armeria/Armeria086ClientInterceptor.java | 14 --
...eptor.java => Armeria098ClientInterceptor.java} | 18 ++
.../plugin/armeria/ArmeriaClientInterceptor.java | 7 +++
.../define/Armeria085ClientInstrumentation.java| 3 +++
.../define/Armeria085ServerInstrumentation.java| 8 ++--
.../define/Armeria086ClientInstrumentation.java| 3 +++
...n.java => Armeria098ClientInstrumentation.java} | 22 +-
.../src/main/resources/skywalking-plugin.def | 3 ++-
11 files changed, 62 insertions(+), 42 deletions(-)
copy
apm-sniffer/apm-sdk-plugin/armeria-0.85.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/armeria/{Armeria085ClientInterceptor.java
=> Armeria098ClientInterceptor.java} (85%)
copy
apm-sniffer/apm-sdk-plugin/armeria-0.85.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/armeria/define/{Armeria085ClientInstrumentation.java
=> Armeria098ClientInstrumentation.java} (77%)
[GitHub] [skywalking] wu-sheng commented on issue #4362: Adapt Armeria plugin to version 0.98.0
wu-sheng commented on issue #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#issuecomment-586549638 Go for that. It is fine. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 edited a comment on issue #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 edited a comment on issue #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#issuecomment-586549323 The failure is due to network issue and I had re-run it several times, I'll merge this after the running tests finished, instead of re-running over and over This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on issue #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 commented on issue #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#issuecomment-586549323 The failure is due to network issue and I had re-run it several times, I'll merge this instead of re-running over and over This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4362: Adapt Armeria plugin to version 0.98.0
codecov-io commented on issue #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#issuecomment-586545936 # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4362?src=pr&el=h1) Report > Merging [#4362](https://codecov.io/gh/apache/skywalking/pull/4362?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/ae442e364f88f3bee35b327f32c07097af00c060?src=pr&el=desc) will **decrease** coverage by `0.01%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/skywalking/pull/4362?src=pr&el=tree) ```diff @@Coverage Diff @@ ## master#4362 +/- ## == - Coverage 26.16% 26.14% -0.02% == Files1181 1182 +1 Lines 2691026922 +12 Branches 3713 3714 +1 == - Hits 7040 7039 -1 - Misses 1923619248 +12 - Partials 634 635 +1 ``` | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4362?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [...pm/plugin/armeria/Armeria085ServerInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4362/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDg1U2VydmVySW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: | | [...pm/plugin/armeria/Armeria085ClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4362/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDg1Q2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: | | [...pm/plugin/armeria/Armeria086ClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4362/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDg2Q2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: | | [...g/apm/plugin/armeria/ArmeriaClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4362/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhQ2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: | | [...pm/plugin/armeria/Armeria098ClientInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4362/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLXNkay1wbHVnaW4vYXJtZXJpYS0wLjg1LngtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vYXJtZXJpYS9Bcm1lcmlhMDk4Q2xpZW50SW50ZXJjZXB0b3IuamF2YQ==) | `0% <0%> (ø)` | | | [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4362/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `58.33% <0%> (-1.67%)` | :arrow_down: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4362?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4362?src=pr&el=footer). Last update [ae442e3...e4196e5](https://codecov.io/gh/apache/skywalking/pull/4362?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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4362: Adapt Armeria plugin to version 0.98.0
wu-sheng commented on issue #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#issuecomment-586542152 Tests seem passed, please continue. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379514119
##
File path:
test/e2e/e2e-base/src/main/java/org/apache/skywalking/e2e/metrics/MetricsMatcher.java
##
@@ -64,15 +72,23 @@ public static void verifyMetrics(SimpleQueryClient
queryClient, String metricNam
}
public static void verifyPercentileMetrics(SimpleQueryClient queryClient,
String metricName, String id,
-final LocalDateTime minutesAgo, long retryInterval, Runnable
generateTraffic) throws Exception {
+ final LocalDateTime minutesAgo,
long retryInterval, Runnable generateTraffic) throws Exception {
+verifyPercentileMetrics(queryClient, metricName, id, minutesAgo,
LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1), retryInterval,
generateTraffic);
Review comment:
My point is, if you want to change, why don't change the existing method?
Instead, you add a new method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0
wu-sheng commented on a change in pull request #4362: Adapt Armeria plugin to
version 0.98.0
URL: https://github.com/apache/skywalking/pull/4362#discussion_r379510143
##
File path:
apm-sniffer/apm-sdk-plugin/armeria-0.85.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/armeria/define/Armeria086ClientInstrumentation.java
##
@@ -32,6 +32,7 @@
public class Armeria086ClientInstrumentation extends
ClassInstanceMethodsEnhancePluginDefine {
Review comment:
> For example Armeria096ClientInstrumentation indicates that it supports
0.96+
That totally works for me.
> But I’m recently consider to rewrite the plugin from 1.0.0 with a more
recommended tracing solution in Armeria, decorator, but that’s another story.
If that could replace some of the existing plugins, we could delete them
later. It is not an issue.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at
OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379509831
##
File path:
test/e2e/e2e-base/src/main/java/org/apache/skywalking/e2e/metrics/MetricsMatcher.java
##
@@ -64,15 +72,23 @@ public static void verifyMetrics(SimpleQueryClient
queryClient, String metricNam
}
public static void verifyPercentileMetrics(SimpleQueryClient queryClient,
String metricName, String id,
-final LocalDateTime minutesAgo, long retryInterval, Runnable
generateTraffic) throws Exception {
+ final LocalDateTime minutesAgo,
long retryInterval, Runnable generateTraffic) throws Exception {
+verifyPercentileMetrics(queryClient, metricName, id, minutesAgo,
LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1), retryInterval,
generateTraffic);
Review comment:
Convenient debug, set end time according to local TZ.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379504306
##
File path:
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling
downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " +
downsampling.name());
}
+
+public DateTime startTimeBucket2DateTime(Downsampling downsampling, long
startTB) {
+switch (downsampling) {
+case Month:
+return MM.parseDateTime(String.valueOf(startTB));
+case Day:
+return MMDD.parseDateTime(String.valueOf(startTB));
+case Hour:
+return MMDDHH.parseDateTime(String.valueOf(startTB));
+case Minute:
+return MMDDHHMM.parseDateTime(String.valueOf(startTB));
+case Second:
+return MMDDHHMMSS.parseDateTime(String.valueOf(startTB));
+}
+throw new UnexpectedException("Unexpected downsampling: " +
downsampling.name());
+}
+
+public DateTime endTimeBucket2DateTime(Downsampling downsampling, long
endTB) {
+switch (downsampling) {
+case Month:
+return MM.parseDateTime(String.valueOf(endTB));
+case Day:
+return MMDD.parseDateTime(String.valueOf(endTB));
+case Hour:
+return MMDDHH.parseDateTime(String.valueOf(endTB));
+case Minute:
+return MMDDHHMM.parseDateTime(String.valueOf(endTB));
+case Second:
+return MMDDHHMMSS.parseDateTime(String.valueOf(endTB));
+}
+throw new UnexpectedException("Unexpected downsampling: " +
downsampling.name());
Review comment:
ack, for the day and month, different circulation methods are used.
Considering that the monthly increase calculation should be time-consuming, it
is still the original method. WDYT?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#discussion_r379502634 ## File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java ## @@ -291,6 +291,24 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB return client.search(searchRequest); } +/** + * Search results from ES search engine according to various search conditions, + * Note the method is usered for the list of index names is optimized based on + * the scope of startTimeBucket and endTimeBucket + * @param indexNameList full index names list base on timebucket scope. + * Except for endpoint_inventory network_address_inventory service_inventory service_instance_inventory + * @param searchSourceBuilder Various search query conditions + * @return ES search query results + * @throws IOException throw IOException Review comment: ack This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 commented on a change in pull request #4362: Adapt Armeria plugin to
version 0.98.0
URL: https://github.com/apache/skywalking/pull/4362#discussion_r379497366
##
File path:
apm-sniffer/apm-sdk-plugin/armeria-0.85.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/armeria/define/Armeria086ClientInstrumentation.java
##
@@ -32,6 +32,7 @@
public class Armeria086ClientInstrumentation extends
ClassInstanceMethodsEnhancePluginDefine {
Review comment:
Sure, what about renaming to the lowest version that the instrumentation
supports from? For example Armeria096ClientInstrumentation indicates that it
supports 0.96+, and if it supports all future versions that would be best, if
it doesn’t support from, for example 1.2.3, we will have another Armeria123...?
But I’m recently consider to rewrite the plugin from 1.0.0 with a more
recommended tracing solution in Armeria, decorator, but that’s another story.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0
wu-sheng commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#discussion_r379493454 ## File path: test/plugin/scenarios/armeria-0.96plus-scenario/support-version.list ## @@ -14,5 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. +0.98.0 Review comment: Make sense to me. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0
wu-sheng commented on a change in pull request #4362: Adapt Armeria plugin to
version 0.98.0
URL: https://github.com/apache/skywalking/pull/4362#discussion_r379491221
##
File path:
apm-sniffer/apm-sdk-plugin/armeria-0.85.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/armeria/define/Armeria086ClientInstrumentation.java
##
@@ -32,6 +32,7 @@
public class Armeria086ClientInstrumentation extends
ClassInstanceMethodsEnhancePluginDefine {
Review comment:
This class name is not suitable anymore. Please consider to refactor?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on issue #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 commented on issue #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#issuecomment-586335019 here are the changes between 0.97.0 and 0.98.0 that breaks the plugin https://github.com/line/armeria/blob/aba612a0e6b980334c5a40818047efe2aecc8e4e/core/src/main/java/com/linecorp/armeria/client/UserClient.java#L133-L135 https://github.com/line/armeria/blob/d5ba7e73cd6d4df72e6ed55fe9153b6f7a7f19e9/core/src/main/java/com/linecorp/armeria/client/UserClient.java#L145-L147 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 commented on a change in pull request #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362#discussion_r379488858 ## File path: test/plugin/scenarios/armeria-0.96plus-scenario/support-version.list ## @@ -14,5 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. +0.98.0 Review comment: @wu-sheng let me add this version to verify it works (although I verified locally), and revert this to merge #4328 :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 opened a new pull request #4362: Adapt Armeria plugin to version 0.98.0
kezhenxu94 opened a new pull request #4362: Adapt Armeria plugin to version 0.98.0 URL: https://github.com/apache/skywalking/pull/4362 ### Motivation: Support Armeria 0.98.0 in its plugin. ### Modifications: Add an intercept point that is refactored in Armeria 0.98. ### Result: Now Armeria plugin supports 0.98.0, and 0.98.1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379436664
##
File path:
test/e2e/e2e-base/src/main/java/org/apache/skywalking/e2e/metrics/MetricsMatcher.java
##
@@ -64,15 +72,23 @@ public static void verifyMetrics(SimpleQueryClient
queryClient, String metricNam
}
public static void verifyPercentileMetrics(SimpleQueryClient queryClient,
String metricName, String id,
-final LocalDateTime minutesAgo, long retryInterval, Runnable
generateTraffic) throws Exception {
+ final LocalDateTime minutesAgo,
long retryInterval, Runnable generateTraffic) throws Exception {
+verifyPercentileMetrics(queryClient, metricName, id, minutesAgo,
LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1), retryInterval,
generateTraffic);
Review comment:
Why change it? Please give me more details?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at
OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379435243
##
File path:
test/e2e/e2e-base/src/main/java/org/apache/skywalking/e2e/metrics/MetricsMatcher.java
##
@@ -64,15 +72,23 @@ public static void verifyMetrics(SimpleQueryClient
queryClient, String metricNam
}
public static void verifyPercentileMetrics(SimpleQueryClient queryClient,
String metricName, String id,
-final LocalDateTime minutesAgo, long retryInterval, Runnable
generateTraffic) throws Exception {
+ final LocalDateTime minutesAgo,
long retryInterval, Runnable generateTraffic) throws Exception {
+verifyPercentileMetrics(queryClient, metricName, id, minutesAgo,
LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1), retryInterval,
generateTraffic);
Review comment:
Before that was the UTC TZ, I just overload `verifyMetrics`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379416559
##
File path:
test/e2e/e2e-browser/e2e-browser-es/src/test/java/org/apache/skywalking/e2e/ESWithBrowserPerfITCase.java
##
@@ -0,0 +1,26 @@
+/*
+ * 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.e2e;
+
+import lombok.extern.slf4j.Slf4j;
+import org.apache.skywalking.e2e.browser.BrowserPerfITCase;
+
+@Slf4j
+public class ESWithBrowserPerfITCase extends BrowserPerfITCase {
Review comment:
Storage should be based on backend storage settings, why test case cares
about this? Especially, there is nothing different with `BrowserPerfITCase`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379416071
##
File path:
test/e2e/e2e-base/src/main/java/org/apache/skywalking/e2e/metrics/MetricsMatcher.java
##
@@ -64,15 +72,23 @@ public static void verifyMetrics(SimpleQueryClient
queryClient, String metricNam
}
public static void verifyPercentileMetrics(SimpleQueryClient queryClient,
String metricName, String id,
-final LocalDateTime minutesAgo, long retryInterval, Runnable
generateTraffic) throws Exception {
+ final LocalDateTime minutesAgo,
long retryInterval, Runnable generateTraffic) throws Exception {
+verifyPercentileMetrics(queryClient, metricName, id, minutesAgo,
LocalDateTime.now(ZoneOffset.UTC).plusMinutes(1), retryInterval,
generateTraffic);
Review comment:
Why use UTC TZ specifically? Could you explain?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379413452
##
File path:
oap-server/server-receiver-plugin/skywalking-browser-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/recevier/browser/provider/parser/errorlog/listener/BrowserErrorLogListener.java
##
@@ -0,0 +1,30 @@
+/*
+ * 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.oap.server.recevier.browser.provider.parser.errorlog.listener;
+
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.decorator.BrowserErrorLogCoreInfo;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.decorator.BrowserErrorLogDecorator;
+
+public interface BrowserErrorLogListener {
Review comment:
Comments for the interface and all implementations are required.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379412514
##
File path:
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/browser/source/BrowserAppErrorTraffic.java
##
@@ -0,0 +1,52 @@
+/*
+ * 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.oap.server.core.browser.source;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.oap.server.core.source.ScopeDeclaration;
+import org.apache.skywalking.oap.server.core.source.ScopeDefaultColumn;
+import org.apache.skywalking.oap.server.core.source.Source;
+
+import static
org.apache.skywalking.oap.server.core.source.DefaultScopeDefine.BROWSER_APP_ERROR_TRAFFIC;
+import static
org.apache.skywalking.oap.server.core.source.DefaultScopeDefine.SERVICE_CATALOG_NAME;
+
+@ScopeDeclaration(id = BROWSER_APP_ERROR_TRAFFIC, name =
"BrowserAppErrorTraffic", catalog = SERVICE_CATALOG_NAME)
+@ScopeDefaultColumn.VirtualColumnDefinition(fieldName = "entityId", columnName
= "entity_id", isID = true, type = String.class)
+public class BrowserAppErrorTraffic extends Source {
Review comment:
Just for confirmation, is this source forwarding to OAL and manual record
entity at the same time?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379420025 ## File path: .github/workflows/e2e.yaml ## @@ -118,3 +118,19 @@ jobs: run: export E2E_VERSION=jdk8-1.3 && bash -x test/e2e/run.sh e2e-profile/e2e-profile-test-runner --storage=elasticsearch - name: Profile Tests ES7(JDK8) run: export E2E_VERSION=jdk8-1.3 DIST_PACKAGE=apache-skywalking-apm-bin-es7.tar.gz ES_VERSION=7.4.2 && bash -x test/e2e/run.sh e2e-profile/e2e-profile-test-runner --storage=elasticsearch + + Browser: +runs-on: ubuntu-latest +steps: + - uses: actions/checkout@v1 +with: + submodules: true + - name: Set environment +run: export MAVEN_OPTS='-Dmaven.repo.local=~/.m2/repository -XX:+TieredCompilation -XX:TieredStopAtLevel=1 -XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -XX:-UseGCOverheadLimit -Xmx3g' + - name: Compile & Install Test Codes +run: | + ./mvnw checkstyle:check apache-rat:check + ./mvnw -Dcheckstyle.skip -Drat.skip -T2 -Dmaven.compile.fork -Dmaven.compiler.maxmem=3072 -DskipTests clean install + ./mvnw -f test/e2e/pom.xml -pl e2e-base clean install + - name: Browser Tests ES6(JDK8) +run: export E2E_VERSION=jdk8-1.3 && bash -x test/e2e/run.sh e2e-browser/e2e-browser-es Review comment: Please add the tests for different storage. And browser e2e tests should be in an separated `e2e-browser-perf.yaml` file. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379413621
##
File path:
oap-server/server-receiver-plugin/skywalking-browser-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/recevier/browser/provider/parser/errorlog/BrowserErrorLogParser.java
##
@@ -0,0 +1,197 @@
+/*
+ * 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.oap.server.recevier.browser.provider.parser.errorlog;
+
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.skywalking.apm.network.language.agent.BrowserErrorLog;
+import org.apache.skywalking.oap.server.core.Const;
+import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.analysis.TimeBucket;
+import
org.apache.skywalking.oap.server.core.cache.ServiceInstanceInventoryCache;
+import org.apache.skywalking.oap.server.library.buffer.BufferData;
+import org.apache.skywalking.oap.server.library.buffer.DataStreamReader;
+import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.BrowserServiceModuleConfig;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.BrowserDataSource;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.decorator.BrowserErrorLogCoreInfo;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.decorator.BrowserErrorLogDecorator;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.listener.BrowserErrorLogListener;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.standardization.BrowserErrorLogStandardization;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.standardization.BrowserErrorLogStandardizationWorker;
+import
org.apache.skywalking.oap.server.recevier.browser.provider.parser.errorlog.standardization.PagePathIdExchanger;
+import org.apache.skywalking.oap.server.telemetry.TelemetryModule;
+import org.apache.skywalking.oap.server.telemetry.api.CounterMetrics;
+import org.apache.skywalking.oap.server.telemetry.api.MetricsCreator;
+import org.apache.skywalking.oap.server.telemetry.api.MetricsTag;
+
+import java.util.LinkedList;
+import java.util.List;
+
+@Slf4j
+public class BrowserErrorLogParser {
Review comment:
Comments are required.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379407019 ## File path: oap-server/server-bootstrap/src/main/resources/browser_analysis.oal ## @@ -0,0 +1,56 @@ +/* +* 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. +* +*/ + +// browser app +browser_app_pv = from(BrowserAppPerf.count).sum(); +browser_app_error_rate = from(BrowserAppPerf.*).percent(status == false) +browser_app_error_sum = from(BrowserAppErrorTraffic.count).sum(); + +// browser app single version +browser_app_single_version_pv = from(BrowserAppSingleVersionPerf.count).sum(); +browser_app_single_version_error_rate = from(BrowserAppSingleVersionPerf.*).percent(status == false) + +// browser app page +browser_app_page_pv = from(BrowserAppPagePerf.count).sum(); +browser_app_page_error_rate = from(BrowserAppPagePerf.*).percent(status == false) +browser_app_page_error_sum = from(BrowserAppPageErrorTraffic.count).sum(); + +browser_app_page_ajax_error_sum = from(BrowserAppPageErrorTraffic.count).filter(category == BrowserErrorCategory.AJAX).sum(); +browsre_app_page_resource_error_sum = from(BrowserAppPageErrorTraffic.count).filter(category == BrowserErrorCategory.RESOURCE).sum(); +browsre_app_page_vue_error_sum = from(BrowserAppPageErrorTraffic.count).filter(category == BrowserErrorCategory.VUE).sum(); +browsre_app_page_promise_error_sum = from(BrowserAppPageErrorTraffic.count).filter(category == BrowserErrorCategory.PROMISE).sum(); +browser_app_page_js_error_sum = from(BrowserAppPageErrorTraffic.count).filter(category == BrowserErrorCategory.JS).sum(); +browser_app_page_unknown_error_sum = from(BrowserAppPageErrorTraffic.count).filter(category == BrowserErrorCategory.UNKNOWN).sum(); + +browser_app_page_redirect_avg = from(BrowserAppPagePerf.redirectTime).longAvg(); +browser_app_page_dns_avg = from(BrowserAppPagePerf.dnsTime).longAvg(); +browser_app_page_req_avg = from(BrowserAppPagePerf.reqTime).longAvg(); +browser_app_page_dom_analysis_avg = from(BrowserAppPagePerf.domAnalysisTime).longAvg(); +browser_app_page_dom_ready_avg = from(BrowserAppPagePerf.domReadyTime).longAvg(); +browser_app_page_blank_avg = from(BrowserAppPagePerf.blankTime).longAvg(); + +browser_app_page_redirect_percentile = from(BrowserAppPagePerf.redirectTime).percentile(10); +browser_app_page_dns_percentile = from(BrowserAppPagePerf.dnsTime).percentile(10); +browser_app_page_req_percentile = from(BrowserAppPagePerf.reqTime).percentile(10); +browser_app_page_dom_analysis_percentile = from(BrowserAppPagePerf.domAnalysisTime).percentile(10); +browser_app_page_dom_ready_percentile = from(BrowserAppPagePerf.domReadyTime).percentile(10); +browser_app_page_blank_percentile = from(BrowserAppPagePerf.blankTime).percentile(10); + +// Disable unnecessary hard core stream, targeting @Stream#name +/ +// disable(browser_error_traffic) Review comment: This requires the antlr change. `browser_error_traffic` should be a source. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379404093 ## File path: oap-server/server-bootstrap/src/main/resources/browser_analysis.oal ## @@ -0,0 +1,49 @@ +/* +* 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. +* +*/ + +// browser app +browser_app_pv = from(BrowserAppPerf.count).sum(); +browser_app_error_sum = from(BrowserAppErrorTraffic.count).sum(); Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379404199 ## File path: docs/en/setup/README.md ## @@ -25,6 +25,12 @@ You can go to their project repositories for additional info about guides and re - [SkyAPM GO2Sky](https://github.com/SkyAPM/go2sky). See GO2Sky project document for more details. +## Browser agent + +- Activate the [browser-receiver](backend/backend-receivers.md#browser-receiver) following the document. + +- [Browser Js Client](https://github.com/apache/skywalking-client-js). See `skywalking-client-js` project document for more details. Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379404115 ## File path: dist-material/application.yml ## @@ -151,6 +151,17 @@ receiver-jvm: default: receiver-clr: default: +#receiver-browser: Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379404246 ## File path: docs/en/setup/README.md ## @@ -25,6 +25,12 @@ You can go to their project repositories for additional info about guides and re - [SkyAPM GO2Sky](https://github.com/SkyAPM/go2sky). See GO2Sky project document for more details. +## Browser agent + +- Activate the [browser-receiver](backend/backend-receivers.md#browser-receiver) following the document. Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] arugal commented on a change in pull request #4228: Support Browser protocol at OAP
arugal commented on a change in pull request #4228: Support Browser protocol at
OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379403998
##
File path:
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/browser/manual/BrowserErrorTrafficRecord.java
##
@@ -0,0 +1,129 @@
+/*
+ * 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.oap.server.core.browser.manual;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.apm.util.StringUtil;
+import org.apache.skywalking.oap.server.core.Const;
+import org.apache.skywalking.oap.server.core.analysis.Stream;
+import org.apache.skywalking.oap.server.core.analysis.record.Record;
+import
org.apache.skywalking.oap.server.core.analysis.worker.RecordStreamProcessor;
+import org.apache.skywalking.oap.server.core.source.DefaultScopeDefine;
+import org.apache.skywalking.oap.server.core.storage.StorageBuilder;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+import org.apache.skywalking.oap.server.library.util.CollectionUtils;
+
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+@Stream(name = BrowserErrorTrafficRecord.INDEX_NAME, scopeId =
DefaultScopeDefine.BROWSER_ERROR_TRAFFIC, builder =
BrowserErrorTrafficRecord.Builder.class, processor =
RecordStreamProcessor.class)
+public class BrowserErrorTrafficRecord extends Record {
Review comment:
done
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379381981
##
File path:
oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##
@@ -403,4 +421,11 @@ public String formatIndexName(String indexName) {
}
return indexName;
}
+
+public List formatIndexNames(List indexNameList) {
+if (StringUtil.isNotEmpty(namespace)) {
+indexNameList.stream().map(indexName -> namespace + "_" +
indexName);
Review comment:
oops, ack
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379375965
##
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+private static final DateTimeFormatter MM =
DateTimeFormat.forPattern("MM");
+private static final DateTimeFormatter MMDD =
DateTimeFormat.forPattern("MMdd");
+
+public static final List WHITE_INDEX_LIST = new
ArrayList() {
+{
+add(EndpointInventory.INDEX_NAME);
+add(NetworkAddressInventory.INDEX_NAME);
+add(ServiceInventory.INDEX_NAME);
+add(ServiceInstanceInventory.INDEX_NAME);
+}
+};
Review comment:
ack
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379373679
##
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+private static final DateTimeFormatter MM =
DateTimeFormat.forPattern("MM");
+private static final DateTimeFormatter MMDD =
DateTimeFormat.forPattern("MMdd");
+
+public static final List WHITE_INDEX_LIST = new
ArrayList() {
+{
+add(EndpointInventory.INDEX_NAME);
+add(NetworkAddressInventory.INDEX_NAME);
+add(ServiceInventory.INDEX_NAME);
+add(ServiceInstanceInventory.INDEX_NAME);
+}
+};
+
+public static List build(Downsampling downsampling, String
modelName, long startTB, long endTB) {
+if (WHITE_INDEX_LIST.contains(modelName)) {
+return new ArrayList() {
+{
+add(modelName);
+}
+};
Review comment:
ack
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379366744 ## File path: dist-material/application.yml ## @@ -151,6 +151,17 @@ receiver-jvm: default: receiver-clr: default: +#receiver-browser: Review comment: Open as default This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379368921 ## File path: oap-server/server-bootstrap/src/main/resources/browser_analysis.oal ## @@ -0,0 +1,49 @@ +/* +* 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. +* +*/ + +// browser app +browser_app_pv = from(BrowserAppPerf.count).sum(); +browser_app_error_sum = from(BrowserAppErrorTraffic.count).sum(); Review comment: Where is the error rate you asked before? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol
at OAP
URL: https://github.com/apache/skywalking/pull/4228#discussion_r379369588
##
File path:
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/browser/manual/BrowserErrorTrafficRecord.java
##
@@ -0,0 +1,129 @@
+/*
+ * 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.oap.server.core.browser.manual;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.skywalking.apm.util.StringUtil;
+import org.apache.skywalking.oap.server.core.Const;
+import org.apache.skywalking.oap.server.core.analysis.Stream;
+import org.apache.skywalking.oap.server.core.analysis.record.Record;
+import
org.apache.skywalking.oap.server.core.analysis.worker.RecordStreamProcessor;
+import org.apache.skywalking.oap.server.core.source.DefaultScopeDefine;
+import org.apache.skywalking.oap.server.core.storage.StorageBuilder;
+import org.apache.skywalking.oap.server.core.storage.annotation.Column;
+import org.apache.skywalking.oap.server.library.util.CollectionUtils;
+
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+@Stream(name = BrowserErrorTrafficRecord.INDEX_NAME, scopeId =
DefaultScopeDefine.BROWSER_ERROR_TRAFFIC, builder =
BrowserErrorTrafficRecord.Builder.class, processor =
RecordStreamProcessor.class)
+public class BrowserErrorTrafficRecord extends Record {
Review comment:
As a record type, this should be in the disable oal too. Make sense?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379366583 ## File path: README.md ## @@ -27,6 +27,7 @@ The core features are following. - Distributed tracing and context propagation - Database access metrics. Detect slow database access statements(including SQL statements). - Alarm +- Browser performance monitoring Review comment: Add [WIP] as prefix. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] aderm commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379368436
##
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+private static final DateTimeFormatter MM =
DateTimeFormat.forPattern("MM");
+private static final DateTimeFormatter MMDD =
DateTimeFormat.forPattern("MMdd");
+
+public static final List WHITE_INDEX_LIST = new
ArrayList() {
+{
+add(EndpointInventory.INDEX_NAME);
+add(NetworkAddressInventory.INDEX_NAME);
+add(ServiceInventory.INDEX_NAME);
+add(ServiceInstanceInventory.INDEX_NAME);
+}
+};
+
+public static List build(Downsampling downsampling, String
modelName, long startTB, long endTB) {
+if (WHITE_INDEX_LIST.contains(modelName)) {
+return new ArrayList() {
+{
+add(modelName);
+}
+};
+}
+
+List indexNameList = new LinkedList<>();
+DateTime startDT =
DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+DateTime endDT =
DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+if (endDT.isAfter(startDT)) {
+switch (downsampling) {
+case Month:
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MM.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusMonths(1);
+}
+break;
+case Day:
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MMDD.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusDays(1);
+}
+break;
+case Hour:
+//current hour index is also suffix with MMDD
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MMDD.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusDays(1);
+}
+break;
+case Minute:
+//current minute index is also suffix with MMDD
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MMDD.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusDays(1);
+}
+break;
+case Second:
+//current second index is also suffix with MMDD
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379341010 ## File path: docs/en/setup/README.md ## @@ -25,6 +25,12 @@ You can go to their project repositories for additional info about guides and re - [SkyAPM GO2Sky](https://github.com/SkyAPM/go2sky). See GO2Sky project document for more details. +## Browser agent + +- Activate the [browser-receiver](backend/backend-receivers.md#browser-receiver) following the document. + +- [Browser Js Client](https://github.com/apache/skywalking-client-js). See `skywalking-client-js` project document for more details. Review comment: Move this to the 1st point. And change to [Browser JS Client](https://github.com/apache/skywalking-client-js). JS client reports the browser performance data to the backend. See `skywalking-client-js` project document for more details. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP
wu-sheng commented on a change in pull request #4228: Support Browser protocol at OAP URL: https://github.com/apache/skywalking/pull/4228#discussion_r379339284 ## File path: docs/en/setup/README.md ## @@ -25,6 +25,12 @@ You can go to their project repositories for additional info about guides and re - [SkyAPM GO2Sky](https://github.com/SkyAPM/go2sky). See GO2Sky project document for more details. +## Browser agent + +- Activate the [browser-receiver](backend/backend-receivers.md#browser-receiver) following the document. Review comment: I think `receiver-browser` should be activated by default, right? Even the UI is not ready, but it should be there because this will be a part of UI when it is ready. Change this to `Find browser-receiver setup in the backend receiver doc.` as the 2nd point. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[skywalking] branch master updated (5f4b566 -> ae442e3)
This is an automated email from the ASF dual-hosted git repository. kezhenxu94 pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/skywalking.git. from 5f4b566 Fix missing mysql index for profile entities (#4360) add ae442e3 More comments for important classes (#4361) No new revisions were added by this update. Summary of changes: .../apm/agent/core/boot/AgentPackagePath.java | 15 +++-- .../oap/server/core/CoreModuleConfig.java | 20 ++- .../oap/server/core/CoreModuleProvider.java| 8 +-- .../oap/server/core/analysis/SourceDispatcher.java | 9 +++ .../oap/server/core/source/SourceReceiver.java | 4 ++ .../oap/server/library/buffer/DataStream.java | 5 ++ .../server/library/buffer/DataStreamReader.java| 44 +- .../server/library/buffer/DataStreamWriter.java| 19 +++--- .../oap/server/library/buffer/Offset.java | 14 - .../oap/server/library/buffer/OffsetStream.java| 11 +++- .../oap/server/library/util/GRPCStreamStatus.java | 4 ++ .../sharing/server/SharingServerConfig.java| 6 ++ .../sharing/server/SharingServerModule.java| 10 .../provider/parser/ISegmentParserService.java | 3 + .../trace/provider/parser/SegmentParseV2.java | 68 +++--- .../provider/parser/SegmentParserServiceImpl.java | 3 + .../trace/provider/parser/SegmentSource.java | 12 +++- .../provider/parser/decorator/SpanDecorator.java | 4 ++ .../provider/parser/decorator/StandardBuilder.java | 6 ++ .../parser/listener/EntrySpanListener.java | 3 + .../provider/parser/listener/ExitSpanListener.java | 3 + .../parser/listener/FirstSpanListener.java | 3 + .../parser/listener/GlobalTraceIdsListener.java| 4 ++ .../parser/listener/LocalSpanListener.java | 3 + .../provider/parser/listener/SpanListener.java | 13 + .../parser/listener/SpanListenerFactory.java | 4 ++ .../listener/endpoint/MultiScopesSpanListener.java | 26 +++-- .../listener/segment/SegmentSpanListener.java | 16 ++--- .../ServiceInstanceMappingSpanListener.java| 49 +--- .../service/ServiceMappingSpanListener.java| 45 -- .../parser/standardization/IdExchanger.java| 14 + .../standardization/ReferenceIdExchanger.java | 44 +- .../parser/standardization/SpanExchanger.java | 48 --- 33 files changed, 396 insertions(+), 144 deletions(-)
[GitHub] [skywalking] kezhenxu94 merged pull request #4361: More comments for important classes
kezhenxu94 merged pull request #4361: More comments for important classes URL: https://github.com/apache/skywalking/pull/4361 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586151116 Before doing code level deep dive, please read my comments, we should set the target clear about this optimization. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
kezhenxu94 commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379287530
##
File path:
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java
##
@@ -213,4 +213,36 @@ private DateTime parseToDateTime(Downsampling
downsampling, long time) {
}
throw new UnexpectedException("Unexpected downsampling: " +
downsampling.name());
}
+
+public DateTime startTimeBucket2DateTime(Downsampling downsampling, long
startTB) {
+switch (downsampling) {
+case Month:
+return MM.parseDateTime(String.valueOf(startTB));
+case Day:
+return MMDD.parseDateTime(String.valueOf(startTB));
+case Hour:
+return MMDDHH.parseDateTime(String.valueOf(startTB));
+case Minute:
+return MMDDHHMM.parseDateTime(String.valueOf(startTB));
+case Second:
+return MMDDHHMMSS.parseDateTime(String.valueOf(startTB));
+}
+throw new UnexpectedException("Unexpected downsampling: " +
downsampling.name());
+}
+
+public DateTime endTimeBucket2DateTime(Downsampling downsampling, long
endTB) {
+switch (downsampling) {
+case Month:
+return MM.parseDateTime(String.valueOf(endTB));
+case Day:
+return MMDD.parseDateTime(String.valueOf(endTB));
+case Hour:
+return MMDDHH.parseDateTime(String.valueOf(endTB));
+case Minute:
+return MMDDHHMM.parseDateTime(String.valueOf(endTB));
+case Second:
+return MMDDHHMMSS.parseDateTime(String.valueOf(endTB));
+}
+throw new UnexpectedException("Unexpected downsampling: " +
downsampling.name());
Review comment:
The logic seems not to be related to `startTime` or `endTime`, so I don't
think it's a good method name `endTimeBucket2DateTime ` or
`startTimeBucket2DateTime `, and the method body are exactly the same in
`parseToDateTime`, if you meant it, simply use `parseToDateTime` is enough?
Otherwise, you may forget to `.plusMonths(1)`, `plusDays(1)`, etc. like
`org.apache.skywalking.oap.server.core.query.DurationUtils#endTimeToTimestamp`
```java
public long endTimeToTimestamp(Step step, String dateStr) {
switch (step) {
case MONTH:
return
_MM.parseDateTime(dateStr).plusMonths(1).getMillis();
case DAY:
return
_MM_DD.parseDateTime(dateStr).plusDays(1).getMillis();
case HOUR:
return
_MM_DD_HH.parseDateTime(dateStr).plusHours(1).getMillis();
case MINUTE:
return
_MM_DD_HHMM.parseDateTime(dateStr).plusMinutes(1).getMillis();
case SECOND:
return
_MM_DD_HHMMSS.parseDateTime(dateStr).plusSeconds(1).getMillis();
}
throw new UnexpectedException("Unsupported step " + step.name());
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#discussion_r379290950 ## File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java ## @@ -291,6 +291,24 @@ public SearchResponse search(String indexName, SearchSourceBuilder searchSourceB return client.search(searchRequest); } +/** + * Search results from ES search engine according to various search conditions, + * Note the method is usered for the list of index names is optimized based on + * the scope of startTimeBucket and endTimeBucket + * @param indexNameList full index names list base on timebucket scope. + * Except for endpoint_inventory network_address_inventory service_inventory service_instance_inventory + * @param searchSourceBuilder Various search query conditions + * @return ES search query results + * @throws IOException throw IOException Review comment: not a good java doc: `throw IOException`, better to describe **WHEN** the exception is thrown This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
kezhenxu94 commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379285619
##
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+private static final DateTimeFormatter MM =
DateTimeFormat.forPattern("MM");
+private static final DateTimeFormatter MMDD =
DateTimeFormat.forPattern("MMdd");
+
+public static final List WHITE_INDEX_LIST = new
ArrayList() {
+{
+add(EndpointInventory.INDEX_NAME);
+add(NetworkAddressInventory.INDEX_NAME);
+add(ServiceInventory.INDEX_NAME);
+add(ServiceInstanceInventory.INDEX_NAME);
+}
+};
+
+public static List build(Downsampling downsampling, String
modelName, long startTB, long endTB) {
+if (WHITE_INDEX_LIST.contains(modelName)) {
+return new ArrayList() {
+{
+add(modelName);
+}
+};
+}
+
+List indexNameList = new LinkedList<>();
+DateTime startDT =
DurationUtils.INSTANCE.startTimeBucket2DateTime(downsampling, startTB);
+DateTime endDT =
DurationUtils.INSTANCE.endTimeBucket2DateTime(downsampling, endTB);
+if (endDT.isAfter(startDT)) {
+switch (downsampling) {
+case Month:
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MM.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusMonths(1);
+}
+break;
+case Day:
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MMDD.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusDays(1);
+}
+break;
+case Hour:
+//current hour index is also suffix with MMDD
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MMDD.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusDays(1);
+}
+break;
+case Minute:
+//current minute index is also suffix with MMDD
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName) +
"-" + MMDD.print(startDT);
+indexNameList.add(indexName);
+startDT = startDT.plusDays(1);
+}
+break;
+case Second:
+//current second index is also suffix with MMDD
+while (endDT.isAfter(startDT)) {
+String indexName = build(downsampling, modelName)
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
kezhenxu94 commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379294683
##
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+private static final DateTimeFormatter MM =
DateTimeFormat.forPattern("MM");
+private static final DateTimeFormatter MMDD =
DateTimeFormat.forPattern("MMdd");
+
+public static final List WHITE_INDEX_LIST = new
ArrayList() {
+{
+add(EndpointInventory.INDEX_NAME);
+add(NetworkAddressInventory.INDEX_NAME);
+add(ServiceInventory.INDEX_NAME);
+add(ServiceInstanceInventory.INDEX_NAME);
+}
+};
+
+public static List build(Downsampling downsampling, String
modelName, long startTB, long endTB) {
+if (WHITE_INDEX_LIST.contains(modelName)) {
+return new ArrayList() {
+{
+add(modelName);
+}
+};
Review comment:
Same here, please be careful when using this kind of initialization, it
creates unnecessary inner class, if you always use this, there may be many many
unnecessary inner classes, exploding the meta space of the JVM, and the problem
is that we have much better method to do this, like `Arrays.asList`, Guava
list, etc.:
```java
new ArrayList() {
{
add(modelName);
}
};
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
kezhenxu94 commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379286512
##
File path:
oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/base/EsModelName.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.oap.server.storage.plugin.elasticsearch.base;
+
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+import org.apache.skywalking.oap.server.core.analysis.Downsampling;
+import org.apache.skywalking.oap.server.core.query.DurationUtils;
+import org.apache.skywalking.oap.server.core.register.EndpointInventory;
+import org.apache.skywalking.oap.server.core.register.NetworkAddressInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInstanceInventory;
+import org.apache.skywalking.oap.server.core.register.ServiceInventory;
+import org.apache.skywalking.oap.server.core.storage.model.ModelName;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+
+public class EsModelName extends ModelName {
+
+private static final DateTimeFormatter MM =
DateTimeFormat.forPattern("MM");
+private static final DateTimeFormatter MMDD =
DateTimeFormat.forPattern("MMdd");
+
+public static final List WHITE_INDEX_LIST = new
ArrayList() {
+{
+add(EndpointInventory.INDEX_NAME);
+add(NetworkAddressInventory.INDEX_NAME);
+add(ServiceInventory.INDEX_NAME);
+add(ServiceInstanceInventory.INDEX_NAME);
+}
+};
Review comment:
This is not a good practice to initialize a list, it creates an unnecessary
inner class. Based on your usage, please consider make it immutable by
`com.google.common.collect.Sets#newHashSet(E...)`, because you're always using
the `contains` method to determine the existence, `Set` should be faster IMHO
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
wu-sheng commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586150036 `allow_no_indices` and `ignore_unavailable` should be good choices. Could you try to use these? And check whether there is error log in OAP and ES server? Here is the report from the original issue author, in Chinese, [skywalking查询优化基准测试.pdf](https://github.com/apache/skywalking/files/4203428/skywalking.pdf) This only shows the necessity to support this in the segment query, also, it requires a change in the UI. @kezhenxu94 @zhaoyuguang @JaredTan95 @wayilau @aderm @arugal Did any of you face this issue in the production environment? Should we continue on this PR? Or tag this as TBD later? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4353: Optimizing performance reduces es index queries scope by timebucket
kezhenxu94 commented on a change in pull request #4353: Optimizing performance
reduces es index queries scope by timebucket
URL: https://github.com/apache/skywalking/pull/4353#discussion_r379290165
##
File path:
oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
##
@@ -403,4 +421,11 @@ public String formatIndexName(String indexName) {
}
return indexName;
}
+
+public List formatIndexNames(List indexNameList) {
+if (StringUtil.isNotEmpty(namespace)) {
+indexNameList.stream().map(indexName -> namespace + "_" +
indexName);
Review comment:
Bug here, you map the `indexNameList` but ignore the result, the
`indexNameList` itself is unchanged
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859 @wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set. Refer this: [https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url) - gap time near 00:00 - multi OAP node - product error log The above mentioned scenarios are all fine. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859 @wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set. Refer this:[[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)](url) - gap time near 00:00 - multi OAP node - product error log The above mentioned scenarios are all fine. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket
aderm edited a comment on issue #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859 @wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set. Refer this:[[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)](url) - gap time near 00:00 - multi OAP node - product error log The above mentioned scenarios are all fine. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket
aderm commented on issue #4353: Optimizing performance reduces es index queries scope by timebucket URL: https://github.com/apache/skywalking/pull/4353#issuecomment-586144859 @wu-sheng Worried about the problem of error in log( `index_not_found` exception ), I checked the relevant documents. Found that ES has configuration parameters that can be set. Refer this:[[https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html](url)](url) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [skywalking] wu-sheng closed issue #4359: Add MySQL index for profile entities
wu-sheng closed issue #4359: Add MySQL index for profile entities URL: https://github.com/apache/skywalking/issues/4359 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[skywalking] branch master updated (6be3e99 -> 5f4b566)
This is an automated email from the ASF dual-hosted git repository. wusheng pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/skywalking.git. from 6be3e99 Help people to read source codes of core module. (#4357) add 5f4b566 Fix missing mysql index for profile entities (#4360) No new revisions were added by this update. Summary of changes: .../plugin/jdbc/mysql/MySQLTableInstaller.java | 76 +- 1 file changed, 74 insertions(+), 2 deletions(-)
[skywalking] branch comments updated (e3c61af -> 9989afe)
This is an automated email from the ASF dual-hosted git repository. wusheng pushed a change to branch comments in repository https://gitbox.apache.org/repos/asf/skywalking.git. from e3c61af Fix format. add 5f4b566 Fix missing mysql index for profile entities (#4360) add 9989afe Merge branch 'master' into comments No new revisions were added by this update. Summary of changes: .../plugin/jdbc/mysql/MySQLTableInstaller.java | 76 +- 1 file changed, 74 insertions(+), 2 deletions(-)
[GitHub] [skywalking] wu-sheng merged pull request #4360: Fix missing mysql index for profile entities
wu-sheng merged pull request #4360: Fix missing mysql index for profile entities URL: https://github.com/apache/skywalking/pull/4360 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
