Skip to content
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.

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

TRT-1619: variant cross-comparison api #1937

Merged
30 changes: 23 additions & 7 deletions pkg/api/componentreadiness/component_report.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,15 +413,24 @@ func (c *componentReportGenerator) getCommonTestStatusQuery(allJobVariants crtyp
queryString += ` AND NOT 'Disruption' in UNNEST(capabilities)`
}

for k, vs := range c.IncludeVariants {
variantGroups := c.IncludeVariants
// potentially cross-compare variants for the sample
if isSample && len(c.VariantCrossCompare) > 0 {
variantGroups = c.CompareVariants
}
if variantGroups == nil { // server-side view definitions may omit a variants map
variantGroups = map[string][]string{}
}

for group, variants := range variantGroups {
queryString += " AND ("
first := true
for _, v := range vs {
for _, variant := range variants {
if first {
queryString += fmt.Sprintf(`jv_%s.variant_value = '%s'`, k, v)
queryString += fmt.Sprintf(`jv_%s.variant_value = '%s'`, group, variant)
first = false
} else {
queryString += fmt.Sprintf(` OR jv_%s.variant_value = '%s'`, k, v)
queryString += fmt.Sprintf(` OR jv_%s.variant_value = '%s'`, group, variant)
}
}
queryString += ")"
Expand Down Expand Up @@ -534,7 +543,10 @@ func (c *componentReportGenerator) getSampleQueryStatus(
ComponentReportGenerator: c,
}

componentReportTestStatus, errs := api.GetDataFromCacheOrGenerate[crtype.ReportTestStatus](c.client.Cache, c.cacheOption, api.GetPrefixedCacheKey("SampleTestStatus~", generator), generator.queryTestStatus, crtype.ReportTestStatus{})
componentReportTestStatus, errs := api.GetDataFromCacheOrGenerate[crtype.ReportTestStatus](
c.client.Cache, c.cacheOption,
api.GetPrefixedCacheKey("SampleTestStatus~", generator),
generator.queryTestStatus, crtype.ReportTestStatus{})

if len(errs) > 0 {
return nil, errs
Expand Down Expand Up @@ -1322,8 +1334,12 @@ func (c *componentReportGenerator) generateComponentTestReport(baseStatus map[st
if !ok {
testStats.ReportStatus = crtype.MissingSample
} else {
approvedRegression := regressionallowances.IntentionalRegressionFor(c.SampleRelease.Release, testID.ColumnIdentification, testID.TestID)
resolvedIssueCompensation, triagedIncidents = c.triagedIncidentsFor(testID)
var approvedRegression *regressionallowances.IntentionalRegression
if len(c.VariantCrossCompare) == 0 { // only really makes sense when not cross-comparing variants:
// look for corresponding regressions we can account for in the analysis
approvedRegression = regressionallowances.IntentionalRegressionFor(c.SampleRelease.Release, testID.ColumnIdentification, testID.TestID)
resolvedIssueCompensation, triagedIncidents = c.triagedIncidentsFor(testID)
}
requiredConfidence := c.getRequiredConfidence(testID.TestID, testID.Variants)
testStats = c.assessComponentStatus(requiredConfidence, sampleStats.TotalCount, sampleStats.SuccessCount,
sampleStats.FlakeCount, baseStats.TotalCount, baseStats.SuccessCount,
Expand Down
51 changes: 39 additions & 12 deletions pkg/api/componentreadiness/queryparamparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,29 +158,56 @@ func parsePROptions(req *http.Request) *crtype.PullRequestOptions {
return &pro
}

func parseVariantOptions(req *http.Request, allJobVariants crtype.JobVariants) (crtype.RequestVariantOptions, error) {
var err error
variantOption := crtype.RequestVariantOptions{}
func parseVariantOptions(req *http.Request, allJobVariants crtype.JobVariants) (opts crtype.RequestVariantOptions, err error) {
columnGroupBy := req.URL.Query().Get("columnGroupBy")
variantOption.ColumnGroupBy, err = api.VariantsStringToSet(allJobVariants, columnGroupBy)
opts.ColumnGroupBy, err = api.VariantsStringToSet(allJobVariants, columnGroupBy)
if err != nil {
return variantOption, err
return
}
dbGroupBy := req.URL.Query().Get("dbGroupBy")
variantOption.DBGroupBy, err = api.VariantsStringToSet(allJobVariants, dbGroupBy)
opts.DBGroupBy, err = api.VariantsStringToSet(allJobVariants, dbGroupBy)
if err != nil {
return variantOption, err
return
}
variantOption.RequestedVariants = map[string]string{}

opts.RequestedVariants = map[string]string{}
// Only the dbGroupBy variants can be specifically requested
for _, variant := range variantOption.DBGroupBy.List() {
for _, variant := range opts.DBGroupBy.List() {
if value := req.URL.Query().Get(variant); value != "" {
variantOption.RequestedVariants[variant] = value
opts.RequestedVariants[variant] = value
}
}
includeVariants := req.URL.Query()["includeVariant"]
variantOption.IncludeVariants, err = api.IncludeVariantsToMap(allJobVariants, includeVariants)
return variantOption, err
opts.IncludeVariants, err = api.VariantListToMap(allJobVariants, includeVariants)
if err != nil {
return
}
compareVariants, err := api.VariantListToMap(allJobVariants, req.URL.Query()["compareVariant"])
if err != nil {
return
}

opts.VariantCrossCompare = req.URL.Query()["variantCrossCompare"]
if len(opts.VariantCrossCompare) > 0 {
// when we are cross-comparing variants, we need to construct the compareVariants map from the parameters.
// the resulting compareVariants map is includeVariants...
opts.CompareVariants = map[string][]string{}
for group, variants := range opts.IncludeVariants {
opts.CompareVariants[group] = variants
}

// ...with overrides from compareVariant parameters.
for _, group := range opts.VariantCrossCompare {
if variants := compareVariants[group]; len(variants) > 0 {
opts.CompareVariants[group] = variants
} else {
// a group override without any variants listed means not to restrict the variants in this group.
// in that case we don't want any where clause for the group, so we just omit it from the map.
delete(opts.CompareVariants, group)
}
}
}
return
}

func ParseIntArg(req *http.Request, name string, defaultVal int, validator func(int) bool) (int, error) {
Expand Down
131 changes: 128 additions & 3 deletions pkg/api/componentreadiness/queryparamparser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var (
func TestParseComponentReportRequest(t *testing.T) {

allJobVariants := crtype.JobVariants{Variants: map[string][]string{
"Architecture": {"amd64", "arm64", "heterogeneous"},
"Architecture": {"amd64", "arm64", "s390x", "ppc64le", "heterogeneous"},
"FeatureSet": {"default", "techpreview"},
"Installer": {"ipi", "upi"},
"Network": {"ovn", "sdn"},
Expand Down Expand Up @@ -60,8 +60,29 @@ func TestParseComponentReportRequest(t *testing.T) {
IgnoreDisruption: true,
},
}
// would like to test with a view that does define cross-compare variants
view417cross := view417main
view417cross.Name = "4.17-cross"
view417cross.VariantOptions = crtype.RequestVariantOptions{
VariantCrossCompare: []string{"Topology"},
IncludeVariants: map[string][]string{
"Architecture": []string{"amd64"},
"Installer": []string{"ipi", "upi"},
"Topology": []string{"ha"},
},
CompareVariants: map[string][]string{
"Architecture": []string{"amd64"},
"Installer": []string{"ipi", "upi"},
"Topology": []string{"single"},
},
// also remove Topology from columnGroupBy and dbGroupBy
ColumnGroupBy: sets.NewString("Platform", "Architecture", "Network"),
DBGroupBy: sets.NewString("Platform", "Architecture", "Network", "Suite", "FeatureSet", "Upgrade", "Installer"),
}

views := []crtype.View{
view417main,
view417cross,
}

now := time.Now().UTC()
Expand Down Expand Up @@ -92,7 +113,6 @@ func TestParseComponentReportRequest(t *testing.T) {
{"baseRelease", "4.15"},
{"baseStartTime", "2024-02-01T00:00:00Z"},
{"confidence", "95"},
{"groupBy", "cloud,arch,network"},
{"columnGroupBy", "Platform,Architecture,Network"},
{"dbGroupBy", "Platform,Architecture,Network,Topology,FeatureSet,Upgrade,Installer"},
{"ignoreDisruption", "true"},
Expand Down Expand Up @@ -146,7 +166,6 @@ func TestParseComponentReportRequest(t *testing.T) {
{"baseRelease", "4.15"},
{"baseStartTime", "ga-30d"},
{"confidence", "95"},
{"groupBy", "cloud,arch,network"},
{"columnGroupBy", "Platform,Architecture,Network"},
{"dbGroupBy", "Platform,Architecture,Network,Topology,FeatureSet,Upgrade,Installer"},
{"ignoreDisruption", "true"},
Expand Down Expand Up @@ -206,6 +225,7 @@ func TestParseComponentReportRequest(t *testing.T) {
"FeatureSet": {"default", "techpreview"},
"Installer": {"ipi", "upi"},
},
CompareVariants: nil, // the view is likely not to specify compare variants at all
},
baseRelease: crtype.RequestReleaseOptions{
Release: "4.16",
Expand Down Expand Up @@ -244,6 +264,111 @@ func TestParseComponentReportRequest(t *testing.T) {
},
errMessage: "params cannot be combined with view",
},
{
name: "normal query params but with variant cross-compare",
queryParams: [][]string{
{"baseEndTime", "2024-02-28T23:59:59Z"},
{"baseRelease", "4.15"},
{"baseStartTime", "2024-02-01T00:00:00Z"},
{"columnGroupBy", "Platform,Network"},
{"dbGroupBy", "Platform,Network,FeatureSet,Upgrade,Installer"},
{"sampleEndTime", "2024-04-11T23:59:59Z"},
{"sampleRelease", "4.16"},
{"sampleStartTime", "2024-04-04T00:00:05Z"},
{"includeVariant", "Architecture:amd64"},
{"includeVariant", "Architecture:arm64"},
{"includeVariant", "Topology:ha"},
{"includeVariant", "FeatureSet:default"},
{"includeVariant", "Installer:ipi"},
{"includeVariant", "Installer:upi"},
{"variantCrossCompare", "Architecture"},
{"variantCrossCompare", "Topology"},
{"compareVariant", "Architecture:s390x"},
{"compareVariant", "Architecture:ppc64le"},
{"compareVariant", "Topology:single"},
},
variantOption: crtype.RequestVariantOptions{
ColumnGroupBy: sets.NewString("Platform", "Network"),
DBGroupBy: sets.NewString("Platform", "Network", "FeatureSet", "Upgrade", "Installer"),
IncludeVariants: map[string][]string{
"Architecture": {"amd64", "arm64"},
"Topology": {"ha"},
"FeatureSet": {"default"},
"Installer": {"ipi", "upi"},
},
CompareVariants: map[string][]string{
"Architecture": {"s390x", "ppc64le"},
"Topology": {"single"},
"FeatureSet": {"default"},
"Installer": {"ipi", "upi"},
},
VariantCrossCompare: []string{"Architecture", "Topology"},
RequestedVariants: map[string]string{},
},
baseRelease: crtype.RequestReleaseOptions{
Release: "4.15",
Start: time.Date(2024, time.February, 1, 0, 0, 0, 0, time.UTC),
End: time.Date(2024, time.February, 28, 23, 59, 59, 0, time.UTC),
},
sampleRelease: crtype.RequestReleaseOptions{
Release: "4.16",
Start: time.Date(2024, time.April, 4, 0, 0, 5, 0, time.UTC),
End: time.Date(2024, time.April, 11, 23, 59, 59, 0, time.UTC),
},
testIDOption: crtype.RequestTestIdentificationOptions{},
advancedOption: crtype.RequestAdvancedOptions{
MinimumFailure: 3,
Confidence: 95,
PityFactor: 5,
IgnoreMissing: false,
IgnoreDisruption: true,
},
cacheOption: cache.RequestOptions{
ForceRefresh: false,
},
},
{
name: "cross-compare view",
queryParams: [][]string{
{"view", "4.17-cross"},
},
variantOption: crtype.RequestVariantOptions{
ColumnGroupBy: sets.NewString("Platform", "Architecture", "Network"),
DBGroupBy: sets.NewString("Platform", "Architecture", "Network", "Suite", "FeatureSet", "Upgrade", "Installer"),
IncludeVariants: map[string][]string{
"Architecture": {"amd64"},
"Installer": {"ipi", "upi"},
"Topology": {"ha"},
},
VariantCrossCompare: []string{"Topology"},
CompareVariants: map[string][]string{
"Architecture": {"amd64"},
"Installer": {"ipi", "upi"},
"Topology": {"single"},
},
},
baseRelease: crtype.RequestReleaseOptions{
Release: "4.16",
Start: time.Date(2024, time.May, 28, 0, 0, 0, 0, time.UTC),
End: time.Date(2024, time.June, 27, 23, 59, 59, 0, time.UTC),
},
sampleRelease: crtype.RequestReleaseOptions{
Release: "4.17",
Start: time.Date(nowMinus7Days.Year(), nowMinus7Days.Month(), nowMinus7Days.Day(), 0, 0, 0, 0, time.UTC),
End: nowRoundUp,
},
testIDOption: crtype.RequestTestIdentificationOptions{},
advancedOption: crtype.RequestAdvancedOptions{
MinimumFailure: 3,
Confidence: 95,
PityFactor: 5,
IgnoreMissing: false,
IgnoreDisruption: true,
},
cacheOption: cache.RequestOptions{
ForceRefresh: false,
},
},
}

for _, tc := range tests {
Expand Down
Loading