PC-18992: Add multi-query support for Sumo Logic metrics#545
PC-18992: Add multi-query support for Sumo Logic metrics#545nikodemrafalski wants to merge 5 commits intomainfrom
Conversation
- Bump nobl9-go to include SumoLogicQuery struct - Add 'queries' ListNestedBlock to sumologic schema (row_id + query) - Deprecate singular 'query' attribute (now Optional) - Update SumoLogicModel with Queries slice and SumoLogicQueryModel - Handle bi-directional conversion in sumoLogicToModel/modelToSumoLogic
Update nobl9-go to include multi-query review fixes.
Null out legacy query field when multi-query is present to prevent perpetual plan drift. Add ConflictsWith validators ensuring query and queries are mutually exclusive, and a custom validator forbidding queries block for logs type. Fix template test helper to render all slice elements and add multi-query golden file test. Regenerate docs.
skrolikiewicz
left a comment
There was a problem hiding this comment.
Review Summary
Overall well-structured PR that follows existing codebase conventions. Some positive highlights:
- Plan drift handling is well implemented —
types.StringNull()onQuerywhenQueriesis populated prevents perpetual drift - Bidirectional
ConflictsWithvalidators are correctly set up on bothqueryandqueries - Custom
sumoLogicQueriesTypeValidatorcleanly preventsqueriesfor logs type - Template rendering fix (all slice elements instead of just first) is safe — existing tests use single-element slices so no breakage
- Model type change (
string→types.String) is correct and doesn't require a schema version bump - ThousandEyes
account_group_idaddition follows established codebase patterns
Left a few suggestions inline — nothing blocking, just ideas to harden validation and improve test coverage.
| Optional: true, | ||
| Description: "Query for the metrics. Deprecated: use 'queries' block instead.", | ||
| DeprecationMessage: "Use 'queries' block instead for multi-query support.", | ||
| Validators: []validator.String{ |
There was a problem hiding this comment.
ConflictsWith ensures mutual exclusivity, but there's no validation that at least one of query or queries is provided. A user could submit a sumologic block with only type set and get a confusing API-level error. Consider adding stringvalidator.AtLeastOneOf(path.MatchRelative().AtParent().AtName("queries")) here.
| Blocks: map[string]schema.Block{ | ||
| "queries": schema.ListNestedBlock{ | ||
| Description: "Multi-query configuration for metrics type (ABC pattern). Each query row has a row ID (A-F) and a query string. The SLI result is taken from the row with the highest letter ID (e.g., if rows A, B, C are defined, the result comes from row C).", | ||
| Validators: []validator.List{ |
There was a problem hiding this comment.
No validation for duplicate row_id values. A user could define two queries both with row_id = "A". Either add a custom validator checking uniqueness, or confirm the SDK catches this and document that decision.
|
|
||
| ### Optional | ||
|
|
||
| > **NOTE**: [Write-only arguments](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) are supported in Terraform 1.11 and later. |
There was a problem hiding this comment.
This removal (write-only arguments note + retrieve_historical_data_from type change) appears unrelated to the Sumo Logic multi-query feature. Consider splitting into a separate commit or calling it out in the PR description.
| return model | ||
| }, | ||
| }, | ||
| "sumologic multi-query": { |
There was a problem hiding this comment.
Consider adding negative test cases: (1) queries with type = "logs" to verify the custom validator, (2) both query and queries empty, (3) both set simultaneously. These are cheap to add and protect against regressions.
|
Verified CRUD operations for both Agent and Direct modes that support the ABC pattern. Additionally, confirmed that unsupported configurations (Agent, Direct, and Logs data types) are handled correctly. |
Summary
Add multi-query (ABC pattern) support for Sumo Logic metrics integration, aligning the Terraform provider with the updated
nobl9-goSDK.Changes
Sumo Logic Multi-Query Support
queriesblock to thesumologicschema, enabling multiple named queries (rows A–F) that can reference each otherqueryattribute in favor of the newqueriesblockqueryandqueriesare mutually exclusivequeriesattributeThousandEyes Fix
account_group_idattribute to thethousandeyesschema to match the updated SDKOther
nobl9-goand other dependenciesExample Usage
Test Plan
make test/unit)dev-demo-3.nobl9.dev(all 160+ SLO subtests)Part of PC-18992.