Skip to content

better error message for using invalid field in agg#16413

Closed
bmcconaghy wants to merge 2 commits intoelastic:masterfrom
bmcconaghy:use_raw_keyword_field_when_exists
Closed

better error message for using invalid field in agg#16413
bmcconaghy wants to merge 2 commits intoelastic:masterfrom
bmcconaghy:use_raw_keyword_field_when_exists

Conversation

@bmcconaghy
Copy link
Copy Markdown
Contributor

@bmcconaghy bmcconaghy commented Jan 30, 2018

This addresses: #9869

It presents an improved error message to the user. Multifield detection needs to be addressed in a separate PR.

…st throwing an error. also improved error message.
@bmcconaghy
Copy link
Copy Markdown
Contributor Author

Maybe this should also display a warning if we do go with the .raw field so the user can go fix the issue in the query (and also get a little knowledge).

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

if (!validField) {
notifier.error(`Saved "field" parameter is now invalid. Please select a new field.`);
// check to see if a .raw field exists and use it if it does
const validRawField = this.getFieldOptions(aggConfig).byName[`${fieldName}.raw`];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work for newer fields using the .keyword convention. More generally, these names are just a convention, users can name their keyword multi fields anything they want. It would be nice if we interrogated the mappings and actually detected when a multi field exists instead of relying on names. Of course, that would blow this tiny change up into big change. We've needed knowledge about multi fields for a long time though :) At the very least, we should check for .keyword in addition to .raw.

// check to see if a .raw field exists and use it if it does
const validRawField = this.getFieldOptions(aggConfig).byName[`${fieldName}.raw`];
if (validRawField) {
return validRawField;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably display an "info" notification when we change the field, so the user is aware of what's going on. It's possible they're intentionally trying to aggregate on a text field and just forgot to turn field data on, so without a notification that the field changed they may never realize they're not getting what they want.

@bmcconaghy
Copy link
Copy Markdown
Contributor Author

I removed the field swapping logic from this. Now it is just a better error message. We can tackle multifields as part of a bigger (and better) change.

@bmcconaghy bmcconaghy changed the title using .raw keyword field for aggregation when it exists instead of ju… better error message for using invalid field in agg Jan 31, 2018
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Copy link
Copy Markdown
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sebelga
Copy link
Copy Markdown
Contributor

sebelga commented Oct 10, 2018

Checked in with @bmcconaghy and decided to close this PR

@sebelga sebelga closed this Oct 10, 2018
patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request Mar 20, 2026
…tion scripts

Tested all 10 v2.0 enhancements on Alert Investigation Pipeline spike:

**GitHub Issues Created (with Elastic-specific context):**
- elastic#16410 - GraphRAG Attack Path Prediction (HIGH priority, 5-7d)
  → What we have: ES Graph API, entity extraction, Agent Builder
  → What's missing: Graph schema, MITRE KB, traversal algorithms
  → Feasibility: 90% (ES graphs vs Neo4j trade-off documented)

- elastic#16411 - RLHF Continuous Learning Pipeline (MEDIUM, 5-7d)
  → What we have: LangSmith, ES storage, feedback UI
  → What's missing: Training pipeline, A/B framework
  → Feasibility: 85% (Elasticsearch aggregations advantage)

- elastic#16412 - NL to ES|QL Query Generator (MEDIUM, 2-3d)
  → What we have: ES|QL (GA), schema introspection, Claude API
  → What's missing: Schema-aware prompts, validator
  → Feasibility: 90% (ES|QL simpler than Query DSL)

- elastic#16413 - AI Interviewer / User Context (MEDIUM, 3-4d)
  → What we have: Slack connector, Cases API, Agent Builder
  → What's missing: User lookup (AD), consent management
  → Feasibility: 70% (privacy/compliance considerations)

- elastic#16414 - Proactive Autonomous Threat Hunter (ROADMAP, 5-7d)
  → What we have: ES ML, Detection Engine, unified data access
  → What's missing: Hunting hypotheses library, cross-index orchestration
  → Feasibility: 85% (Elastic's unified data is key advantage)

**Master Dependency Graph:**
- Posted to spike issue elastic#16339 with Mermaid visualization
- Shows build order: Foundation → Infrastructure → Applications → Advanced
- Color-coded by priority (Red=HIGH, Blue/Yellow=MEDIUM, Gray=ROADMAP)
- Effort estimates: 25-35 eng-days across 12 months

**Automation Scripts Created:**
- capture_spike_screenshots.sh (Playwright-based, 8 screenshots + video)
- Autonomous Kibana startup if needed
- Professional resolution (1920x1080)
- Screenshot manifest auto-generation

**v2.0 Validation Results:**
- ✅ 10/13 success criteria met (77%)
- ✅ Issue creation: WORKS (5 issues with full Elastic context)
- ✅ Dependency graphs: WORKS (beautiful Mermaid visualizations)
- ✅ Market analysis: WORKS (urgency 8.7, 12-18mo window)
- ⚠️ Screenshots: READY (script created, awaiting execution)
- ❌ Feature flag: MISSING (critical gap discovered)

**Gaps Identified:**
1. CRITICAL: Add feature flag before merge (30 min effort)
2. OPTIONAL: Execute screenshot capture (5 min when demo-ready)
3. OPTIONAL: Add competitive benchmark tests (2-3h if needed)

spike-builder v2.0 validated as production-ready with significant value add.

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request Mar 27, 2026
…tion scripts

Tested all 10 v2.0 enhancements on Alert Investigation Pipeline spike:

**GitHub Issues Created (with Elastic-specific context):**
- elastic#16410 - GraphRAG Attack Path Prediction (HIGH priority, 5-7d)
  → What we have: ES Graph API, entity extraction, Agent Builder
  → What's missing: Graph schema, MITRE KB, traversal algorithms
  → Feasibility: 90% (ES graphs vs Neo4j trade-off documented)

- elastic#16411 - RLHF Continuous Learning Pipeline (MEDIUM, 5-7d)
  → What we have: LangSmith, ES storage, feedback UI
  → What's missing: Training pipeline, A/B framework
  → Feasibility: 85% (Elasticsearch aggregations advantage)

- elastic#16412 - NL to ES|QL Query Generator (MEDIUM, 2-3d)
  → What we have: ES|QL (GA), schema introspection, Claude API
  → What's missing: Schema-aware prompts, validator
  → Feasibility: 90% (ES|QL simpler than Query DSL)

- elastic#16413 - AI Interviewer / User Context (MEDIUM, 3-4d)
  → What we have: Slack connector, Cases API, Agent Builder
  → What's missing: User lookup (AD), consent management
  → Feasibility: 70% (privacy/compliance considerations)

- elastic#16414 - Proactive Autonomous Threat Hunter (ROADMAP, 5-7d)
  → What we have: ES ML, Detection Engine, unified data access
  → What's missing: Hunting hypotheses library, cross-index orchestration
  → Feasibility: 85% (Elastic's unified data is key advantage)

**Master Dependency Graph:**
- Posted to spike issue elastic#16339 with Mermaid visualization
- Shows build order: Foundation → Infrastructure → Applications → Advanced
- Color-coded by priority (Red=HIGH, Blue/Yellow=MEDIUM, Gray=ROADMAP)
- Effort estimates: 25-35 eng-days across 12 months

**Automation Scripts Created:**
- capture_spike_screenshots.sh (Playwright-based, 8 screenshots + video)
- Autonomous Kibana startup if needed
- Professional resolution (1920x1080)
- Screenshot manifest auto-generation

**v2.0 Validation Results:**
- ✅ 10/13 success criteria met (77%)
- ✅ Issue creation: WORKS (5 issues with full Elastic context)
- ✅ Dependency graphs: WORKS (beautiful Mermaid visualizations)
- ✅ Market analysis: WORKS (urgency 8.7, 12-18mo window)
- ⚠️ Screenshots: READY (script created, awaiting execution)
- ❌ Feature flag: MISSING (critical gap discovered)

**Gaps Identified:**
1. CRITICAL: Add feature flag before merge (30 min effort)
2. OPTIONAL: Execute screenshot capture (5 min when demo-ready)
3. OPTIONAL: Add competitive benchmark tests (2-3h if needed)

spike-builder v2.0 validated as production-ready with significant value add.

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request Mar 30, 2026
…tion scripts

Tested all 10 v2.0 enhancements on Alert Investigation Pipeline spike:

**GitHub Issues Created (with Elastic-specific context):**
- elastic#16410 - GraphRAG Attack Path Prediction (HIGH priority, 5-7d)
  → What we have: ES Graph API, entity extraction, Agent Builder
  → What's missing: Graph schema, MITRE KB, traversal algorithms
  → Feasibility: 90% (ES graphs vs Neo4j trade-off documented)

- elastic#16411 - RLHF Continuous Learning Pipeline (MEDIUM, 5-7d)
  → What we have: LangSmith, ES storage, feedback UI
  → What's missing: Training pipeline, A/B framework
  → Feasibility: 85% (Elasticsearch aggregations advantage)

- elastic#16412 - NL to ES|QL Query Generator (MEDIUM, 2-3d)
  → What we have: ES|QL (GA), schema introspection, Claude API
  → What's missing: Schema-aware prompts, validator
  → Feasibility: 90% (ES|QL simpler than Query DSL)

- elastic#16413 - AI Interviewer / User Context (MEDIUM, 3-4d)
  → What we have: Slack connector, Cases API, Agent Builder
  → What's missing: User lookup (AD), consent management
  → Feasibility: 70% (privacy/compliance considerations)

- elastic#16414 - Proactive Autonomous Threat Hunter (ROADMAP, 5-7d)
  → What we have: ES ML, Detection Engine, unified data access
  → What's missing: Hunting hypotheses library, cross-index orchestration
  → Feasibility: 85% (Elastic's unified data is key advantage)

**Master Dependency Graph:**
- Posted to spike issue elastic#16339 with Mermaid visualization
- Shows build order: Foundation → Infrastructure → Applications → Advanced
- Color-coded by priority (Red=HIGH, Blue/Yellow=MEDIUM, Gray=ROADMAP)
- Effort estimates: 25-35 eng-days across 12 months

**Automation Scripts Created:**
- capture_spike_screenshots.sh (Playwright-based, 8 screenshots + video)
- Autonomous Kibana startup if needed
- Professional resolution (1920x1080)
- Screenshot manifest auto-generation

**v2.0 Validation Results:**
- ✅ 10/13 success criteria met (77%)
- ✅ Issue creation: WORKS (5 issues with full Elastic context)
- ✅ Dependency graphs: WORKS (beautiful Mermaid visualizations)
- ✅ Market analysis: WORKS (urgency 8.7, 12-18mo window)
- ⚠️ Screenshots: READY (script created, awaiting execution)
- ❌ Feature flag: MISSING (critical gap discovered)

**Gaps Identified:**
1. CRITICAL: Add feature flag before merge (30 min effort)
2. OPTIONAL: Execute screenshot capture (5 min when demo-ready)
3. OPTIONAL: Add competitive benchmark tests (2-3h if needed)

spike-builder v2.0 validated as production-ready with significant value add.

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants