Skip to content

Commit

Permalink
Fix maxdimassociator to loop through all mappings
Browse files Browse the repository at this point in the history
Mappings in the new associator are sorted by decreasing number of
dimensions, e.g.

[ Mapping_Dim_A_Dim_B, Mapping_Dim_A_Dim_C, Mapping_Dim_A ]

If trying to map a metric with Dim_A, Dim_B and Dim_C, the current
implementation would pic the first mapping (Mapping_Dim_A_Dim_B) because
it already matches part of the metric dimensions and would try to find
a match for dimensions names and values. However, if values don't match
it would give up and mark the metric to be skipped.

The new implementation continues to loop through the regex mappings to
check if there another one that contains the metric dimensions. In the
example above, Mapping_Dim_A_Dim_C would match again and the algo would
try again to find a match for dimensions names and values.

This should fix cases where ListMetrics returns metrics with more
dimensions than can be extracted from the ARN regex. E.g. for ApiGateway
the ARN contains only the ApiId dimension, but we should now be able to
match metrics that contains both ApiId and Method (as long as the ApiId
value matches).
  • Loading branch information
cristiangreco committed Jul 31, 2023
1 parent f0ceb44 commit ae51b50
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 26 deletions.
65 changes: 40 additions & 25 deletions pkg/job/maxdimassociator/associator.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,17 @@ func NewAssociator(logger logging.Logger, dimensionRegexps []*regexp.Regexp, res
mappedResources[idx] = true
}

assoc.mappings = append(assoc.mappings, m)
if len(m.dimensionsMapping) > 0 {
assoc.mappings = append(assoc.mappings, m)
} else {
// The mapping might end up as empty in cases e.g. where
// one of the regexps defined for the namespace doesn't match
// against any of the tagged resources. This might happen for
// example when we define multiple regexps (to capture sibling
// or sub-resources) but none are matched.
// This is fine, logging at debug just to keep track of it.
logger.Debug("unable to define a regex mapping", "regex", regex.String())
}
}

// sort all mappings by decreasing number of dimensions names
Expand Down Expand Up @@ -127,37 +137,42 @@ func (assoc Associator) AssociateMetricToResource(cwMetric *model.Metric) (*mode
assoc.logger.Debug("associating metric", "metric_name", cwMetric.MetricName, "dimensions", strings.Join(dimensions, ","))
}

// Find the mapping which contains the most
// Attempt to find the regex mapping which contains the most
// (but not necessarily all) the metric's dimensions names.
// Mappings are sorted by decreasing number of dimensions names.
var regexpMapping *dimensionsRegexpMapping
for idx, m := range assoc.mappings {
if containsAll(dimensions, m.dimensions) {
// Regex mappings are sorted by decreasing number of dimensions names,
// which favours find the mapping with most dimensions.
mappingFound := false
for idx, regexpMapping := range assoc.mappings {
if containsAll(dimensions, regexpMapping.dimensions) {
assoc.logger.Debug("matched metric with mapping", "mapping_idx", idx)
regexpMapping = m
break
}
}

if regexpMapping == nil {
// if no mapping is found, it means the ListMetrics API response
// did not contain any subset of the metric's dimensions names.
// Do not skip the metric though (create a "global" metric).
return nil, false
}
// A regex mapping has been found. The metric has all (and possibly more)
// the dimensions computed for the mapping. Now compute a signature
// of the labels (names and values) of the dimensions of this mapping.
mappingFound = true
labels := buildLabelsMap(cwMetric, regexpMapping)
signature := prom_model.LabelsToSignature(labels)

// A mapping has been found. The metric has all (and possibly more)
// the dimensions computed for the mapping. Pick only exactly
// the dimensions of the mapping to build a labels signature.
labels := buildLabelsMap(cwMetric, regexpMapping)
signature := prom_model.LabelsToSignature(labels)
// Check if there's an entry for the labels (names and values) of the metric,
// and return the resource in case.
if resource, ok := regexpMapping.dimensionsMapping[signature]; ok {
return resource, false
}

if resource, ok := regexpMapping.dimensionsMapping[signature]; ok {
return resource, false
// Otherwise, continue iterating across the rest of regex mappings
// to attempt to find another one with fewer dimensions.
}
}

// if there's no mapping entry for this resource, skip it
return nil, true
// At this point, we haven't been able to match the metric against
// any resource based on the dimensions the associator knows.
// If a regex mapping was ever found in the loop above but no entry
// (i.e. matching labels names and values) matched the metric dimensions,
// skip the metric altogether.
// Otherwise, if we didn't find any regex mapping it means we can't
// correctly map the dimensions names to a resource arn regex,
// but we still want to keep the metric and create a "global" metric.
return nil, mappingFound
}

// buildLabelsMap returns a map of labels names and values.
Expand Down
3 changes: 2 additions & 1 deletion pkg/job/maxdimassociator/associator_api_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ func TestAssociatorAPIGateway(t *testing.T) {
},
},
},
expectedSkip: true,
expectedSkip: false,
expectedResource: apiGatewayV1,
},
}

Expand Down

0 comments on commit ae51b50

Please sign in to comment.