Skip to content

Conversation

@cbcoutinho
Copy link

Description

Adds validation for PromQL and LogQL queries before execution to provide early feedback on syntax errors and reduce wasted API calls.

Fixes #430

Changes

Core Implementation

  • tools/query_validation.go: New validation functions using official parsers
    • ValidatePromQL(): Uses github.com/prometheus/prometheus/promql/parser
    • ValidateLogQL(): Uses github.com/grafana/loki/v3/pkg/logql/syntax

Integration

  • tools/prometheus.go: Added inline validation to query_prometheus tool
  • tools/loki.go: Added inline validation to query_loki_logs tool

Tests

  • tools/query_validation_unit_test.go: Comprehensive unit tests
    • 11 test cases for PromQL validation
    • 14 test cases for LogQL validation
    • Coverage for valid queries, invalid syntax, and edge cases

Dependencies

  • Added github.com/grafana/loki/v3 for LogQL parsing
  • Already had github.com/prometheus/prometheus for PromQL parsing

Benefits

  1. Early error detection: Syntax errors caught before API calls
  2. Clear error messages: Parser errors are more specific than API errors
  3. Reduced iteration: LLMs can fix syntax errors in one iteration instead of multiple back-and-forth
  4. Better UX: Faster feedback loop for query development
  5. Production-grade parsers: Uses the same parsers as Prometheus and Loki

Testing

All tests pass:

=== RUN   TestValidatePromQL
--- PASS: TestValidatePromQL (0.00s)

=== RUN   TestValidateLogQL
--- PASS: TestValidateLogQL (0.00s)

Example Error Messages

Invalid PromQL:

invalid PromQL syntax: 1:38: parse error: unclosed left parenthesis

Invalid LogQL:

invalid LogQL syntax: syntax error: unexpected $end

Design Decisions

Inline Validation vs Separate Tools

  • Chosen: Inline validation in existing query tools
  • Rationale: Simpler, provides immediate feedback, no additional tool calls needed
  • Alternative considered: Separate validate_promql and validate_logql tools

Parser Selection

  • PromQL: github.com/prometheus/prometheus/promql/parser - official Prometheus parser
  • LogQL: github.com/grafana/loki/v3/pkg/logql/syntax - official Loki parser
  • Rationale: Production-grade, well-tested, matches server-side validation

Migration Notes

This is a non-breaking change:

  • Existing valid queries continue to work unchanged
  • Invalid queries that previously failed on the server now fail earlier with better error messages
  • No changes to tool interfaces or parameters

Related Issues

Addresses the common problem where LLMs generate invalid queries and need multiple iterations to fix syntax errors. This reduces context usage and improves the overall query generation workflow.


This PR was generated with the help of AI, and reviewed by a Human

This commit adds validation for PromQL and LogQL queries before execution
to provide early feedback on syntax errors and reduce wasted API calls.

Changes:
- Add ValidatePromQL() using Prometheus's official parser
- Add ValidateLogQL() using Loki's official syntax parser
- Integrate inline validation into query_prometheus tool
- Integrate inline validation into query_loki_logs tool
- Add comprehensive unit tests for both validators
- Add github.com/grafana/loki/v3 dependency for LogQL parsing

Benefits:
- Catches syntax errors before sending to datasource
- Provides clear error messages for debugging
- Reduces iteration cycles for LLM-generated queries
- Uses production-grade parsers from official projects

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@cbcoutinho cbcoutinho requested a review from a team as a code owner December 17, 2025 02:34
Copy link
Collaborator

@sd2k sd2k left a comment

Choose a reason for hiding this comment

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

Hi @cbcoutinho! Thanks for the contribution, the code looks solid.

The main reason we haven't added this before is because it adds a rather large dependency on Loki just for the parsing, and another 11MB to the final binary, which felt a bit overkill; ideally we would be getting decent error messages back from Loki/Prometheus anyway if the query was invalid. Did you find that wasn't the case? Were we returning poor error messages for invalid queries?

@amansingh207
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

430 - Partially compliant

Compliant requirements:

  • Added ValidatePromQL() using Prometheus parser.
  • Added ValidateLogQL() using Loki parser.
  • Integrated validation inline in query_prometheus and query_loki_logs.
  • Clear error messages returned on syntax errors.
  • Early error detection to reduce wasted API calls.
  • Comprehensive unit tests with multiple cases for both PromQL and LogQL.
  • Added github.com/grafana/loki/v3 dependency.

Non-compliant requirements:

  • None identified.

Requires further human verification:

  • Verify runtime behavior and error messages in actual query execution environments.
  • Confirm no performance regressions due to added validation.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Validation correctness

Ensure that the validation functions correctly identify all invalid PromQL and LogQL syntax cases without false positives or negatives.
Confirm that error messages are clear and helpful for users.

func ValidatePromQL(expr string) error {
	if expr == "" {
		return fmt.Errorf("invalid PromQL syntax: expression cannot be empty")
	}

	// Use the Prometheus parser to validate the expression
	_, err := parser.ParseExpr(expr)
	if err != nil {
		return fmt.Errorf("invalid PromQL syntax: %w", err)
	}

	return nil
}

// ValidateLogQL validates a LogQL expression using the Loki parser
// Returns nil if the expression is valid, or an error describing the syntax issue
func ValidateLogQL(expr string) error {
	if expr == "" {
		return fmt.Errorf("invalid LogQL syntax: expression cannot be empty")
	}

	// Use the Loki parser to validate the expression
	// ParseExpr validates both log selector and metric query syntax
	_, err := syntax.ParseExpr(expr)
	if err != nil {
		return fmt.Errorf("invalid LogQL syntax: %w", err)
	}

	return nil
}
Test coverage and edge cases

Review the comprehensiveness of test cases for both PromQL and LogQL validation, including edge cases and error scenarios.
Validate that tests properly assert error messages and success cases.

func TestValidatePromQL(t *testing.T) {
	tests := []struct {
		name    string
		expr    string
		wantErr bool
	}{
		{
			name:    "valid simple metric query",
			expr:    "up",
			wantErr: false,
		},
		{
			name:    "valid metric with label matcher",
			expr:    `up{job="prometheus"}`,
			wantErr: false,
		},
		{
			name:    "valid rate query",
			expr:    `rate(http_requests_total[5m])`,
			wantErr: false,
		},
		{
			name:    "valid aggregation query",
			expr:    `sum by(job) (rate(http_requests_total[5m]))`,
			wantErr: false,
		},
		{
			name:    "valid binary operation",
			expr:    `up == 1`,
			wantErr: false,
		},
		{
			name:    "empty expression",
			expr:    "",
			wantErr: true,
		},
		{
			name:    "invalid syntax - unclosed bracket",
			expr:    `rate(http_requests_total[5m]`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - unclosed label matcher",
			expr:    `up{job="prometheus"`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - unknown function",
			expr:    `unknown_function(up)`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - malformed label matcher",
			expr:    `up{job=}`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - unexpected character",
			expr:    `up @`,
			wantErr: true,
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			err := ValidatePromQL(tt.expr)
			if tt.wantErr {
				assert.Error(t, err, "Expected error for expression: %s", tt.expr)
				assert.Contains(t, err.Error(), "PromQL", "Error message should mention PromQL")
			} else {
				require.NoError(t, err, "Expected no error for valid expression: %s", tt.expr)
			}
		})
	}
}

func TestValidateLogQL(t *testing.T) {
	tests := []struct {
		name    string
		expr    string
		wantErr bool
	}{
		{
			name:    "valid simple log selector",
			expr:    `{app="foo"}`,
			wantErr: false,
		},
		{
			name:    "valid log selector with multiple labels",
			expr:    `{app="foo", env="prod"}`,
			wantErr: false,
		},
		{
			name:    "valid log selector with filter",
			expr:    `{app="foo"} |= "error"`,
			wantErr: false,
		},
		{
			name:    "valid log selector with regex filter",
			expr:    `{app="foo"} |~ "error.*"`,
			wantErr: false,
		},
		{
			name:    "valid log selector with json parser",
			expr:    `{app="foo"} | json`,
			wantErr: false,
		},
		{
			name:    "valid metric query with rate",
			expr:    `rate({app="foo"}[5m])`,
			wantErr: false,
		},
		{
			name:    "valid metric query with count_over_time",
			expr:    `count_over_time({app="foo"}[5m])`,
			wantErr: false,
		},
		{
			name:    "valid metric query with sum aggregation",
			expr:    `sum(rate({app="foo"}[5m])) by (host)`,
			wantErr: false,
		},
		{
			name:    "valid complex pipeline",
			expr:    `{app="foo"} | json | line_format "{{.message}}"`,
			wantErr: false,
		},
		{
			name:    "empty expression",
			expr:    "",
			wantErr: true,
		},
		{
			name:    "invalid syntax - unclosed label matcher",
			expr:    `{app="foo"`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - missing equals in label",
			expr:    `{app}`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - invalid filter operator",
			expr:    `{app="foo"} |* "error"`,
			wantErr: true,
		},
		{
			name:    "invalid syntax - malformed rate",
			expr:    `rate({app="foo"})`,
			wantErr: true,
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			err := ValidateLogQL(tt.expr)
			if tt.wantErr {
				assert.Error(t, err, "Expected error for expression: %s", tt.expr)
				assert.Contains(t, err.Error(), "LogQL", "Error message should mention LogQL")
			} else {
				require.NoError(t, err, "Expected no error for valid expression: %s", tt.expr)
			}
		})
	}
}
Integration correctness

Verify that PromQL validation is correctly integrated before query execution and that errors are properly propagated.

func queryPrometheus(ctx context.Context, args QueryPrometheusParams) (model.Value, error) {
	// Validate PromQL syntax before executing
	if err := ValidatePromQL(args.Expr); err != nil {
		return nil, err
	}

@cbcoutinho
Copy link
Author

cbcoutinho commented Dec 19, 2025

Hi @sd2k, you're right that the error messages themselves are almost identical from prometheus/loki; however, this PR makes a few adjustments to that flow:

  • Invalid queries are not routed to the data sources, saving time and potentially reducing latency
  • Current error messages include the http details of the request, this PR unifies them into a single format

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.

Add query validation for PromQL and LogQL

3 participants