Skip to content

Commit 29339c9

Browse files
committed
Add PPL IT coverage report against analytics-engine path
Run every PPL IT through the analytics-engine planner and post-process the JUnit XMLs into a markdown coverage report bucketed by exception type and normalized error message. Lets us measure compatibility gaps between the analytics-engine path and the v2 path without a CI break. End-to-end usage: 1. Start an OpenSearch node with the analytics-engine plugin set installed and the SQL plugin built locally. 2. ./gradlew :integ-test:analyticsCompatibilityReport \ -Dtests.rest.cluster=localhost:9200 \ -Dtests.cluster=localhost:9300 \ -Dtests.clustername=runTask 3. Read integ-test/build/reports/analytics-compatibility/REPORT.md. How force-routing works: - New cluster setting `plugins.calcite.analytics.force_routing` (off by default; test-only) makes RestUnifiedQueryAction.isAnalyticsIndex() return true unconditionally so every query traverses the analytics-engine path regardless of index name. - The Gradle test task sets `-Dtests.analytics.force_routing=true`. PPLIntegTestCase.init() reads that property and flips the cluster setting on at test setup via enableAnalyticsForceRouting(). - The analyticsCompatibilityTest task runs `ignoreFailures=true` so a failure-rich report doesn't break the build — failures are the signal we're after. The analyticsCompatibilityReport task always runs (no input/output declarations) so the "no results" log line is always visible when -Dtests.rest.cluster is missing. Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 5c5ffa2 commit 29339c9

6 files changed

Lines changed: 409 additions & 7 deletions

File tree

common/src/main/java/org/opensearch/sql/common/setting/Settings.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ public enum Key {
4545
"plugins.calcite.pushdown.rowcount.estimation.factor"),
4646
CALCITE_SUPPORT_ALL_JOIN_TYPES("plugins.calcite.all_join_types.allowed"),
4747

48+
/**
49+
* Force-route every PPL/SQL query through the analytics engine path, regardless of the index
50+
* name pattern. Test-only knob used by the analytics compatibility report; should remain {@code
51+
* false} in production. When {@code true}, {@link
52+
* org.opensearch.sql.plugin.rest.RestUnifiedQueryAction#isAnalyticsIndex} returns {@code true}
53+
* unconditionally.
54+
*/
55+
CALCITE_ANALYTICS_FORCE_ROUTING("plugins.calcite.analytics.force_routing"),
56+
4857
/** Query Settings. */
4958
FIELD_TYPE_TOLERANCE("plugins.query.field_type_tolerance"),
5059

integ-test/build.gradle

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,3 +797,312 @@ task integTestRemote(type: RestIntegTestTask) {
797797
exclude 'org/opensearch/sql/legacy/OrderIT.class'
798798
exclude 'org/opensearch/sql/jdbc/**'
799799
}
800+
801+
// =============================================================================
802+
// Analytics-engine PPL IT compatibility report
803+
//
804+
// Runs every PPL IT through the analytics-engine path against an externally-
805+
// managed cluster (started with the analytics-engine plugin set installed),
806+
// then post-processes the JUnit XMLs into a markdown coverage report bucketed
807+
// by exception type and normalized error message.
808+
//
809+
// End-to-end usage:
810+
//
811+
// 1. Start an OpenSearch node with the analytics-engine plugin set
812+
// installed and the SQL plugin built locally.
813+
//
814+
// 2. Run:
815+
// ./gradlew :integ-test:analyticsCompatibilityReport \
816+
// -Dtests.rest.cluster=localhost:9200 \
817+
// -Dtests.cluster=localhost:9300 \
818+
// -Dtests.clustername=runTask
819+
//
820+
// 3. Read the report at:
821+
// integ-test/build/reports/analytics-compatibility/REPORT.md
822+
//
823+
// The test task always succeeds (`ignoreFailures = true`) so the report is
824+
// produced even when the analytics path can't yet handle a command — failures
825+
// are the signal we're after, not a build break.
826+
//
827+
// How the test-side hook works (PPLIntegTestCase.java):
828+
// - The test task sets `-Dtests.analytics.force_routing=true`.
829+
// - PPLIntegTestCase.init() reads that property and calls
830+
// enableAnalyticsForceRouting() during setup, which flips the cluster
831+
// setting plugins.calcite.analytics.force_routing=true.
832+
// - RestUnifiedQueryAction.isAnalyticsIndex() honors that setting and
833+
// returns true unconditionally — so every query routes through the
834+
// analytics-engine planner regardless of index name.
835+
// =============================================================================
836+
task analyticsCompatibilityTest(type: RestIntegTestTask) {
837+
description = "Runs all PPL ITs through the analytics-engine path. Always succeeds; results " +
838+
"are post-processed by analyticsCompatibilityReport."
839+
group = "verification"
840+
841+
testLogging {
842+
events "skipped", "failed"
843+
exceptionFormat "short"
844+
}
845+
testClassesDirs = sourceSets.test.output.classesDirs
846+
classpath = sourceSets.test.runtimeClasspath
847+
systemProperty 'tests.security.manager', 'false'
848+
systemProperty 'project.root', project.projectDir.absolutePath
849+
systemProperty 'java.io.tmpdir', opensearch_tmp_dir.absolutePath
850+
systemProperty 'defaultQuerySizeLimit', '10000'
851+
852+
// Force every PPL query through the analytics-engine path. Picked up by
853+
// PPLIntegTestCase.init() via the ANALYTICS_FORCE_ROUTING_PROP constant.
854+
systemProperty 'tests.analytics.force_routing', 'true'
855+
856+
systemProperty "https", System.getProperty("https")
857+
systemProperty "user", System.getProperty("user")
858+
systemProperty "password", System.getProperty("password")
859+
860+
// Don't fail the build on test failures — failures are the report payload.
861+
ignoreFailures = true
862+
863+
reports {
864+
junitXml.required.set(true)
865+
junitXml.outputLocation.set(file("$buildDir/test-results/analyticsCompatibility"))
866+
html.required.set(true)
867+
html.outputLocation.set(file("$buildDir/reports/tests/analyticsCompatibility"))
868+
}
869+
870+
// PPL ITs only: both the calcite remote suite and the legacy ppl/ package.
871+
filter {
872+
includeTestsMatching "org.opensearch.sql.calcite.remote.*IT"
873+
includeTestsMatching "org.opensearch.sql.ppl.*IT"
874+
}
875+
876+
// BWC, correctness, doctest and legacy oddballs aren't in scope for the analytics report.
877+
exclude 'org/opensearch/sql/bwc/**'
878+
exclude 'org/opensearch/sql/doctest/**/*IT.class'
879+
exclude 'org/opensearch/sql/correctness/**'
880+
exclude 'org/opensearch/sql/jdbc/**'
881+
exclude 'org/opensearch/sql/security/**'
882+
883+
// The test task hits whatever cluster `-Dtests.rest.cluster=...` points at; no testClusters
884+
// dependency. Only run if the user supplied a cluster.
885+
onlyIf {
886+
if (System.getProperty("tests.rest.cluster") == null) {
887+
logger.lifecycle(
888+
"[analyticsCompatibility] Skipping: pass -Dtests.rest.cluster=<host:port> " +
889+
"(and optionally -Dtests.cluster=, -Dtests.clustername=) pointing at " +
890+
"an externally-running cluster with the analytics-engine plugin set installed.")
891+
return false
892+
}
893+
return true
894+
}
895+
}
896+
897+
task analyticsCompatibilityReport {
898+
description = "Generates a markdown coverage report classifying analyticsCompatibilityTest " +
899+
"failures by exception type, with messages normalized so per-index/per-field " +
900+
"variations collapse into a single bucket."
901+
group = "verification"
902+
dependsOn analyticsCompatibilityTest
903+
904+
def xmlDir = file("$buildDir/test-results/analyticsCompatibility")
905+
def reportDir = file("$buildDir/reports/analytics-compatibility")
906+
def reportFile = file("$reportDir/REPORT.md")
907+
908+
// No inputs/outputs declared on purpose: the upstream test task is gated on
909+
// -Dtests.rest.cluster and may produce no XMLs. Up-to-date checks would
910+
// suppress the "no results" log line we want users to see.
911+
doLast {
912+
if (!xmlDir.exists()) {
913+
logger.lifecycle("[analyticsCompatibilityReport] No test results at ${xmlDir}; nothing to report.")
914+
return
915+
}
916+
reportDir.mkdirs()
917+
918+
// Collapse variable text so similar errors land in one bucket. E.g.
919+
// "No backend can scan all requested fields on index [opensearch-sql_test_index_bank]"
920+
// "No backend can scan all requested fields on index [opensearch-sql_test_index_account]"
921+
// both normalize to:
922+
// "No backend can scan all requested fields on index [<INDEX>]"
923+
Closure normalize = { String s ->
924+
if (s == null) return ''
925+
String n = s
926+
n = n.replaceAll(/(?i)on index \[[^\]]+\]/, 'on index [<INDEX>]')
927+
n = n.replaceAll(/(?i)index \[[^\]]+\]/, 'index [<INDEX>]')
928+
n = n.replaceAll(/Field \[[^\]]+\]/, 'Field [<FIELD>]')
929+
n = n.replaceAll(/Field '[^']+'/, "Field '<FIELD>'")
930+
n = n.replaceAll(/Table '[^']+'/, "Table '<TABLE>'")
931+
n = n.replaceAll(/Line \d+, Column \d+/, 'Line <L>, Column <C>')
932+
return n
933+
}
934+
935+
def parser = new groovy.xml.XmlParser(false, false)
936+
int totalTests = 0
937+
int totalFailures = 0
938+
int totalErrors = 0
939+
int totalSkipped = 0
940+
long totalTimeMs = 0
941+
// Bucket: "<ExcLabel> | <normalized message>" -> [list of "Class.method"]
942+
def buckets = new java.util.LinkedHashMap<String, java.util.List<String>>()
943+
// Per-class summary: className -> [tests, failures+errors, skipped]
944+
def perClass = new java.util.TreeMap<String, int[]>()
945+
946+
xmlDir.eachFileMatch(~/TEST-.*\.xml/) { xmlFile ->
947+
def root
948+
try {
949+
root = parser.parse(xmlFile)
950+
} catch (Exception e) {
951+
logger.warn("[analyticsCompatibilityReport] Failed to parse ${xmlFile.name}: ${e.message}")
952+
return
953+
}
954+
def className = root.@name?.toString() ?: xmlFile.name
955+
int classTests = (root.@tests ?: '0').toString().toInteger()
956+
int classFailures = (root.@failures ?: '0').toString().toInteger()
957+
int classErrors = (root.@errors ?: '0').toString().toInteger()
958+
int classSkipped = (root.@skipped ?: '0').toString().toInteger()
959+
double classTime = ((root.@time ?: '0') as String).replace(',', '.').toDouble()
960+
961+
totalTests += classTests
962+
totalFailures += classFailures
963+
totalErrors += classErrors
964+
totalSkipped += classSkipped
965+
totalTimeMs += (long) (classTime * 1000)
966+
perClass[className] = [classTests, classFailures + classErrors, classSkipped] as int[]
967+
968+
root.testcase.each { tc ->
969+
def testName = tc.@name?.toString() ?: '<unknown>'
970+
def fqid = "${className}#${testName}"
971+
def failureNode = tc.failure ? tc.failure[0] : (tc.error ? tc.error[0] : null)
972+
if (failureNode == null) return
973+
974+
def excType = failureNode.@type?.toString() ?: '<unknown>'
975+
def msg = failureNode.@message?.toString() ?: ''
976+
def body = failureNode.text() ?: ''
977+
978+
// ResponseException collapses every 500/4xx into one bucket because its message
979+
// starts with the same wrapper text. Reach into the response body and extract the
980+
// server's actual exception class + reason — that's what we actually want to bucket
981+
// on. Same for AssertionError that wraps a ResponseException stacktrace.
982+
def excLabel = excType
983+
def primary = msg
984+
def innerType = (msg =~ /"type"\s*:\s*"([^"]+)"/)
985+
def innerReason = (msg =~ /"reason"\s*:\s*"([^"]+)"/)
986+
if (excType.endsWith('ResponseException') || excType.endsWith('AssertionError')) {
987+
if (innerType.find()) {
988+
excLabel = "${excType}${innerType.group(1)}"
989+
}
990+
innerReason.reset()
991+
if (innerReason.find()) primary = innerReason.group(1)
992+
}
993+
994+
if (primary == null || primary.trim().isEmpty()) {
995+
primary = body.split(/\r?\n/).find { it != null && !it.trim().isEmpty() } ?: ''
996+
}
997+
primary = primary.replace('\n', ' ').replace('\r', ' ').trim()
998+
999+
String normalized = normalize(primary)
1000+
if (normalized.length() > 200) normalized = normalized.substring(0, 200) + ''
1001+
1002+
def key = "${excLabel} | ${normalized}".toString()
1003+
buckets.computeIfAbsent(key, { new java.util.ArrayList<String>() }).add(fqid)
1004+
}
1005+
}
1006+
1007+
int totalFailed = totalFailures + totalErrors
1008+
int totalPassed = totalTests - totalFailed - totalSkipped
1009+
double passPct = totalTests == 0 ? 0.0 : (100.0 * totalPassed / totalTests)
1010+
1011+
def sb = new StringBuilder()
1012+
sb << "# Analytics-engine PPL IT Compatibility Report\n\n"
1013+
sb << "Generated: ${new Date()}\n\n"
1014+
sb << "Cluster under test: `${System.getProperty('tests.rest.cluster') ?: '<unspecified>'}`\n\n"
1015+
sb << "Routing: every PPL query forced through the analytics-engine path " +
1016+
"(`tests.analytics.force_routing=true`, which sets the cluster setting " +
1017+
"`plugins.calcite.analytics.force_routing=true`).\n\n"
1018+
sb << "## Summary\n\n"
1019+
sb << "| Metric | Count |\n"
1020+
sb << "|---|---|\n"
1021+
sb << "| Tests executed | ${totalTests} |\n"
1022+
sb << "| Passed | ${totalPassed} |\n"
1023+
sb << "| Failed | ${totalFailures} |\n"
1024+
sb << "| Errored | ${totalErrors} |\n"
1025+
sb << "| Skipped | ${totalSkipped} |\n"
1026+
sb << "| Pass rate | **${String.format('%.1f', passPct)}%** |\n"
1027+
sb << "| Total time | ${String.format('%.1f', totalTimeMs / 1000.0)}s |\n\n"
1028+
1029+
int TOP_BUCKETS = 25
1030+
Closure cell = { String s ->
1031+
(s ?: '').replace('|', '/').replace('\n', ' ').replace('\r', ' ').trim()
1032+
}
1033+
Closure clip = { String s, int n -> s.length() > n ? s.substring(0, n - 1) + '' : s }
1034+
1035+
def sortedBuckets = buckets.isEmpty() ? [] : buckets.entrySet().toList().sort { -it.value.size() }
1036+
int totalFailedAll = (int) sortedBuckets.sum(0) { it.value.size() }
1037+
int remainingBuckets = Math.max(0, sortedBuckets.size() - TOP_BUCKETS)
1038+
int remainingFails = remainingBuckets == 0 ? 0 :
1039+
(int) sortedBuckets.drop(TOP_BUCKETS).sum(0) { it.value.size() }
1040+
1041+
sb << "## Top ${Math.min(TOP_BUCKETS, sortedBuckets.size())} failure buckets\n\n"
1042+
sb << "Cells are clipped to keep table renderers from line-wrapping. The full uncut messages live in `integ-test/build/test-results/analyticsCompatibility/`.\n\n"
1043+
if (sortedBuckets.isEmpty()) {
1044+
sb << "_No failures._\n\n"
1045+
} else {
1046+
sb << "| # | Count | Error |\n"
1047+
sb << "|---:|---:|---|\n"
1048+
int idx = 0
1049+
sortedBuckets.take(TOP_BUCKETS).each { entry ->
1050+
idx++
1051+
def parts = entry.key.split(/ \| /, 2)
1052+
def excType = parts.length > 0 ? parts[0] : '<unknown>'
1053+
def msg = parts.length > 1 ? parts[1] : ''
1054+
def shortExc = excType.replaceFirst(/^.*?(ResponseException|AssertionError) → /, '')
1055+
shortExc = shortExc.replaceFirst(/^java\.lang\./, '').replaceFirst(/^org\.opensearch\.[^ ]*\./, '')
1056+
def combined = "${shortExc}${msg}"
1057+
sb << "| ${idx} | ${entry.value.size()} | ${cell(clip(combined, 90))} |\n"
1058+
}
1059+
if (remainingBuckets > 0) {
1060+
sb << "\n_${remainingBuckets} more buckets cover the remaining ${remainingFails} failures (long tail, each ≤${sortedBuckets.get(TOP_BUCKETS).value.size()} tests)._\n\n"
1061+
}
1062+
}
1063+
1064+
// ---- Per-class summary ------------------------------------------
1065+
def perClassActive = []
1066+
int perClassZeroCount = 0
1067+
perClass.each { className, counts ->
1068+
int t = counts[0], f = counts[1], s = counts[2]
1069+
if (s > 0 || (t - f - s) > 0) perClassActive.add([className, counts])
1070+
else perClassZeroCount++
1071+
}
1072+
perClassActive.sort { a, b ->
1073+
int[] ca = a[1], cb = b[1]
1074+
int pa = ca[0] - ca[1] - ca[2], pb = cb[0] - cb[1] - cb[2]
1075+
double pctA = ca[0] == 0 ? 0.0 : (100.0 * pa / ca[0])
1076+
double pctB = cb[0] == 0 ? 0.0 : (100.0 * pb / cb[0])
1077+
pctB <=> pctA ?: cb[0] <=> ca[0]
1078+
}
1079+
sb << "## Per-class pass-rate highlights\n\n"
1080+
sb << "Of ${perClass.size()} PPL IT classes:\n\n"
1081+
def fullyPassing = perClassActive.findAll { row ->
1082+
int[] c = row[1]; int t = c[0], f = c[1], s = c[2]
1083+
t > 0 && f == 0 && s == 0
1084+
}
1085+
sb << "- **${fullyPassing.size()} pass 100%** — ${fullyPassing.collect { it[0].toString().replaceFirst(/^.*\./, '') }.join(', ')}\n"
1086+
def partial = perClassActive.findAll { row ->
1087+
int[] c = row[1]; int t = c[0], f = c[1], s = c[2]
1088+
int p = t - f - s
1089+
t > 0 && p > 0 && p < t
1090+
}
1091+
if (!partial.isEmpty()) {
1092+
sb << "- **${partial.size()} partially pass** (≥1 passing test). Top by pass-rate: " +
1093+
partial.take(8).collect { row ->
1094+
int[] c = row[1]
1095+
int t = c[0], f = c[1], s = c[2], p = t - f - s
1096+
int pct = t == 0 ? 0 : (int) Math.round(100.0 * p / t)
1097+
"${row[0].toString().replaceFirst(/^.*\./, '')} ${pct}%"
1098+
}.join(', ') + "\n"
1099+
}
1100+
sb << "- **${perClassZeroCount} fail every test.** Full per-class breakdown is in the JUnit HTML report at `integ-test/build/reports/tests/analyticsCompatibility/index.html`.\n"
1101+
sb << "\n"
1102+
1103+
reportFile.text = sb.toString()
1104+
logger.lifecycle("[analyticsCompatibilityReport] Wrote ${reportFile}")
1105+
logger.lifecycle("[analyticsCompatibilityReport] ${totalPassed}/${totalTests} passed " +
1106+
"(${String.format('%.1f', passPct)}%), ${totalFailed} failed, ${totalSkipped} skipped")
1107+
}
1108+
}

integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,38 @@ public abstract class PPLIntegTestCase extends SQLIntegTestCase {
4242
public static final Integer DEFAULT_SUBSEARCH_MAXOUT = 10000;
4343
public static final Integer DEFAULT_JOIN_SUBSEARCH_MAXOUT = 50000;
4444

45+
/**
46+
* System property toggle for the analytics compatibility report. When set, every PPL IT base
47+
* setup forces the analytics routing flag on so all queries traverse the analytics-engine path.
48+
* Off by default — normal CI runs untouched.
49+
*/
50+
public static final String ANALYTICS_FORCE_ROUTING_PROP = "tests.analytics.force_routing";
51+
4552
@Override
4653
protected void init() throws Exception {
4754
super.init();
4855
updatePushdownSettings();
4956
disableCalcite(); // calcite is enabled by default from 3.3.0
57+
if (Boolean.parseBoolean(System.getProperty(ANALYTICS_FORCE_ROUTING_PROP, "false"))) {
58+
enableCalcite();
59+
enableAnalyticsForceRouting();
60+
}
61+
}
62+
63+
/**
64+
* Force every PPL/SQL query to route through the analytics-engine path regardless of the index
65+
* name pattern. Used by the analytics compatibility report.
66+
*/
67+
public static void enableAnalyticsForceRouting() throws IOException {
68+
updateClusterSettings(
69+
new SQLIntegTestCase.ClusterSetting(
70+
"persistent", Settings.Key.CALCITE_ANALYTICS_FORCE_ROUTING.getKeyValue(), "true"));
71+
}
72+
73+
public static void disableAnalyticsForceRouting() throws IOException {
74+
updateClusterSettings(
75+
new SQLIntegTestCase.ClusterSetting(
76+
"persistent", Settings.Key.CALCITE_ANALYTICS_FORCE_ROUTING.getKeyValue(), "false"));
5077
}
5178

5279
protected JSONObject executeQuery(String query) throws IOException {

0 commit comments

Comments
 (0)