This is an automated email from the ASF dual-hosted git repository. yihaochen pushed a commit to branch fix-aipipeline in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit cc711c6ed82b58c062dbee86580931797b7e3348 Author: Superskyyy <[email protected]> AuthorDate: Tue Jun 27 16:26:19 2023 -0400 Fix and finalize ai-endpoint-grouping related functionalities. --- docs/en/changes/changes.md | 1 + .../services/HttpUriRecognitionService.java | 32 ++++-- .../group/uri/RegexVSQuickMatchBenchmark.java | 119 +++++++++++---------- .../core/config/group/EndpointNameGrouping.java | 15 +-- .../openapi/EndpointGroupingRule4Openapi.java | 2 +- .../config/group/uri/quickmatch/PatternTree.java | 75 ++++++++++--- .../config/group/uri/quickmatch/StringToken.java | 7 +- .../core/config/group/uri/quickmatch/VarToken.java | 7 +- .../group/EndpointGroupingRuleReaderTest.java | 6 ++ .../group/uri/quickmatch/PatternTreeTest.java | 64 +++++++++-- .../oap/server/library/util/StringFormatGroup.java | 2 +- 11 files changed, 225 insertions(+), 105 deletions(-) diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md index 06824dc7b2..0bc4a5fad2 100644 --- a/docs/en/changes/changes.md +++ b/docs/en/changes/changes.md @@ -26,6 +26,7 @@ * Add component ID for Aerospike(ID=149). * Packages with name `recevier` are renamed to `receiver`. * `BanyanDBMetricsDAO` handles `storeIDTag` in `multiGet` for `BanyanDBModelExtension`. +* Fix endpoint grouping-related logic and enhance the performance of PatternTree retrieval. #### UI diff --git a/oap-server/ai-pipeline/src/main/java/org/apache/skywalking/oap/server/ai/pipeline/services/HttpUriRecognitionService.java b/oap-server/ai-pipeline/src/main/java/org/apache/skywalking/oap/server/ai/pipeline/services/HttpUriRecognitionService.java index 51a59821d7..e4e37289e2 100644 --- a/oap-server/ai-pipeline/src/main/java/org/apache/skywalking/oap/server/ai/pipeline/services/HttpUriRecognitionService.java +++ b/oap-server/ai-pipeline/src/main/java/org/apache/skywalking/oap/server/ai/pipeline/services/HttpUriRecognitionService.java @@ -68,14 +68,19 @@ public class HttpUriRecognitionService implements HttpUriRecognition { .build() ); final String newVersion = httpUriRecognitionResponse.getVersion(); + if (log.isDebugEnabled()) { + log.debug("Remote response version: {}, local version {}", newVersion, version); + } if (version.equals(newVersion)) { - // Same version, nothing changed. return null; + } else { + version = newVersion; } + return httpUriRecognitionResponse.getPatternsList() - .stream() - .map(pattern -> new HttpUriPattern(pattern.getPattern())) - .collect(Collectors.toList()); + .stream() + .map(pattern -> new HttpUriPattern(pattern.getPattern())) + .collect(Collectors.toList()); } catch (Exception e) { log.error("fetch all patterns failed from remote server.", e); @@ -91,15 +96,20 @@ public class HttpUriRecognitionService implements HttpUriRecognition { } final HttpUriRecognitionRequest.Builder builder = HttpUriRecognitionRequest.newBuilder(); builder.setService(service); - unrecognizedURIs.forEach(httpUri -> { - builder.getUnrecognizedUrisBuilderList().add( - HttpRawUri.newBuilder().setName(httpUri.getName()) - ); - }); + if (log.isDebugEnabled()) { + log.debug("Feeding to remote, service: {}, uri size: {}", service, unrecognizedURIs.size()); + } + List<HttpRawUri> rawUriList = unrecognizedURIs.stream() + .map(httpUri -> HttpRawUri.newBuilder() + .setName(httpUri.getName()) + .build()) + .collect(Collectors.toList()); + + builder.addAllUnrecognizedUris(rawUriList); stub.withDeadlineAfter(30, TimeUnit.SECONDS) - .feedRawData(builder.build()); + .feedRawData(builder.build()); } catch (Exception e) { - log.error("feed matched and unmatched URIs to the remote server.", e); + log.error("Failed to feed matched and unmatched URIs to the remote server.", e); } } } diff --git a/oap-server/microbench/src/main/java/org/apache/skywalking/oap/server/microbench/core/config/group/uri/RegexVSQuickMatchBenchmark.java b/oap-server/microbench/src/main/java/org/apache/skywalking/oap/server/microbench/core/config/group/uri/RegexVSQuickMatchBenchmark.java index bc3761a175..60e676783e 100644 --- a/oap-server/microbench/src/main/java/org/apache/skywalking/oap/server/microbench/core/config/group/uri/RegexVSQuickMatchBenchmark.java +++ b/oap-server/microbench/src/main/java/org/apache/skywalking/oap/server/microbench/core/config/group/uri/RegexVSQuickMatchBenchmark.java @@ -126,69 +126,70 @@ public class RegexVSQuickMatchBenchmark extends AbstractMicrobenchmark { /** * # JMH version: 1.25 - * # VM version: JDK 11.0.18, OpenJDK 64-Bit Server VM, 11.0.18+10 - * # VM invoker: /Users/wusheng/Library/Java/JavaVirtualMachines/temurin-11.0.18/Contents/Home/bin/java - * # VM options: -ea --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED -Didea.test.cyclic.buffer.size=1048576 -javaagent:/Applications/IntelliJ IDEA.app/Contents/lib/idea_rt.jar=53714:/Applications/IntelliJ IDEA.app/Contents/bin -Dfile.encoding=UTF-8 -Xmx512m -Xms512m -XX:MaxDirectMemorySize=512m -XX:BiasedLockingStartupDelay=0 -Djmh.executor=CUSTOM -Djmh.executor.class=org.apache.skywalking.oap.server.microbench.base.AbstractMicrobenchmark$JmhThread [...] + * # VM version: JDK 16.0.1, OpenJDK 64-Bit Server VM, 16.0.1+9-24 + * # VM invoker: C:\Users\Sky\.jdks\openjdk-16.0.1\bin\java.exe + * # VM options: -ea --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED -Didea.test.cyclic.buffer.size=1048576 -javaagent:Y:\jetbrains\apps\IDEA-U\ch-0\231.8109.175\lib\idea_rt.jar=54938:Y:\jetbrains\apps\IDEA-U\ch-0\231.8109.175\bin -Dfile.encoding=UTF-8 -Xmx512m -Xms512m -XX:MaxDirectMemorySize=512m -XX:BiasedLockingStartupDelay=0 -Djmh.executor=CUSTOM -Djmh.executor.class=org.apache.skywalking.oap.server.microbench.base.AbstractMicrobenchmark$JmhTh [...] * # Warmup: 1 iterations, 10 s each * # Measurement: 1 iterations, 10 s each * # Timeout: 10 min per iteration * # Threads: 4 threads, will synchronize iterations * # Benchmark mode: Throughput, ops/time - * # Benchmark: org.apache.skywalking.oap.server.microbench.core.config.group.uri.RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping + * # Benchmark: org.apache.skywalking.oap.server.microbench.core.config.group.uri.RegexVSQuickMatchBenchmark.notMatchRegex + * Benchmark Mode Cnt Score Error Units + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping thrpt 48317763.786 ops/s + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.alloc.rate thrpt 8773.225 MB/sec + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.alloc.rate.norm thrpt 200.014 B/op + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Eden_Space thrpt 8807.405 MB/sec + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Eden_Space.norm thrpt 200.794 B/op + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Survivor_Space thrpt 0.050 MB/sec + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Survivor_Space.norm thrpt 0.001 B/op + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.count thrpt 303.000 counts + * RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.time thrpt 325.000 ms + * RegexVSQuickMatchBenchmark.matchFirstRegex thrpt 41040542.288 ops/s + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.alloc.rate thrpt 8348.690 MB/sec + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.alloc.rate.norm thrpt 224.016 B/op + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Eden_Space thrpt 8378.454 MB/sec + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Eden_Space.norm thrpt 224.815 B/op + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Survivor_Space thrpt 0.057 MB/sec + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Survivor_Space.norm thrpt 0.002 B/op + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.count thrpt 288.000 counts + * RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.time thrpt 282.000 ms + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping thrpt 35658131.267 ops/s + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.alloc.rate thrpt 8020.546 MB/sec + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.alloc.rate.norm thrpt 248.018 B/op + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Eden_Space thrpt 8043.279 MB/sec + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Eden_Space.norm thrpt 248.721 B/op + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Survivor_Space thrpt 0.045 MB/sec + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Survivor_Space.norm thrpt 0.001 B/op + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.count thrpt 277.000 counts + * RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.time thrpt 302.000 ms + * RegexVSQuickMatchBenchmark.matchFourthRegex thrpt 11066068.208 ops/s + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.alloc.rate thrpt 8273.312 MB/sec + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.alloc.rate.norm thrpt 824.060 B/op + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Eden_Space thrpt 8279.984 MB/sec + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Eden_Space.norm thrpt 824.724 B/op + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Survivor_Space thrpt 0.052 MB/sec + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Survivor_Space.norm thrpt 0.005 B/op + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.count thrpt 285.000 counts + * RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.time thrpt 324.000 ms + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping thrpt 45843193.472 ops/s + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.alloc.rate thrpt 8653.215 MB/sec + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.alloc.rate.norm thrpt 208.015 B/op + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Eden_Space thrpt 8652.365 MB/sec + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Eden_Space.norm thrpt 207.995 B/op + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Survivor_Space thrpt 0.048 MB/sec + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Survivor_Space.norm thrpt 0.001 B/op + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.count thrpt 298.000 counts + * RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.time thrpt 358.000 ms + * RegexVSQuickMatchBenchmark.notMatchRegex thrpt 3434953.426 ops/s + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.alloc.rate thrpt 8898.075 MB/sec + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.alloc.rate.norm thrpt 2856.206 B/op + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Eden_Space thrpt 8886.568 MB/sec + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Eden_Space.norm thrpt 2852.512 B/op + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Survivor_Space thrpt 0.052 MB/sec + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Survivor_Space.norm thrpt 0.017 B/op + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.count thrpt 306.000 counts + * RegexVSQuickMatchBenchmark.notMatchRegex:·gc.time thrpt 377.000 ms * - Benchmark Mode Cnt Score Error Units - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping thrpt 28464926.797 ops/s - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.alloc.rate thrpt 6194.492 MB/sec - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.alloc.rate.norm thrpt 240.000 B/op - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Eden_Space thrpt 6222.267 MB/sec - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Eden_Space.norm thrpt 241.076 B/op - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Old_Gen thrpt 0.023 MB/sec - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.churn.G1_Old_Gen.norm thrpt 0.001 B/op - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.count thrpt 214.000 counts - RegexVSQuickMatchBenchmark.matchFirstQuickUriGrouping:·gc.time thrpt 194.000 ms - RegexVSQuickMatchBenchmark.matchFirstRegex thrpt 51679120.204 ops/s - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.alloc.rate thrpt 7130.116 MB/sec - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.alloc.rate.norm thrpt 152.000 B/op - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Eden_Space thrpt 7162.842 MB/sec - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Eden_Space.norm thrpt 152.698 B/op - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Old_Gen thrpt 0.020 MB/sec - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.churn.G1_Old_Gen.norm thrpt ≈ 10⁻³ B/op - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.count thrpt 246.000 counts - RegexVSQuickMatchBenchmark.matchFirstRegex:·gc.time thrpt 224.000 ms - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping thrpt 23359343.934 ops/s - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.alloc.rate thrpt 6106.164 MB/sec - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.alloc.rate.norm thrpt 288.000 B/op - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Eden_Space thrpt 6143.526 MB/sec - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Eden_Space.norm thrpt 289.762 B/op - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Old_Gen thrpt 0.023 MB/sec - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.churn.G1_Old_Gen.norm thrpt 0.001 B/op - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.count thrpt 211.000 counts - RegexVSQuickMatchBenchmark.matchFourthQuickUriGrouping:·gc.time thrpt 143.000 ms - RegexVSQuickMatchBenchmark.matchFourthRegex thrpt 24074353.094 ops/s - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.alloc.rate thrpt 17999.991 MB/sec - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.alloc.rate.norm thrpt 824.000 B/op - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Eden_Space thrpt 18070.905 MB/sec - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Eden_Space.norm thrpt 827.246 B/op - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Old_Gen thrpt 0.095 MB/sec - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.churn.G1_Old_Gen.norm thrpt 0.004 B/op - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.count thrpt 621.000 counts - RegexVSQuickMatchBenchmark.matchFourthRegex:·gc.time thrpt 934.000 ms - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping thrpt 27031477.704 ops/s - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.alloc.rate thrpt 6081.482 MB/sec - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.alloc.rate.norm thrpt 248.000 B/op - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Eden_Space thrpt 6109.321 MB/sec - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Eden_Space.norm thrpt 249.135 B/op - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Old_Gen thrpt 0.022 MB/sec - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.churn.G1_Old_Gen.norm thrpt 0.001 B/op - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.count thrpt 210.000 counts - RegexVSQuickMatchBenchmark.notMatchQuickUriGrouping:·gc.time thrpt 171.000 ms - RegexVSQuickMatchBenchmark.notMatchRegex thrpt 9368757.119 ops/s - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.alloc.rate thrpt 23999.619 MB/sec - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.alloc.rate.norm thrpt 2824.000 B/op - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Eden_Space thrpt 24087.019 MB/sec - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Eden_Space.norm thrpt 2834.284 B/op - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Old_Gen thrpt 0.114 MB/sec - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.churn.G1_Old_Gen.norm thrpt 0.013 B/op - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.count thrpt 828.000 counts - RegexVSQuickMatchBenchmark.notMatchRegex:·gc.time thrpt 896.000 ms + * Process finished with exit code 0 */ \ No newline at end of file diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/EndpointNameGrouping.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/EndpointNameGrouping.java index c122db7fae..9872e73b51 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/EndpointNameGrouping.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/EndpointNameGrouping.java @@ -97,7 +97,7 @@ public class EndpointNameGrouping { if (svrHttpUris.size() < maxHttpUrisNumberPerService) { if (formattedName._2()) { // Algorithm side should not return a pattern that has no {var} in it else this - // code may accidentally retreive the size 1 queue created by unformatted endpoint + // code may accidentally retrieve the size 1 queue created by unformatted endpoint // The queue size is 10, which means only cache the first 10 formatted names. final ArrayBlockingQueue<String> formattedURIs = svrHttpUris.computeIfAbsent( formattedName._1(), k -> new ArrayBlockingQueue<>(10)); @@ -140,11 +140,13 @@ public class EndpointNameGrouping { log.trace("Endpoint {} of Service {} keeps unchanged.", endpointName, serviceName); } } - return new Tuple2<>(formatResult.getName(), formatResult.isMatch()); + return new Tuple2<>(formatResult.getReplacedName(), formatResult.isMatch()); } private Tuple2<String, Boolean> formatByQuickUriPattern(String serviceName, String endpointName) { final StringFormatGroup.FormatResult formatResult = quickUriGroupingRule.format(serviceName, endpointName); + + log.info("formatByQuickUriPattern: {} {}, formatResult {}", serviceName, endpointName, formatResult); if (log.isDebugEnabled() || log.isTraceEnabled()) { if (formatResult.isMatch()) { log.debug( @@ -155,7 +157,7 @@ public class EndpointNameGrouping { log.trace("Endpoint {} of Service {} keeps unchanged.", endpointName, serviceName); } } - return new Tuple2<>(formatResult.getName(), formatResult.isMatch()); + return new Tuple2<>(formatResult.getReplacedName(), formatResult.isMatch()); } public void startHttpUriRecognitionSvr(final HttpUriRecognition httpUriRecognitionSvr, @@ -172,7 +174,8 @@ public class EndpointNameGrouping { .scheduleWithFixedDelay( new RunnableWithExceptionProtection( () -> { - if (aiPipelineExecutionCounter.incrementAndGet() % trainingPeriodHttpUriRecognitionPattern == 0) { + int currentExecutionCounter = aiPipelineExecutionCounter.incrementAndGet(); + if (currentExecutionCounter % trainingPeriodHttpUriRecognitionPattern == 0) { // Send the cached URIs to the recognition server to build new patterns. cachedHttpUris.forEach((serviceName, httpUris) -> { final List<HttpUriRecognition.HTTPUri> candidates4UriPatterns = new ArrayList<>( @@ -182,7 +185,7 @@ public class EndpointNameGrouping { //unrecognized uri candidates4UriPatterns.add(new HttpUriRecognition.HTTPUri(uri)); } else { - String candidateUri = null; + String candidateUri; while ((candidateUri = candidates.poll()) != null) { candidates4UriPatterns.add( new HttpUriRecognition.HTTPUri(candidateUri)); @@ -195,7 +198,7 @@ public class EndpointNameGrouping { httpUriRecognitionSvr.feedRawData(serviceName, candidates4UriPatterns); }); } - if (aiPipelineExecutionCounter.incrementAndGet() % syncPeriodHttpUriRecognitionPattern == 0) { + if (currentExecutionCounter % syncPeriodHttpUriRecognitionPattern == 0) { // Sync with the recognition server per 1 min to get the latest patterns. try { metadataQueryService.listServices(null, null).forEach( diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/openapi/EndpointGroupingRule4Openapi.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/openapi/EndpointGroupingRule4Openapi.java index f81cf8ddf9..4fa1b09cba 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/openapi/EndpointGroupingRule4Openapi.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/openapi/EndpointGroupingRule4Openapi.java @@ -44,7 +44,7 @@ public class EndpointGroupingRule4Openapi { public StringFormatGroup.FormatResult format(String service, String endpointName) { Map<String, String> endpointNameLookup = directLookup.get(service); if (endpointNameLookup != null && endpointNameLookup.get(endpointName) != null) { - return new StringFormatGroup.FormatResult(true, endpointNameLookup.get(endpointName), endpointName); + return new StringFormatGroup.FormatResult(true, endpointName, endpointNameLookup.get(endpointName)); } Map<String, StringFormatGroup> rules = groupedRules.get(service); diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTree.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTree.java index 129e703166..1c3527f6d3 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTree.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTree.java @@ -40,28 +40,28 @@ public class PatternTree { * @param pattern of URIs */ public void addPattern(String pattern) { - final String[] tokens = pattern.split("/"); + final List<String> tokens = splitByCharacter(pattern); PatternToken current = null; for (final PatternToken patternToken : roots) { - if (patternToken.isMatch(tokens[0])) { + if (patternToken.isMatch(tokens.get(0))) { current = patternToken; break; } } if (current == null) { - current = new StringToken(tokens[0]); + current = new StringToken(tokens.get(0)); roots.add(current); } - if (tokens.length == 1) { + if (tokens.size() == 1) { current.setExpression(pattern); return; } - for (int i = 1; i < tokens.length; i++) { - final String token = tokens[i]; + for (int i = 1; i < tokens.size(); i++) { + final String token = tokens.get(i); PatternToken newToken; if (VarToken.VAR_TOKEN.equals(token)) { newToken = new VarToken(); @@ -78,19 +78,24 @@ public class PatternTree { current.setExpression(pattern); } - List<String> splitByCharacter(String input, char delimiter) { + List<String> splitByCharacter(String input) { List<String> parts = new ArrayList<>(); + int length = input.length(); int start = 0; - for (int i = 0; i < input.length(); i++) { - if (input.charAt(i) == delimiter) { + for (int i = 0; i < length; i++) { + if (input.charAt(i) == '/') { + if (i == 0) { + start = i + 1; + continue; + } parts.add(input.substring(start, i)); start = i + 1; } } // Add the last part if necessary - if (start < input.length()) { + if (start < length) { parts.add(input.substring(start)); } @@ -98,7 +103,13 @@ public class PatternTree { } public StringFormatGroup.FormatResult match(String uri) { - final List<String> slices = splitByCharacter(uri, '/'); + final List<String> slices = splitByCharacter(uri); + if (slices.size() == 1) { + // Special case handling, since if a URI is just length one + // itself will never be a variable, so simply return true and itself + // trailing slashes, if ever encountered will be kept as is + return new StringFormatGroup.FormatResult(true, uri, uri); + } List<PatternToken> current = roots; PatternToken matchedToken = null; for (final String slice : slices) { @@ -111,14 +122,52 @@ public class PatternTree { } } if (!matched) { - return new StringFormatGroup.FormatResult(false, uri, null); + return new StringFormatGroup.FormatResult(false, uri, uri); } current = matchedToken.children(); } if (matchedToken.isLeaf()) { return new StringFormatGroup.FormatResult(true, uri, matchedToken.expression()); } else { - return new StringFormatGroup.FormatResult(false, uri, null); + return new StringFormatGroup.FormatResult(false, uri, uri); + } + } + + @SuppressWarnings("unused") + // Utility method to visualize the full tree for debugging purposes + public String printTree() { + StringBuilder sb = new StringBuilder(); + for (PatternToken root : roots) { + sb.append(printNode(root, 0)); } + return sb.toString(); } + + private String printNode(PatternToken node, int depth) { + StringBuilder sb = new StringBuilder(); + + sb.append(" ".repeat(Math.max(0, depth))); + + sb.append(node.toString()).append("\n"); + + // Append expression if not null + if (node.expression() != null) { + sb.append(" ").append(node.expression()).append("\n"); + } + + if (node instanceof StringToken) { + StringToken stringToken = (StringToken) node; + for (PatternToken child : stringToken.children()) { + sb.append(printNode(child, depth + 1)); + } + } else if (node instanceof VarToken) { + VarToken varToken = (VarToken) node; + for (PatternToken child : varToken.children()) { + sb.append(printNode(child, depth + 1)); + } + } + + return sb.toString(); + } + } diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/StringToken.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/StringToken.java index 350502ae68..bce401b0ec 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/StringToken.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/StringToken.java @@ -22,10 +22,8 @@ import java.util.ArrayList; import java.util.List; import lombok.EqualsAndHashCode; import lombok.Setter; -import lombok.ToString; @EqualsAndHashCode(of = "value") -@ToString public class StringToken implements PatternToken { private final String value; private final List<PatternToken> children; @@ -56,4 +54,9 @@ public class StringToken implements PatternToken { public List<PatternToken> children() { return children; } + + @Override + public String toString() { + return "StringToken: \"" + value + "\""; + } } diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/VarToken.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/VarToken.java index c668fd5ae9..4991af930f 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/VarToken.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/VarToken.java @@ -22,10 +22,8 @@ import java.util.ArrayList; import java.util.List; import lombok.EqualsAndHashCode; import lombok.Setter; -import lombok.ToString; @EqualsAndHashCode(of = "") -@ToString public class VarToken implements PatternToken { public static final String VAR_TOKEN = "{var}"; private List<PatternToken> children = new ArrayList<>(); @@ -54,4 +52,9 @@ public class VarToken implements PatternToken { public List<PatternToken> children() { return children; } + + @Override + public String toString() { + return "VarToken: \"" + VAR_TOKEN + "\""; + } } diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/EndpointGroupingRuleReaderTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/EndpointGroupingRuleReaderTest.java index d07abfe702..3d89d6180c 100644 --- a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/EndpointGroupingRuleReaderTest.java +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/EndpointGroupingRuleReaderTest.java @@ -36,8 +36,14 @@ public class EndpointGroupingRuleReaderTest { Assertions.assertTrue(formatResult.isMatch()); Assertions.assertEquals("/prod/{var}", formatResult.getReplacedName()); + // This will always match, since after slicing length is 1, which goes into special handling formatResult = rule.format("serviceA", "/prod/"); + Assertions.assertTrue(formatResult.isMatch()); + Assertions.assertEquals("/prod/", formatResult.getReplacedName()); + + formatResult = rule.format("serviceA", "/prod/123/456"); Assertions.assertFalse(formatResult.isMatch()); + Assertions.assertEquals("/prod/123/456", formatResult.getReplacedName()); formatResult = rule.format("serviceB", "/prod/123"); Assertions.assertFalse(formatResult.isMatch()); diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTreeTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTreeTest.java index 469741b040..c07f30c229 100644 --- a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTreeTest.java +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/config/group/uri/quickmatch/PatternTreeTest.java @@ -30,31 +30,49 @@ class PatternTreeTest { PatternTree tree = new PatternTree(); tree.addPattern("/products/{var}"); tree.addPattern("/products/{var}/detail"); + tree.addPattern("/products/{var}/refund"); + tree.addPattern("/products/{var}/reorder/extra"); tree.addPattern("/sales/{var}"); tree.addPattern("/employees/{var}/profile"); + // This should map to exact same tree nodes + tree.addPattern("produces/{var}/profile"); + tree.addPattern("GET:/posts/{var}"); + tree.addPattern("https://abc.com/posts/{var}";); final Field rootField = PatternTree.class.getDeclaredField("roots"); rootField.setAccessible(true); final List<PatternToken> roots = (List<PatternToken>) rootField.get(tree); - final PatternToken root = roots.get(0); - Assertions.assertEquals(new StringToken(""), root); - Assertions.assertEquals(3, root.children().size()); - final PatternToken prodToken = root.children().get(0); + final PatternToken prodToken = roots.get(0); Assertions.assertEquals(new StringToken("products"), prodToken); Assertions.assertEquals(1, prodToken.children().size()); - final PatternToken prodVarToken = prodToken.children().get(0); - Assertions.assertEquals(new VarToken(), prodVarToken); - final PatternToken detailToken = prodVarToken.children().get(0); + final PatternToken varToken = prodToken.children().get(0); + Assertions.assertEquals(new VarToken(), varToken); + Assertions.assertEquals(3, varToken.children().size()); + final PatternToken detailToken = varToken.children().get(0); Assertions.assertEquals(new StringToken("detail"), detailToken); - final PatternToken salesToken = root.children().get(1); + final PatternToken salesToken = roots.get(1); Assertions.assertEquals(new StringToken("sales"), salesToken); Assertions.assertEquals(1, salesToken.children().size()); - final PatternToken employeeToken = root.children().get(2); + final PatternToken employeeToken = roots.get(2); Assertions.assertEquals(new StringToken("employees"), employeeToken); Assertions.assertEquals(1, employeeToken.children().size()); + + final PatternToken producesToken = roots.get(3); + Assertions.assertEquals(new StringToken("produces"), producesToken); + Assertions.assertEquals(1, producesToken.children().size()); + + final PatternToken getPostsToken = roots.get(4); + Assertions.assertEquals(new StringToken("GET:"), getPostsToken); + + final PatternToken abcToken = roots.get(5); + Assertions.assertEquals(new StringToken("https:"), abcToken); + final PatternToken abcComToken = abcToken.children().get(0); + // For general performance purposes, double / will result in an empty string token + // This is considered an intentional feature rather than a bug + Assertions.assertEquals(new StringToken(""), abcComToken); } @Test @@ -62,8 +80,13 @@ class PatternTreeTest { PatternTree tree = new PatternTree(); tree.addPattern("/products/{var}"); tree.addPattern("/products/{var}/detail"); + tree.addPattern("/products/{var}/refund"); + tree.addPattern("/products/{var}/reorder/extra"); tree.addPattern("/sales/{var}"); tree.addPattern("/employees/{var}/profile"); + tree.addPattern("produces/{var}/profile"); + tree.addPattern("GET:/posts/{var}"); + tree.addPattern("https://abc.com/posts/{var}";); StringFormatGroup.FormatResult result; result = tree.match("/products/123"); @@ -77,13 +100,34 @@ class PatternTreeTest { result = tree.match("/employees/skywalking/profile"); Assertions.assertTrue(result.isMatch()); - // URI doesn't have / as prefix + // URI doesn't have / as prefix but should still match result = tree.match("products/123/detail"); + Assertions.assertTrue(result.isMatch()); + + // URI has / as suffix but should still match + result = tree.match("products/123/detail/"); + Assertions.assertTrue(result.isMatch()); + + // URI shorter than pattern + result = tree.match("/products/123/reorder"); Assertions.assertFalse(result.isMatch()); + Assertions.assertEquals("/products/123/reorder", result.getReplacedName()); // URI has extra suffix result = tree.match("/products/123/detail/extra"); Assertions.assertFalse(result.isMatch()); + Assertions.assertEquals("/products/123/detail/extra", result.getReplacedName()); + + // Domain style URI + result = tree.match("https://abc.com/posts/123abc";); + Assertions.assertTrue(result.isMatch()); + + // Special case: When endpoint is like /abc or abc, + // it will always match since itself cannot be a variable + // regardless if abc is actually in the pattern tree + result = tree.match("/abc"); + Assertions.assertTrue(result.isMatch()); + Assertions.assertEquals("/abc", result.getReplacedName()); } @Test diff --git a/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/StringFormatGroup.java b/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/StringFormatGroup.java index 285ac872ca..c1efc6c67d 100644 --- a/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/StringFormatGroup.java +++ b/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/StringFormatGroup.java @@ -68,7 +68,7 @@ public class StringFormatGroup { public FormatResult format(String string) { for (PatternRule rule : rules) { if (rule.getPattern().matcher(string).matches()) { - return new FormatResult(true, rule.getName(), string); + return new FormatResult(true, string, rule.getName()); } } return new FormatResult(false, string, string);
