Skip to content

Fix metric_custom_indicator's good and total metrics array #545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 31, 2024
Merged

Fix metric_custom_indicator's good and total metrics array #545

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
12debcc
first fix attempt
jloleysens Jan 29, 2024
d0c697e
also support multiple metrics in total
jloleysens Jan 29, 2024
ef01b30
run test against fixed version
jloleysens Jan 29, 2024
10c986a
Revert "run test against fixed version"
jloleysens Jan 29, 2024
5ffaa40
fix name extraction
jloleysens Jan 29, 2024
0027cfb
fixed slo read data
jloleysens Jan 29, 2024
e623433
oops
jloleysens Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions internal/kibana/slo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package kibana

import (
"context"
"fmt"

"github.com/elastic/terraform-provider-elasticstack/generated/slo"
"github.com/elastic/terraform-provider-elasticstack/internal/clients"
Expand Down Expand Up @@ -541,34 +542,42 @@ func getSloFromResourceData(d *schema.ResourceData) (models.Slo, diag.Diagnostic
}

case "metric_custom_indicator":
goodMetricsRaw := d.Get(indicatorType + ".0.good.0.metrics").([]interface{})
Copy link
Member

Choose a reason for hiding this comment

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

It seems that in the SLO UI the total can also accept an array of metrics. Should we also support an array of metrics for it?

Screenshot 2024-01-29 at 11 11 02 AM

var goodMetrics []slo.IndicatorPropertiesCustomMetricParamsGoodMetricsInner
for n := range goodMetricsRaw {
idx := fmt.Sprint(n)
goodMetrics = append(goodMetrics, slo.IndicatorPropertiesCustomMetricParamsGoodMetricsInner{
Name: d.Get(indicatorType + ".0.good.0.metrics." + idx + ".name").(string),
Field: d.Get(indicatorType + ".0.good.0.metrics." + idx + ".field").(string),
Aggregation: d.Get(indicatorType + ".0.good.0.metrics." + idx + ".aggregation").(string),
Filter: getOrNilString(indicatorType+".0.good.0.metrics."+idx+".filter", d),
})
}
totalMetricsRaw := d.Get(indicatorType + ".0.total.0.metrics").([]interface{})
var totalMetrics []slo.IndicatorPropertiesCustomMetricParamsTotalMetricsInner
for n := range totalMetricsRaw {
idx := fmt.Sprint(n)
totalMetrics = append(totalMetrics, slo.IndicatorPropertiesCustomMetricParamsTotalMetricsInner{
Name: d.Get(indicatorType + ".0.total.0.metrics.+" + idx + ".name").(string),
Field: d.Get(indicatorType + ".0.total.0.metrics." + idx + ".field").(string),
Aggregation: d.Get(indicatorType + ".0.total.0.metrics." + idx + ".aggregation").(string),
Filter: getOrNilString(indicatorType+".0.total.0.metrics."+idx+".filter", d),
})
}
indicator = slo.SloResponseIndicator{
IndicatorPropertiesCustomMetric: &slo.IndicatorPropertiesCustomMetric{
Type: indicatorAddressToType[indicatorType],
Params: slo.IndicatorPropertiesCustomMetricParams{
Filter: getOrNilString(indicatorType+".0.filter", d),
Index: d.Get(indicatorType + ".0.index").(string),
TimestampField: d.Get(indicatorType + ".0.timestamp_field").(string),
Total: slo.IndicatorPropertiesCustomMetricParamsTotal{
Equation: d.Get(indicatorType + ".0.total.0.equation").(string),
Metrics: []slo.IndicatorPropertiesCustomMetricParamsTotalMetricsInner{ //are there actually instances where there are more than one 'good' / 'total'? Need to build array if so.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes there are 😄

{
Name: d.Get(indicatorType + ".0.total.0.metrics.0.name").(string),
Field: d.Get(indicatorType + ".0.total.0.metrics.0.field").(string),
Aggregation: d.Get(indicatorType + ".0.total.0.metrics.0.aggregation").(string),
Filter: getOrNilString(indicatorType+".0.total.0.metrics.0.filter", d),
},
},
},
Good: slo.IndicatorPropertiesCustomMetricParamsGood{
Equation: d.Get(indicatorType + ".0.good.0.equation").(string),
Metrics: []slo.IndicatorPropertiesCustomMetricParamsGoodMetricsInner{ //are there actually instances where there are more than one 'good' / 'total'? Need to build array if so.
{
Name: d.Get(indicatorType + ".0.good.0.metrics.0.name").(string),
Field: d.Get(indicatorType + ".0.good.0.metrics.0.field").(string),
Aggregation: d.Get(indicatorType + ".0.good.0.metrics.0.aggregation").(string),
Filter: getOrNilString(indicatorType+".0.good.0.metrics.0.filter", d),
},
},
Metrics: goodMetrics,
},
Total: slo.IndicatorPropertiesCustomMetricParamsTotal{
Equation: d.Get(indicatorType + ".0.total.0.equation").(string),
Metrics: totalMetrics,
},
},
},
Expand Down
23 changes: 21 additions & 2 deletions internal/kibana/slo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,18 @@ func TestAccResourceSlo(t *testing.T) {
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.metrics.0.name", "A"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.metrics.0.aggregation", "sum"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.metrics.0.field", "processor.processed"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.metrics.1.name", "B"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.metrics.1.aggregation", "sum"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.metrics.1.field", "processor.processed"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.good.0.equation", "A + B"),

resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.metrics.0.name", "A"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.metrics.0.aggregation", "sum"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.metrics.0.field", "processor.accepted"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.metrics.0.name", "B"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.metrics.0.aggregation", "sum"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.metrics.0.field", "processor.accepted"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "metric_custom_indicator.0.total.0.equation", "A + B"),
resource.TestCheckResourceAttr("elasticstack_kibana_slo.test_slo", "group_by", "some.field"),
),
},
Expand Down Expand Up @@ -389,7 +398,12 @@ func getSLOConfig(name string, indicatorType string, settingsEnabled bool, tags
aggregation = "sum"
field = "processor.processed"
}
equation = "A"
metrics {
name = "B"
aggregation = "sum"
field = "processor.processed"
}
equation = "A + B"
}

total {
Expand All @@ -398,7 +412,12 @@ func getSLOConfig(name string, indicatorType string, settingsEnabled bool, tags
aggregation = "sum"
field = "processor.accepted"
}
equation = "A"
metrics {
name = "B"
aggregation = "sum"
field = "processor.accepted"
}
equation = "A + B"
}
}
`
Expand Down