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

Add support for excluding targets for Probes #15667

Merged

Conversation

@blazember
Copy link
Contributor

blazember commented Oct 3, 2019

The change introduces excludedTargets to the @Probe annotation for
excluding probes on a certain metrics target platform. These platforms
are defined in the MetricsTarget enum, listing MANAGEMENT_CENTER, JMX,
DIAGNOSTICS.

Only probes are supported for exclusion. Metrics inserted directly via
probe functions are not. These are typically JVM/OS level metrics that
we want to publish to all platforms.

The main motivation for this change is to prevent metrics sent to
Management Center. Examples are: operation.thread*, tcp.connection[*],
tcp.inputThread*, tcp.outputThread* and internal-executor*.

This change doesn't extend any production probe with the new tag. That
change needs to be done separately after reviewing the exclusion list.

EE PR: hazelcast/hazelcast-enterprise#3204

@blazember blazember added this to the 4.0 milestone Oct 3, 2019
@blazember blazember requested a review from hazelcast/clients as a code owner Oct 3, 2019
@blazember blazember self-assigned this Oct 3, 2019
@blazember

This comment has been minimized.

Copy link
Contributor Author

blazember commented Oct 3, 2019

@blazember blazember force-pushed the blazember:4.0/mc-metrics/exclude-targets branch from 5eeab88 to 3c387b9 Oct 4, 2019
@puzpuzpuz puzpuzpuz self-requested a review Oct 4, 2019
The change introduces excludedTargets to the @Probe annotation for
excluding probes on a certain metrics target platform. These platforms
are defined in the MetricsTarget enum, listing MANAGEMENT_CENTER, JMX,
DIAGNOSTICS.

Only probes are supported for exclusion. Metrics inserted directly via
probe functions are not. These are typically JVM/OS level metrics that
we want to publish to all platforms.

The main motivation for this change is to prevent metrics sent to
Management Center. Examples are: `operation.thread*`, `tcp.connection[*]`,
`tcp.inputThread*`, `tcp.outputThread*` and `internal-executor*`.

This change doesn't extend any production probe with the new tag. That
change needs to be done separately after reviewing the exclusion list.
@blazember blazember force-pushed the blazember:4.0/mc-metrics/exclude-targets branch from 3c387b9 to 13417a8 Oct 4, 2019
Copy link
Contributor

puzpuzpuz left a comment

LGTM. Left a comment with a potential enhancement for a future PR.


if (excludedTargets.length > 0) {
EnumSet<MetricTarget> metricTargets = EnumSet.noneOf(MetricTarget.class);
Collections.addAll(metricTargets, excludedTargets);

This comment has been minimized.

Copy link
@puzpuzpuz

puzpuzpuz Oct 4, 2019

Contributor

And idea for a potential enhancement that @blazember generated during an internal discussion. This enhancement should not prevent from merging this PR.

We may be able to avoid litter generated by EnumSet allocated in this method by caching all possible combinations of MetricTarget[] in MetricTarget enum. The cache could look like Map<Integer, Set<MetricTarget>> (or it could be Int2ObjectHashMap<Set<MetricTarget>>). Keys could be calculated based on MetricTarget[] elements by a hash function, e.g. something like:

int hash(MetricTarget[] excluded) {
    int hash = 0;
    for (int i = 0; i < excluded.length; i++) {
        hash += excluded[i].ordinal() * Math.pow(10, i);
    }
   return hash;
}
@asimarslan asimarslan removed the request for review from hazelcast/clients Oct 4, 2019
@mmedenjak mmedenjak merged commit 0843f18 into hazelcast:master Oct 4, 2019
1 check passed
1 check passed
default Test PASSed.
Details
@blazember blazember deleted the blazember:4.0/mc-metrics/exclude-targets branch Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.