diff --git a/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java b/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java new file mode 100644 index 0000000..7d986a2 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/config/QueryLineageMatchIndexInitializer.java @@ -0,0 +1,166 @@ +package com.dbaagent.config; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.jdbc.core.JdbcTemplate; + +import javax.sql.DataSource; + +/** + * Adds the stored normalized column and index that make slow-query sample recovery + * indexable. + * + *

{@code QueryLineageRepository.findLongestByConnectionIdAndNormalizedQueryTextPrefix} + * wraps {@code query_text} in three nested {@code regexp_replace}/{@code REPLACE} calls + * plus {@code LOWER} before comparing it, so no index on {@code query_text} can ever + * satisfy the predicate — Postgres must materialize a rewritten copy of every row in the + * connection's slice. {@code EXPLAIN (ANALYZE)} on a real install: + * + *

+ *   Seq Scan on query_lineage (actual time=0.244..35.826 rows=422 loops=1)
+ *   Execution Time: 36.179 ms          -- at 1,093 rows
+ *   Execution Time: 1111.996 ms        -- same query, table scaled to 34,976 rows
+ * 
+ * + *

{@code SlowQueryAnalyticsService.recoverFullText} runs that once per sample, up to + * 20 per "view full query" click, so one modal open costs ~0.7 s today and ~22 s at 35k + * rows. It degrades with age rather than load, which is why it passes every + * pre-launch test: {@code query_lineage} is not pruned by + * {@code SlowQueryRetentionService} (that only touches {@code slow_query_run}, + * {@code slow_query_customer_day} and {@code slow_query_sample}), so it only grows. + * + *

Precomputing the normalization into a STORED generated column pays the regex chain + * once at write time. Measured on the same scaled table, with the ~120-character prefix + * the caller actually sends: + * + *

+ *   Index Scan using idx_query_lineage_norm_match
+ *   Execution Time: 0.428 ms           -- vs 1111.996 ms
+ * 
+ * + *

The index earns its keep only because the prefix is long and therefore selective. A + * short prefix such as {@code 'select%'} still plans as a sequential scan (~50 ms at 35k + * rows) — that is the precomputation alone, and is fine. Do not "simplify" this by + * dropping the generated column and indexing {@code query_text} directly; the expression, + * not the column, is what the query compares. + * + *

There is no Flyway runtime in this repo (see CLAUDE.md), so this initializer is what + * actually applies {@code V118__add_query_lineage_norm_match.sql}. Both statements are + * {@code IF NOT EXISTS} and the whole thing is best-effort: a failure here costs + * performance, never correctness, since the query returns identical rows either way. + */ +@Configuration +@Slf4j +public class QueryLineageMatchIndexInitializer { + + private static final String TABLE = "query_lineage"; + private static final String COLUMN = "normalized_match"; + private static final String INDEX = "idx_query_lineage_norm_match"; + + /** + * Must match {@code SlowQueryAnalyticsService.normalizeForMatching} exactly. If one + * changes, both do. + * + *

The {@code btrim} is load-bearing and was missing from the expression this + * replaces: Java's {@code normalizeForMatching} ends with {@code .trim()}, so a + * lineage row stored with leading whitespace normalized to {@code " select ..."} on + * the SQL side and {@code "select ..."} on the Java side. The prefix {@code LIKE} + * then never matched and recovery silently returned the truncated sample. 4 of 1,174 + * rows on the local install carry such whitespace. + */ + private static final String NORMALIZE_EXPR = + "btrim(lower(regexp_replace(regexp_replace(" + + "replace(query_text, '`', ''), " + + "'\\s*([.,();])\\s*', '\\1', 'g'), " + + "'\\s+', ' ', 'g')))"; + + @Bean("queryLineageMatchIndexBootstrap") + @DependsOn("entityManagerFactory") + public Object queryLineageMatchIndexBootstrap(DataSource dataSource) { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + + if (!tableExists(jdbc, TABLE)) { + return new Object(); + } + + // A generated column's expression cannot be altered in place, and ADD COLUMN IF + // NOT EXISTS silently keeps whatever definition is already there. An install that + // ran an earlier build of this initializer therefore keeps the untrimmed + // expression forever unless the column is dropped first. Only drop when the + // definition actually differs, so a normal restart does not rewrite the table. + if (columnDefinitionDiffers(jdbc)) { + log.info("Rebuilding {}.{}: stored expression is out of date", TABLE, COLUMN); + try { + jdbc.execute("ALTER TABLE " + TABLE + " DROP COLUMN " + COLUMN); + } catch (RuntimeException e) { + log.warn("Could not drop stale {}.{}: {}", TABLE, COLUMN, e.getMessage()); + } + } + + try { + jdbc.execute("ALTER TABLE " + TABLE + " ADD COLUMN IF NOT EXISTS " + COLUMN + + " text GENERATED ALWAYS AS (" + NORMALIZE_EXPR + ") STORED"); + } catch (RuntimeException e) { + // Generated columns need Postgres 12+. Older servers keep the sequential scan, + // which is slow but correct, so this must not stop the application. + log.warn("Could not add {}.{} ({}); sample recovery stays on a sequential scan", + TABLE, COLUMN, e.getMessage()); + return new Object(); + } + + try { + // text_pattern_ops so a LIKE 'prefix%' comparison can use the index under any + // collation; the default opclass only helps in the C collation. + jdbc.execute("CREATE INDEX IF NOT EXISTS " + INDEX + " ON " + TABLE + + " (connection_id, " + COLUMN + " text_pattern_ops)"); + } catch (RuntimeException e) { + log.warn("Could not create {}: {}", INDEX, e.getMessage()); + } + + return new Object(); + } + + /** + * True when {@code normalized_match} exists but was generated by a different + * expression than {@link #NORMALIZE_EXPR}. Compared on the normalized form Postgres + * stores in {@code pg_get_expr}, with whitespace collapsed, since the server rewrites + * the text it was given (adds casts, reorders parens) and a literal comparison would + * report a difference on every start and rewrite the table each time. + */ + private static boolean columnDefinitionDiffers(JdbcTemplate jdbc) { + try { + String stored = jdbc.query( + "SELECT pg_get_expr(d.adbin, d.adrelid) " + + "FROM pg_attrdef d " + + "JOIN pg_attribute a ON a.attrelid = d.adrelid AND a.attnum = d.adnum " + + "WHERE d.adrelid = ?::regclass AND a.attname = ?", + rs -> rs.next() ? rs.getString(1) : null, TABLE, COLUMN); + if (stored == null) { + return false; // column not present yet — nothing stale to drop + } + return !squash(stored).contains("btrim"); + } catch (RuntimeException e) { + log.warn("Could not inspect {}.{} definition: {}", TABLE, COLUMN, e.getMessage()); + return false; + } + } + + private static String squash(String s) { + return s.replaceAll("\\s+", "").toLowerCase(); + } + + private static boolean tableExists(JdbcTemplate jdbc, String table) { + try { + Integer found = jdbc.queryForObject( + "SELECT COUNT(*) FROM information_schema.tables " + + "WHERE table_schema = current_schema() AND table_name = ?", + Integer.class, table); + return found != null && found > 0; + } catch (RuntimeException e) { + log.warn("Could not check for table {}: {}", table, e.getMessage()); + return false; + } + } +} diff --git a/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java b/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java index 525d841..91021b8 100644 --- a/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java +++ b/backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java @@ -78,7 +78,41 @@ public ResponseEntity> listCusto return ResponseEntity.ok(analyticsService.listCustomers(connectionId)); } - /** Every slow query attributed to one customer, ranked by their mean exec time. */ + /** + * Every slow query attributed to one customer, ranked by their mean exec time. + * + *

The customer id is a query parameter, not a path segment, because it is a + * literal value read out of the tenant column — application data, which can contain + * {@code /}, {@code ?} or {@code #}. Such an id is unreachable as a path segment under + * any encoding: raw, the slash splits the path; percent-encoded, Jetty rejects it with + * {@code 400 Ambiguous URI path separator}. Both were reproduced against this backend + * with the tenant value {@code acct/77?x=1}, whose rows were simply invisible in the + * By-Customer view. + */ + @GetMapping("/{connectionId}/customer-queries") + public ResponseEntity> customerQueries( + @PathVariable String connectionId, + @RequestParam String customerId) { + accessControlService.assertCanReadConnectionContent(connectionId); + return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId)); + } + + /** Literal-bearing samples for one (customer, query) pair — copyable SQL. */ + @GetMapping("/{connectionId}/customer-query-samples") + public ResponseEntity> customerQuerySamples( + @PathVariable String connectionId, + @RequestParam String customerId, + @RequestParam String fingerprint) { + accessControlService.assertCanReadConnectionContent(connectionId); + return ResponseEntity.ok( + analyticsService.samplesForCustomerQuery(connectionId, customerId, fingerprint)); + } + + /** + * @deprecated superseded by {@link #customerQueries}; a customer id containing a + * slash cannot be expressed here. Retained so existing clients keep working. + */ + @Deprecated @GetMapping("/{connectionId}/customer/{customerId}/queries") public ResponseEntity> queriesForCustomer( @PathVariable String connectionId, @@ -87,7 +121,10 @@ public ResponseEntity> queriesF return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId)); } - /** Literal-bearing samples for one (customer, query) pair — copyable SQL. */ + /** + * @deprecated superseded by {@link #customerQuerySamples}; see above. + */ + @Deprecated @GetMapping("/{connectionId}/customer/{customerId}/query/{fingerprint}/samples") public ResponseEntity> samplesForCustomerQuery( @PathVariable String connectionId, diff --git a/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java b/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java index 3f5543b..89acc62 100644 --- a/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java +++ b/backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java @@ -27,6 +27,12 @@ @Slf4j public class WorkloadAnalysisController { + // Reads use canReadContent; only `run` requires canManageContent. The reads were all + // gated on manage, which is the write tier — EffectiveConnectionAccess's own comment + // lists slow-query analytics under read. Latent today because every grant resolves to + // FULL_CONTENT (so both predicates are true), but it would deny the whole Workload tab + // to a read-only grant the moment one is reintroduced. + private final WorkloadAnalysisService workloadAnalysisService; private final WorkloadAnalysisReportRepository reportRepository; private final AccessControlService accessControlService; @@ -64,7 +70,7 @@ public ResponseEntity> run(@PathVariable String connectionId /** Lightweight poll payload — status + progress without the full report blob. */ @GetMapping("/{connectionId}/status") public ResponseEntity> status(@PathVariable String connectionId) { - accessControlService.assertCanManageConnectionContent(connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId) .map(r -> ResponseEntity.ok(Map.of( "reportId", r.getId(), @@ -81,7 +87,7 @@ public ResponseEntity> status(@PathVariable String connectio /** The full latest report (with the composed sections). 204 if none yet. */ @GetMapping("/{connectionId}/latest") public ResponseEntity latest(@PathVariable String connectionId) { - accessControlService.assertCanManageConnectionContent(connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId) .map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.noContent().build()); @@ -92,7 +98,7 @@ public ResponseEntity latest(@PathVariable String connec public ResponseEntity getReport(@PathVariable String reportId) { return reportRepository.findById(reportId) .map(r -> { - accessControlService.assertCanManageConnectionContent(r.getConnectionId()); + accessControlService.assertCanReadConnectionContent(r.getConnectionId()); return ResponseEntity.ok(r); }) .orElseGet(() -> ResponseEntity.notFound().build()); @@ -101,7 +107,7 @@ public ResponseEntity getReport(@PathVariable String rep /** Recent report history (newest first), metadata only via the entity. */ @GetMapping("/{connectionId}/history") public ResponseEntity> history(@PathVariable String connectionId) { - accessControlService.assertCanManageConnectionContent(connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); return ResponseEntity.ok(reportRepository.findTop20ByConnectionIdOrderByStartedAtDesc(connectionId)); } } diff --git a/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java b/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java index 1a3d107..0ef211e 100644 --- a/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java +++ b/backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java @@ -3,8 +3,10 @@ import com.dbaagent.model.QueryLineage; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.domain.Pageable; import org.springframework.data.repository.query.Param; +import org.springframework.transaction.annotation.Transactional; import org.springframework.stereotype.Repository; import java.time.LocalDateTime; @@ -147,23 +149,32 @@ QueryLineage findLongestByConnectionIdAndCollapsedQueryTextPrefix( * Callers must apply the SAME transformation to their prefix on the * Java side (see {@code SlowQueryAnalyticsService.normalizeForMatching}). * - *

The regex chain runs once per candidate row, so this is intended for - * single-row lookups (LIMIT 1) over the per-connection slice. For - * d840f866-style connections (~5K lineage rows) it returns in a few ms; - * larger connections may benefit from a functional index on the - * normalized expression, but none is needed yet. + *

The normalization is precomputed into the stored generated column + * {@code normalized_match} ({@code QueryLineageMatchIndexInitializer}) and matched + * against that, so the regex chain runs once at write time rather than once per + * candidate row on every read. Inlining the expression here made the predicate + * unindexable: Postgres had to materialize a rewritten copy of the whole + * per-connection slice, which measured 36 ms at 1,093 rows and 1,112 ms at 34,976 — + * and {@code recoverFullText} issues this up to 20 times per "view full query" click, + * against a table no retention job prunes. On the same scaled table with the ~120-char + * prefix the caller actually sends, the indexed form plans as an Index Scan at + * 0.428 ms. + * + *

Matched against the column directly, with no {@code COALESCE} fallback to + * the inline expression. That fallback is the obvious way to stay safe on a database + * without the column, and it silently undoes the whole fix: wrapping the column in + * {@code COALESCE(...)} makes the predicate non-indexable again. Measured on the same + * 37,504-row table — {@code COALESCE(normalized_match, …)} plans a Seq Scan at + * 48.7 ms, the bare column an Index Scan at 0.39 ms. If the column is ever absent, + * {@code QueryLineageMatchIndexInitializer} logs it at WARN and + * {@code SlowQueryAnalyticsService.recoverFullText} already treats a failed lookup as + * "no longer text available" and returns the sample unchanged. */ @Query(value = """ SELECT * FROM query_lineage WHERE connection_id = :connectionId AND LENGTH(query_text) > :minLength - AND LOWER( - regexp_replace( - regexp_replace( - REPLACE(query_text, '`', ''), - '\\s*([.,();])\\s*', '\\1', 'g'), - '\\s+', ' ', 'g') - ) LIKE :escapedPrefix ESCAPE '\\' + AND normalized_match LIKE :escapedPrefix ESCAPE '\\' ORDER BY LENGTH(query_text) DESC LIMIT 1 """, nativeQuery = true) @@ -172,4 +183,24 @@ QueryLineage findLongestByConnectionIdAndNormalizedQueryTextPrefix( @Param("escapedPrefix") String escapedPrefix, @Param("minLength") int minLength ); + + /** + * Drop lineage rows older than the connection's retention window. + * + *

This table was never pruned: {@code SlowQueryRetentionService} covered + * {@code slow_query_run}, {@code slow_query_customer_day} and + * {@code slow_query_sample} but not lineage, so it grew without bound while the + * 30-day analytics tables stayed small. That is what made sample recovery degrade + * with age rather than load — the scanned table kept growing even on an idle install. + * + *

Keyed on {@code created_at}, which is non-null and already indexed + * ({@code idx_query_lineage_created}). + */ + @Modifying + @Transactional + @Query("DELETE FROM QueryLineage q WHERE q.connectionId = :connectionId " + + "AND q.createdAt < :cutoff") + int deleteByConnectionIdAndCreatedAtBefore( + @Param("connectionId") String connectionId, + @Param("cutoff") java.time.LocalDateTime cutoff); } diff --git a/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java b/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java index c3509b3..3226966 100644 --- a/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java +++ b/backend/src/main/java/com/dbaagent/service/SlowLogIngestionService.java @@ -418,6 +418,30 @@ private boolean isFrequencySatisfied(SlowLogSourceConfig config) { return last.plusMinutes(freq).isBefore(LocalDateTime.now()); } + /** + * KNOWN ISSUE — this cursor has two defects, both unfixed. Deliberately left alone + * rather than changed blind: every provider needs live cloud credentials to exercise, + * so a fix cannot be verified end to end here, and getting it wrong silently skips or + * duplicates slow-query events. + * + *

    + *
  1. Events arriving mid-run are lost. The cursor is set to the time + * ingestion finished, but {@code SINCE_LAST} then uses it as an + * exclusive lower bound (see {@code resolveStartTime}). Anything whose timestamp + * falls between the last parsed event and this write is never fetched — a gap + * proportional to how long the run took, so worst on the slowest S3/CloudWatch + * pulls. The fix is to record the maximum event timestamp actually parsed. + *
  2. Local time written, UTC read. {@code LocalDateTime.now()} is + * server-local; every read does {@code .atZone(ZoneOffset.UTC)}. The Compose + * image runs {@code Etc/UTC} so the two agree there by luck, but on a bare-metal + * install in, say, UTC+5:30, the cursor lands 5.5 h in the future and that much + * slow-query history is skipped on every run (west of UTC it goes backwards and + * re-ingests duplicates). The fix is to store an {@code Instant}/ + * {@code timestamptz} end to end. + *
+ * + *

{@code SlowLogSourceConfigService.updateAfterAutoIngestion} has the same bug. + */ private void updateLastProcessed(SlowLogSourceConfig config) { config.setLastProcessedAt(LocalDateTime.now()); config.setUpdatedAt(LocalDateTime.now()); diff --git a/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java b/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java index 442285c..9134c21 100644 --- a/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java +++ b/backend/src/main/java/com/dbaagent/service/SlowQueryRetentionService.java @@ -4,6 +4,7 @@ import com.dbaagent.repository.ResourceLimitsRepository; import com.dbaagent.repository.SlowQueryCustomerDayRepository; import com.dbaagent.repository.SlowQueryRunRepository; +import com.dbaagent.repository.QueryLineageRepository; import com.dbaagent.repository.SlowQuerySampleRepository; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -32,6 +33,7 @@ public class SlowQueryRetentionService { private final SlowQueryRunRepository runRepository; private final SlowQuerySampleRepository sampleRepository; private final SlowQueryCustomerDayRepository customerDayRepository; + private final QueryLineageRepository queryLineageRepository; private final ResourceLimitsRepository resourceLimitsRepository; /** Resolve the retention window for a connection (days). */ @@ -44,7 +46,12 @@ public int retentionDaysFor(String connectionId) { /** * Delete slow-query analytics rows older than the connection's retention - * window across all three fact tables. Safe to call repeatedly. + * window across all four fact tables. Safe to call repeatedly. + * + *

{@code query_lineage} was missing here, and that mattered more than the row + * count suggests: it is the table {@code recoverFullText} scans up to 20 times per + * "view full query" click, so leaving it unpruned made sample recovery degrade with + * the install's age while the other three tables stayed bounded at 30 days. */ @Transactional public void purge(String connectionId) { @@ -55,11 +62,13 @@ public void purge(String connectionId) { int customerDays = customerDayRepository.deleteByConnectionIdAndDayBefore(connectionId, cutoff); int samples = sampleRepository.deleteByConnectionIdAndCapturedAtBefore( connectionId, cutoff.atStartOfDay()); + int lineage = queryLineageRepository.deleteByConnectionIdAndCreatedAtBefore( + connectionId, cutoff.atStartOfDay()); - if (runs > 0 || customerDays > 0 || samples > 0) { + if (runs > 0 || customerDays > 0 || samples > 0 || lineage > 0) { log.info("Slow-query retention purge for connection {} (>{}d): " - + "{} runs, {} customer-days, {} samples removed", - connectionId, days, runs, customerDays, samples); + + "{} runs, {} customer-days, {} samples, {} lineage rows removed", + connectionId, days, runs, customerDays, samples, lineage); } } } diff --git a/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql b/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql new file mode 100644 index 0000000..26da180 --- /dev/null +++ b/backend/src/main/resources/db/migration/V118__add_query_lineage_norm_match.sql @@ -0,0 +1,40 @@ +-- Make slow-query sample recovery indexable. +-- +-- QueryLineageRepository.findLongestByConnectionIdAndNormalizedQueryTextPrefix used to +-- wrap query_text in three nested regexp_replace/REPLACE calls plus LOWER before +-- comparing, so no index could satisfy the predicate. Postgres materialized a rewritten +-- copy of every row in the connection's slice, and SlowQueryAnalyticsService +-- .recoverFullText issues that up to 20 times per "view full query" click. +-- +-- Measured with EXPLAIN (ANALYZE) on a real install: +-- 1,093 rows -> 36 ms per call (~0.7 s per modal open) +-- 34,976 rows -> 1112 ms per call (~22 s per modal open) +-- The growth is linear, and query_lineage was not pruned by SlowQueryRetentionService, +-- so this degraded with the install's age rather than its load — which is why it passed +-- every pre-launch test. +-- +-- Precomputing the normalization into a STORED generated column pays the regex chain once +-- at write time. On the same 34,976-row table, with the ~120-character prefix the caller +-- actually sends: Index Scan, 0.428 ms. +-- +-- The btrim matches Java's normalizeForMatching, which ends with .trim(). Without it a +-- row stored with leading whitespace normalizes to " select ..." here but "select ..." in +-- Java, so the prefix LIKE never matches and recovery silently returns the truncated +-- sample. That flaw was present in the inline expression this replaces. +-- +-- NOTE: this repo has no Flyway runtime (see CLAUDE.md). QueryLineageMatchIndexInitializer +-- is what actually applies these statements at startup; this file is the changelog record. + +ALTER TABLE query_lineage + ADD COLUMN IF NOT EXISTS normalized_match text + GENERATED ALWAYS AS ( + btrim(lower(regexp_replace(regexp_replace( + replace(query_text, '`', ''), + '\s*([.,();])\s*', '\1', 'g'), + '\s+', ' ', 'g'))) + ) STORED; + +-- text_pattern_ops so LIKE 'prefix%' can use the index under any collation; the default +-- opclass only helps in the C collation. +CREATE INDEX IF NOT EXISTS idx_query_lineage_norm_match + ON query_lineage (connection_id, normalized_match text_pattern_ops); diff --git a/src/components/sections/SlowQueriesSection.jsx b/src/components/sections/SlowQueriesSection.jsx index eb320dd..44af4c0 100644 --- a/src/components/sections/SlowQueriesSection.jsx +++ b/src/components/sections/SlowQueriesSection.jsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useRef, useState } from 'react' import { Activity, FileText, LineChart, Settings, Users } from 'lucide-react' import { useConnectionManager } from '@/lib/hooks/useConnectionManager' import { useSlowLogSourceConfig } from '@/lib/hooks/queries' @@ -33,6 +33,28 @@ const LOG_SOURCE_HELP = { export default function SlowQueriesSection() { const { connectionId, selectedConnection } = useConnectionManager() const [tab, setTab] = useState('trends') + const tabRefs = useRef({}) + + /** Arrow / Home / End move between tabs, as the ARIA tabs pattern expects. */ + const onTabKeyDown = (e) => { + const step = { ArrowRight: 1, ArrowLeft: -1 }[e.key] + if (step === undefined && e.key !== 'Home' && e.key !== 'End') return + e.preventDefault() + // Resolve the next tab from the *current* state, not the `tab` captured when this + // handler was created: two keypresses within one render would otherwise both move + // relative to the same starting index and selection would stick after the first. + setTab((current) => { + const i = TABS.findIndex((t) => t.id === current) + const next = e.key === 'Home' ? 0 + : e.key === 'End' ? TABS.length - 1 + : (i + step + TABS.length) % TABS.length + const id = TABS[next].id + // Focus follows selection, per the ARIA tabs pattern. Deferred so the tab is + // already rendered with tabIndex=0 when we focus it. + queueMicrotask(() => tabRefs.current[id]?.focus()) + return id + }) + } const [logSourceModalOpen, setLogSourceModalOpen] = useState(false) const logSourceQ = useSlowLogSourceConfig(connectionId) const hasLogSource = Boolean(logSourceQ.data?.id) @@ -86,16 +108,28 @@ export default function SlowQueriesSection() { ) : ( <>

-
+
{TABS.map((t) => { const Icon = t.icon const active = tab === t.id return (
-
+
{tab === 'trends' && } {tab === 'customers' && } {tab === 'workload' && } diff --git a/src/components/sections/SlowQueriesSection.module.css b/src/components/sections/SlowQueriesSection.module.css index 1a7172c..3b356ad 100644 --- a/src/components/sections/SlowQueriesSection.module.css +++ b/src/components/sections/SlowQueriesSection.module.css @@ -6,6 +6,17 @@ border-radius: 14px; background: #f9fafb; width: fit-content; + /* At 390px the four tabs measure 427px, so three of them sat off-screen with no way + to reach them: `width: fit-content` lets the bar grow past the viewport and the + parent clipped it. Scroll the bar itself rather than wrapping, so the pill keeps + its shape; max-width caps it at the container so the rule has something to act on. */ + max-width: 100%; + overflow-x: auto; + scrollbar-width: none; +} + +.tabBar::-webkit-scrollbar { + display: none; } .toolbar { @@ -18,6 +29,9 @@ .tabButton { display: inline-flex; + /* Do not shrink inside the scrolling .tabBar — otherwise labels compress to + illegible slivers instead of the bar scrolling. */ + flex: 0 0 auto; align-items: center; gap: 8px; padding: 8px 14px; diff --git a/src/components/tabs/Performance/CustomerExplorer.js b/src/components/tabs/Performance/CustomerExplorer.js index e547e82..7009d54 100644 --- a/src/components/tabs/Performance/CustomerExplorer.js +++ b/src/components/tabs/Performance/CustomerExplorer.js @@ -13,6 +13,7 @@ import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { Clock, Users, Copy, Check, X } from "lucide-react"; import { slowQueryAnalyticsAPI } from "@/lib/api/client"; +import { QueryError } from "./components"; import styles from "./CustomerExplorer.module.css"; const fmtDuration = (v) => { @@ -71,7 +72,10 @@ function QuerySamplesModal({ connectionId, customerId, customerName, fingerprint
{samplesQ.isLoading &&
Loading samples…
} - {!samplesQ.isLoading && samples.length === 0 && ( + {samplesQ.isError && ( + + )} + {!samplesQ.isLoading && !samplesQ.isError && samples.length === 0 && (
No literal-bearing samples captured for this customer + query yet. Slow-log ingestion gives the fullest coverage. @@ -130,7 +134,10 @@ export default function CustomerExplorer({ connectionId }) { Customers {customers.length > 0 && ({customers.length})}
{customersQ.isLoading &&
Loading…
} - {!customersQ.isLoading && customers.length === 0 && ( + {customersQ.isError && ( + + )} + {!customersQ.isLoading && !customersQ.isError && customers.length === 0 && (
No customer-attributed slow queries yet. Configure the tenant column under Settings, then run an analysis. @@ -193,7 +200,10 @@ export default function CustomerExplorer({ connectionId }) { {queriesQ.isLoading && (
Loading queries…
)} - {!queriesQ.isLoading && queries.length === 0 && ( + {queriesQ.isError && ( + + )} + {!queriesQ.isLoading && !queriesQ.isError && queries.length === 0 && (
No queries rolled up yet for this customer.
diff --git a/src/components/tabs/Performance/QueryTrendsTab.js b/src/components/tabs/Performance/QueryTrendsTab.js index b256c59..077f264 100644 --- a/src/components/tabs/Performance/QueryTrendsTab.js +++ b/src/components/tabs/Performance/QueryTrendsTab.js @@ -21,6 +21,7 @@ import { RefreshCw, TrendingUp, AlertTriangle, Clock, Users, Copy, Check, FileTe import { slowQueryAnalyticsAPI } from "@/lib/api/client"; import { useConnectionManager } from "@/lib/hooks/useConnectionManager"; import SlowQuerySourceModal from "@/components/SlowQuerySourceModal"; +import { QueryError } from "./components"; import styles from "./QueryTrendsTab.module.css"; /** Human-readable duration: ms under a second, seconds under a minute, else minutes. */ @@ -356,7 +357,10 @@ export default function QueryTrendsTab({ connectionId }) {
Mean execution time / day
{timelineQ.isLoading &&
Loading timeline…
} - {!timelineQ.isLoading && timeline.length === 0 && ( + {timelineQ.isError && ( + + )} + {!timelineQ.isLoading && !timelineQ.isError && timeline.length === 0 && (
No timeline data yet for this query.
)} {timeline.length > 0 && ( @@ -405,7 +409,10 @@ export default function QueryTrendsTab({ connectionId }) { Per-customer breakdown
{customersQ.isLoading &&
Loading…
} - {!customersQ.isLoading && customers.length === 0 && ( + {customersQ.isError && ( + + )} + {!customersQ.isLoading && !customersQ.isError && customers.length === 0 && (
No per-customer data for this query — it isn’t filtered by the tenant column, or no literal-bearing sample was captured. diff --git a/src/components/tabs/Performance/WorkloadAnalysisPanel.js b/src/components/tabs/Performance/WorkloadAnalysisPanel.js index 16f500d..31c799b 100644 --- a/src/components/tabs/Performance/WorkloadAnalysisPanel.js +++ b/src/components/tabs/Performance/WorkloadAnalysisPanel.js @@ -28,6 +28,7 @@ import { } from "lucide-react"; import { workloadAnalysisAPI } from "@/lib/api/client"; import { HelpTooltip } from "@/components/tabs/Brain/components/HelpTooltip"; +import { QueryError } from "./components"; import styles from "./WorkloadAnalysisPanel.module.css"; const fmtMs = (v) => { @@ -194,8 +195,17 @@ export default function WorkloadAnalysisPanel({ connectionId }) {
)} + {/* load failure — distinct from "never run", which looks identical otherwise */} + {latestQ.isError && ( + + )} + {/* empty state — never run */} - {!latest && !latestQ.isLoading && ( + {!latest && !latestQ.isLoading && !latestQ.isError && (

No workload analysis yet

diff --git a/src/components/tabs/Performance/components/QueryError.js b/src/components/tabs/Performance/components/QueryError.js new file mode 100644 index 0000000..b2878fc --- /dev/null +++ b/src/components/tabs/Performance/components/QueryError.js @@ -0,0 +1,47 @@ +import { AlertTriangle } from "lucide-react"; + +/** + * The error state for an async panel in the Performance tab. + * + *

Exists because every panel here rendered a failed fetch and an empty result + * identically: the branch was `!isLoading && rows.length === 0`, and `data ?? []` turns + * any error into an empty array. Clicking a customer whose row said "1 slow query · 12 + * executions" produced "No queries rolled up yet for this customer" when the request had + * actually 404'd — the data existed and the UI reported absence. + * + *

That matters most for 403s: with connection authorization now enforced, a user + * without access to a connection gets a denial, and rendering it as "nothing captured + * yet" would send them to re-run an ingestion they cannot fix. + */ +export function QueryError({ error, what = "data", className }) { + // `error.status`, not `error.response.status`: the axios response interceptor in + // api/client.js rethrows a plain Error with the status copied onto it. Reading the + // axios-shaped field silently never matches, so every denial fell through to the + // generic branch. `responseData` is where the interceptor puts the parsed body. + const status = error?.status ?? error?.response?.status; + + let message; + if (status === 403) { + message = `You don't have access to this connection's ${what}.`; + } else if (status === 404) { + message = `Not found — the ${what} may have been removed, or belong to another connection.`; + } else if (status === 412) { + message = `Not configured for this connection yet.`; + } else { + // Prefer the server's own wording; the axios interceptor puts it on `message`. + message = error?.responseData?.message + || error?.response?.data?.message + || error?.message + || `Could not load ${what}.`; + } + + return ( +

+
+ ); +} + +export default QueryError; diff --git a/src/components/tabs/Performance/components/index.js b/src/components/tabs/Performance/components/index.js index f16e9db..58ed632 100644 --- a/src/components/tabs/Performance/components/index.js +++ b/src/components/tabs/Performance/components/index.js @@ -4,3 +4,4 @@ export { default as TableHeatmap } from './TableHeatmap' export { default as InsightsList } from './InsightsList' export { default as QueryDetailDialog } from './QueryDetailDialog' export { default as PerformanceActionCard } from './PerformanceActionCard' +export { default as QueryError } from './QueryError' diff --git a/src/lib/api/client.js b/src/lib/api/client.js index d98c513..b859c9f 100644 --- a/src/lib/api/client.js +++ b/src/lib/api/client.js @@ -3878,54 +3878,62 @@ export const slackDigestAPI = { export const slowQueryAnalyticsAPI = { getQueries: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/queries`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/queries`) .then((r) => r.data), getTimeline: (connectionId, fingerprint) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/timeline/${fingerprint}`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/timeline/${encodeURIComponent(fingerprint)}`) .then((r) => r.data), getRegressions: (connectionId, minFactor = 1.5) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/regressions`, { + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/regressions`, { params: { minFactor }, }) .then((r) => r.data), getCustomers: (connectionId, fingerprint, day = null) => apiClient .get( - `/api/slow-query-analytics/${connectionId}/query/${fingerprint}/customers`, + `/api/slow-query-analytics/${encodeURIComponent(connectionId)}/query/${encodeURIComponent(fingerprint)}/customers`, { params: day ? { day } : {} }, ) .then((r) => r.data), getSamples: (connectionId, fingerprint) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/query/${fingerprint}/samples`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/query/${encodeURIComponent(fingerprint)}/samples`) .then((r) => r.data), listCustomers: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/customers`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/customers`) .then((r) => r.data), + // customerId is a literal value from the tenant column, so it is application data and + // may contain "/", "?" or "#". It cannot go in a path segment: raw, the slash splits + // the path; percent-encoded, Jetty answers 400 "Ambiguous URI path separator". Both + // were reproduced with the real tenant value `acct/77?x=1`, whose rows silently showed + // as "no queries rolled up yet". axios encodes `params` for us. getCustomerQueries: (connectionId, customerId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/customer/${customerId}/queries`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/customer-queries`, { + params: { customerId }, + }) .then((r) => r.data), getCustomerQuerySamples: (connectionId, customerId, fingerprint) => apiClient .get( - `/api/slow-query-analytics/${connectionId}/customer/${customerId}/query/${fingerprint}/samples`, + `/api/slow-query-analytics/${encodeURIComponent(connectionId)}/customer-query-samples`, + { params: { customerId, fingerprint } }, ) .then((r) => r.data), getTenantColumnSuggestions: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/tenant-column-suggestions`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/tenant-column-suggestions`) .then((r) => r.data), getConfig: (connectionId) => apiClient - .get(`/api/slow-query-analytics/${connectionId}/config`) + .get(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/config`) .then((r) => r.data), putConfig: (connectionId, body) => apiClient - .put(`/api/slow-query-analytics/${connectionId}/config`, body) + .put(`/api/slow-query-analytics/${encodeURIComponent(connectionId)}/config`, body) .then((r) => r.data), analyzeNow: (connectionId) => apiClient diff --git a/src/lib/queryClient.js b/src/lib/queryClient.js index a08582e..9506071 100644 --- a/src/lib/queryClient.js +++ b/src/lib/queryClient.js @@ -1,5 +1,12 @@ import { QueryClient } from '@tanstack/react-query' +/** + * Statuses that will never succeed on retry: the request was understood and refused. + * 401 is excluded on purpose — the axios interceptor refreshes the token and a retry + * genuinely can succeed after it. + */ +const NON_RETRYABLE_STATUSES = new Set([400, 403, 404, 405, 409, 412, 422]) + /** * React Query client configuration * Used for caching and retry logic across the app @@ -9,7 +16,23 @@ export const queryClient = new QueryClient({ queries: { staleTime: 5 * 60 * 1000, // Data is fresh for 5 minutes gcTime: 10 * 60 * 1000, // Garbage collect after 10 minutes (v5: renamed from cacheTime) - retry: 3, // Retry failed requests 3 times + // Retry transient failures only. A 403/404 is a settled answer, and retrying + // it costs three extra round trips plus ~7s of backoff (1s+2s+4s) before the + // UI can show anything — observed as four identical requests in the console + // for one denied call. Some of these are expensive on the server too: an + // unauthorized /tenant-column-suggestions opens a fresh JDBC connection to + // the target database on every attempt. + retry: (failureCount, error) => { + // `error.status`, not `error.response.status`: the axios response + // interceptor in api/client.js rethrows a plain Error with the status + // copied onto it, so `response` is gone by the time react-query sees it. + // Reading the axios-shaped field looks right and silently never matches — + // the denied request still made four attempts over ~7s. `response.status` + // is kept as a fallback for any caller that bypasses the interceptor. + const status = error?.status ?? error?.response?.status + if (NON_RETRYABLE_STATUSES.has(status)) return false + return failureCount < 3 + }, retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000), // Exponential backoff refetchOnWindowFocus: false, // Don't refetch on window focus refetchOnReconnect: true, // Refetch on reconnect