Add metric filtering capability #1728
Conversation
| validateSpanProcessorIncludeExclude(includeExclude); | ||
| break; | ||
| case METRIC_FILTER: |
| } | ||
| } | ||
| } | ||
|
|
||
| public static class ProcessorIncludeExclude { | ||
| public MatchType matchType; | ||
| public List<String> spanNames = new ArrayList<>(); | ||
| public List<String> metricNames = new ArrayList<>(); |
There was a problem hiding this comment.
if not renaming to METRIC, can you rename this to metricFilterNames for consistency.
| if (attributes.isEmpty()) { | ||
| throw new FriendlyException("A log processor configuration has an " + includeExclude + " section with no \"attributes\".", | ||
| "Please provide \"attributes\" under the " + includeExclude + " section of the log processor configuration. " + | ||
| "Learn more about log processors here: https://go.microsoft.com/fwlink/?linkid=2151557"); | ||
| } | ||
|
|
||
| validateSectionIsEmpty(spanNames, ProcessorType.LOG, includeExclude, "spanNames"); | ||
| validateSectionIsEmpty(metricNames, ProcessorType.LOG, includeExclude, "metricNames"); |
There was a problem hiding this comment.
metricFilterNames? or rename the processor type to METRIC?
| "type": "metric-filter", | ||
| "exclude": { | ||
| "matchType": "strict", | ||
| "metricNames": [ |
There was a problem hiding this comment.
i see why metricNames makes sense now. please ignore renaming comments.
| // Moshi JSON builder do not allow case insensitive mapping | ||
| strict, regexp | ||
| @Json(name = "strict") | ||
| STRICT { |
There was a problem hiding this comment.
it's not clear to me why you need to define MatchType in both places.. one here and calling MetricFilter's constants.. why not just keep one enum here?
| } | ||
| } | ||
|
|
||
| public enum MatchType { |
There was a problem hiding this comment.
I wonder why not just define this in configuration.java?
There was a problem hiding this comment.
because core doesn't depend on agent (where configuration is)
I left a couple of comments in Configuration about this:
// TODO (trask) this could be revisited in the future once core goes away
// need a different MetricFilter in this case because it lives in core
There was a problem hiding this comment.
i saw that.. didn't quite get it.. ok, now it makes sense.
* Add metric filtering capability * Change enums to uppercase * Spotbugs
Resolves #1665