fix: add componentUid and projectUid filters for alert rule SQL and update required fields in request body

[nilushancosta]

Purpose

openchoreo/openchoreo#2859

Log-based alerts created in the observability-logs-openobserve module match log entries from any component and environment because the generated OpenObserve SQL query only filters by the search pattern, without scoping to a specific environment or component.

Goals

• Scope alert SQL queries by filtering on environmentUid and componentUid in the WHERE clause.
• Enforce all alert request fields as mandatory at the API spec level.
• Remove the inapplicable metric field from the log-based alert request schema.

Approach

• Regenerated Go types via make openapi-codegen, making all fields value types with automatic rejection of incomplete requests.
• Updated generateAlertConfig to include kubernetes_labels_openchoreo_dev_environment_uid and kubernetes_labels_openchoreo_dev_component_uid filters in the SQL WHERE clause.
• Simplified handlers.go by removing manual nil-check validation and pointer dereferencing, since the generated code now enforces required fields.
• Updated tests to match the new non-pointer types and added assertions verifying the UID filters in the generated SQL.

Summary by CodeRabbit

• Release

  • Version 0.4.1 now available for the observability logs component.

• Improvements

  • Enhanced log alert filtering to better isolate and scope alerts to specific environment and component contexts, improving alert accuracy and precision.

[coderabbitai]

📝 Walkthrough

Walkthrough

The chart version is bumped from 0.4.0 to 0.4.1 across metadata and documentation. Alert parameter construction in handlers is refactored to use struct literal syntax, and SQL alert queries are enhanced with environment and component UID filters.

Changes

Cohort / File(s)	Summary
Version Updates observability-logs-openobserve/README.md, observability-logs-openobserve/helm/Chart.yaml	Chart version and appVersion bumped from 0.4.0 to 0.4.1; README Helm install/upgrade commands updated accordingly.
Alert Parameter Refactoring observability-logs-openobserve/internal/handlers.go, observability-logs-openobserve/internal/handlers_test.go	toLogAlertParams refactored to use struct literal construction instead of conditional field assignments; nil checks removed. Test payloads updated to use non-pointer JSON fields instead of pointer-wrapped values.
Query Generation Enhancement observability-logs-openobserve/internal/openobserve/queries.go, observability-logs-openobserve/internal/openobserve/queries_test.go	generateAlertConfig SQL query now filters by kubernetes_labels_openchoreo_dev_environment_uid and kubernetes_labels_openchoreo_dev_component_uid in addition to existing search pattern matching. Tests updated to assert these UID filters are present in generated SQL.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• feat: add webhook API and GET and UPDATE APIs for alert rules in OpenObserve #31: Modifies toLogAlertParams and generateAlertConfig in the same alert plumbing code paths.
• fix: update SQL query in OpenObserve alert rule creation #48: Updates SQL generation logic in generateAlertConfig within the same queries module.
• test: add unit tests for observability modules #53: Adds comprehensive tests for alert handlers and query generation targeting the same code being modified.

Suggested reviewers

• LakshanSS
• akila-i

Poem

🐰 A hop, a skip, the version bumps with glee,
Alert parameters dance, now struct-ly free!
SQL filters refine, environment and component bound,
In OpenObserve's garden, cleaner queries are found! 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: adding componentUid and projectUid filters to alert rule SQL and updating required fields in the request body, which aligns with the core objectives of scoping queries and enforcing mandatory fields.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

🧹 Nitpick comments (2)

observability-logs-openobserve/internal/handlers_test.go (2)

751-760: Consider populating UIDs in update success test.

Similar to the create test, TestUpdateAlertRule_Success doesn't set any UID fields. For consistency with the "all fields required" contract, consider populating ProjectUid, EnvironmentUid, and ComponentUid.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@observability-logs-openobserve/internal/handlers_test.go` around lines 751 -
760, The TestUpdateAlertRule_Success test's Metadata literal omits UID fields;
update the Metadata struct in TestUpdateAlertRule_Success to populate
ProjectUid, EnvironmentUid, and ComponentUid with valid openapi_types.UUID
values (matching the create test pattern) so the "all fields required" contract
is satisfied; locate the Metadata block within TestUpdateAlertRule_Success and
assign non-empty UUIDs to ProjectUid, EnvironmentUid, and ComponentUid.

---

559-575: ProjectUid not set in success test.

In TestCreateAlertRule_Success, envUID and compUID are correctly initialized and assigned, but ProjectUid is left as a zero-value UUID. While this works functionally, consider setting ProjectUid for consistency with real-world usage where all fields are required:

+	projUID, _ := parseUUID("550e8400-e29b-41d4-a716-446655440000")
 	envUID, _ := parseUUID("550e8400-e29b-41d4-a716-446655440001")
 	compUID, _ := parseUUID("550e8400-e29b-41d4-a716-446655440002")
 ...
 			Name:           "test-alert",
 			Namespace:      "ns-1",
+			ProjectUid:     projUID,
 			EnvironmentUid: envUID,
 			ComponentUid:   compUID,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@observability-logs-openobserve/internal/handlers_test.go` around lines 559 -
575, The test TestCreateAlertRule_Success leaves ProjectUid as a zero UUID;
update the test to initialize a project UID (e.g., call parseUUID like
envUID/compUID) and assign it to the ProjectUid field in the AlertRuleRequest
metadata so metadata.ProjectUid is a realistic non-zero value when calling
handler.CreateAlertRule (look for parseUUID usage and the
AlertRuleRequest.Metadata struct in this test).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@observability-logs-openobserve/internal/handlers_test.go`:
- Around line 751-760: The TestUpdateAlertRule_Success test's Metadata literal
omits UID fields; update the Metadata struct in TestUpdateAlertRule_Success to
populate ProjectUid, EnvironmentUid, and ComponentUid with valid
openapi_types.UUID values (matching the create test pattern) so the "all fields
required" contract is satisfied; locate the Metadata block within
TestUpdateAlertRule_Success and assign non-empty UUIDs to ProjectUid,
EnvironmentUid, and ComponentUid.
- Around line 559-575: The test TestCreateAlertRule_Success leaves ProjectUid as
a zero UUID; update the test to initialize a project UID (e.g., call parseUUID
like envUID/compUID) and assign it to the ProjectUid field in the
AlertRuleRequest metadata so metadata.ProjectUid is a realistic non-zero value
when calling handler.CreateAlertRule (look for parseUUID usage and the
AlertRuleRequest.Metadata struct in this test).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2860e735-f5a4-4244-8a16-f3d932547851

📥 Commits

Reviewing files that changed from the base of the PR and between 52bb0e7 and d49fe79.

⛔ Files ignored due to path filters (2)

• observability-logs-openobserve/internal/api/gen/models.gen.go is excluded by !**/gen/**
• observability-logs-openobserve/internal/api/gen/server.gen.go is excluded by !**/gen/**

📒 Files selected for processing (6)

• observability-logs-openobserve/README.md
• observability-logs-openobserve/helm/Chart.yaml
• observability-logs-openobserve/internal/handlers.go
• observability-logs-openobserve/internal/handlers_test.go
• observability-logs-openobserve/internal/openobserve/queries.go
• observability-logs-openobserve/internal/openobserve/queries_test.go