feat(peek): add Perses ES|QL datasource foundation - #590
Conversation
Introduce a Perses-oriented datasource abstraction and route shared query paths through it for migration groundwork. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/ai let's make sure we've fully implemented phase one and then let's work on phase two: Phase 1 — ES|QL custom datasource plugin (foundation) |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces direct Elasticsearch client usage with a Perses ESQL datasource and request builder (createPersesEsqlDatasource, buildPersesEsqlRequest) and switches Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
/ai let's make sure we've fully implemented phase one and then let's work on phase two: Phase 1 — ES|QL custom datasource plugin (foundation) |
|
/ai let's make sure we've fully implemented phase one and then let's implement phase two: |
There was a problem hiding this comment.
Approved: I found no new actionable issues in this PR.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Implemented the requested follow-up by hardening phase 1 and wiring the first phase-2 runtime seam on this PR branch. What changed
Validation
Also resolved the existing CodeRabbit review thread after applying the global unstub fix. What is this? | From workflow: Mention in PR (no sandbox) Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/ai please fix merge conflicts |
1 similar comment
|
/ai please fix merge conflicts |
|
I investigated and attempted to fix the merge conflicts, but I’m blocked by a push limitation in this workflow.
To proceed, a maintainer will need to perform the conflict resolution locally (or with force-push permissions/tooling that allows rewritten history) and then push the updated PR branch. What is this? | From workflow: Mention in PR (no sandbox) Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
…451b26 Resolve merge conflicts in PanelContainer.tsx, PanelEditor.tsx, and useMarkdownEsql.ts by keeping the Perses datasource abstraction layer (createPersesEsqlDatasource / buildPersesEsqlRequest) over the direct ElasticsearchClient / buildEsqlRequest usage from main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
peek/tests/component/PanelContainer.test.tsx (1)
28-39: 🧹 Nitpick | 🔵 TrivialMinor:
onExportCsvprop declared but unused.The mock's type includes
onExportCsvbut the implementation doesn't use it. If PanelContainer no longer passes this prop, remove it; otherwise, keep for interface parity.🧹 If the prop is obsolete
function MockVisualization({ onExportReady, }: { onExportReady?: (exportFn: (() => string) | null) => void; - onExportCsv?: () => void; }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@peek/tests/component/PanelContainer.test.tsx` around lines 28 - 39, The MockVisualization component declares an unused prop onExportCsv; either remove it from the prop type/signature or use it if PanelContainer still passes it. Update the MockVisualization function signature to only accept onExportReady (remove onExportCsv from the typed props) or, if interface parity is required, keep onExportCsv and thread it through the mock body (e.g., call or forward it) so the prop is actually used; adjust the prop type near MockVisualization and any tests that instantiate it accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@peek/tests/component/PanelContainer.test.tsx`:
- Around line 28-39: The MockVisualization component declares an unused prop
onExportCsv; either remove it from the prop type/signature or use it if
PanelContainer still passes it. Update the MockVisualization function signature
to only accept onExportReady (remove onExportCsv from the typed props) or, if
interface parity is required, keep onExportCsv and thread it through the mock
body (e.g., call or forward it) so the prop is actually used; adjust the prop
type near MockVisualization and any tests that instantiate it accordingly.
ℹ️ Review info
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
peek/src/components/ParameterBar.tsxpeek/src/hooks/useEsqlQuery.tspeek/tests/component/PanelContainer.test.tsxpeek/tests/unit/useFleetStore.test.ts
| return queryText; | ||
| } | ||
| const values = new Map(variables.map((variable) => [variable.name, String(variable.value)])); | ||
| return queryText.replace(/\{\{(\w+)\}\}/g, (token, name: string) => values.get(name) ?? token); |
There was a problem hiding this comment.
[MEDIUM] Escape interpolated \{\{var}} string values for ES|QL literals
interpolatePersesVariableTokens() inserts raw variable text into query strings. When a dashboard value contains a single quote (for example O'Reilly) and the query uses the current documented pattern from this PR (env == '\{\{env}}'), the generated ES|QL becomes invalid (env == 'O'Reilly').
Please escape string-like values (at least single quotes) before substitution, or convert \{\{name}} usage into bound ?name parameters so query semantics are preserved for arbitrary user input.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a dashboard variable value contains a single quote (e.g. O'Reilly), interpolating it directly into an ES|QL query string literal produces invalid syntax. This doubles single quotes in replacement values, which is the standard ES|QL escape sequence for a literal single quote inside a string. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
createPersesEsqlDatasourceabstraction atpeek/src/services/perses/esqlDatasource.tspeek/src/hooks/useEsqlQuery.tspeek/src/components/PanelContainer.tsxpeek/src/components/perses/panelRegistry.tspeek/src/components/perses/PersesPanelRenderer.tsxpeek/src/components/DashboardGrid.tsxpeek/src/components/perses/layoutAdapter.tspeek/tests/unit/persesEsqlDatasource.test.tspeek/tests/unit/persesPanelRegistry.test.tspeek/tests/unit/persesLayoutAdapter.test.tsWhy
This creates concrete migration seams for the Perses dashboard cutover: query execution now has a Perses-oriented datasource boundary, and panel/layout runtime wiring can evolve behind Perses-specific adapters without reworking each component call-site.
Validation
make lintmake test-unitmake buildFixes Switch to Perses #589