Add capture_jmx_metrics option by felixbarny · Pull Request #801 · elastic/apm-agent-java

felixbarny · 2019-08-20T20:21:04Z

closes #469

Implement code
Add tests
Update documentation
Update CHANGELOG.md
~~Update supported-technologies.asciidoc~~
Added an API method or config option? Document in which version this will be introduced.

bmorelli25

New docs LGTM!

- uses ObjectName syntax, leverages javax.management.ObjectName for parsing - allow multiple attribute[] elements - more tests, including negative tests - make MapsTokenScanner more resilient against IndexOutOfBoundsExceptions

codecov-io · 2019-08-24T11:55:29Z

Codecov Report

Merging #801 into master will increase coverage by 0.71%.
The diff coverage is 86.17%.

@@             Coverage Diff              @@
##             master     #801      +/-   ##
============================================
+ Coverage     63.51%   64.23%   +0.71%     
  Complexity       80       80              
============================================
  Files           224      228       +4     
  Lines          9035     9280     +245     
  Branches       1154     1202      +48     
============================================
+ Hits           5739     5961     +222     
- Misses         2929     2939      +10     
- Partials        367      380      +13

Impacted Files	Coverage Δ	Complexity Δ
...n/java/co/elastic/apm/agent/metrics/MetricSet.java	`87.5% <ø> (ø)`	`0 <0> (ø)`	⬇️
...ava/co/elastic/apm/agent/jmx/JmxConfiguration.java	`100% <100%> (ø)`	`0 <0> (?)`
...a/co/elastic/apm/agent/metrics/MetricRegistry.java	`82.89% <28.57%> (-5.68%)`	`0 <0> (ø)`
...ava/co/elastic/apm/agent/jmx/JmxMetricTracker.java	`77.33% <77.33%> (ø)`	`0 <0> (?)`
.../main/java/co/elastic/apm/agent/jmx/JmxMetric.java	`87.01% <87.01%> (ø)`	`0 <0> (?)`
...ava/co/elastic/apm/agent/jmx/MapsTokenScanner.java	`97.43% <97.43%> (ø)`	`0 <0> (?)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ae1125...9ef428f. Read the comment docs.

eyalkoren

Very nice!! 🎉

In general, since this is a user-configuration thing, I would print some more errors related to misconfiguration, like:

usage of non-valid characters
usage of non-valid key names

Also- add test for dynamically addition and removal of metrics.

apm-agent-plugins/apm-jmx-plugin/src/main/java/co/elastic/apm/agent/jmx/JmxMetricTracker.java

felixbarny · 2019-08-26T06:43:27Z

I would print some more errors related to misconfiguration, like:

usage of non-valid characters

usage of non-valid key names

Could you be more specific? There are already quite a few error messages in MapsTokenScanner. Which cases do you feel are missing?

Also- add test for dynamically addition and removal of metrics.

All tests already add the metrics dynamically. Removing is not supported at this point.

eyalkoren · 2019-09-01T09:55:06Z

Could you be more specific? There are already quite a few error messages in MapsTokenScanner. Which cases do you feel are missing?

I am not trying to make it 100% bullet-proof, but thinking on possible mis-usages, eg:

typos (so if someone uses an unexpected key name - print it)
misunderstanding of the syntax (so disallow characters used for special purposes in the syntax within values, like , or :)

felixbarny · 2019-09-12T09:59:19Z

@eyalkoren Could you give this another review?

eyalkoren

Awesome! Two minor name change suggestions.

eyalkoren · 2019-09-18T08:45:58Z

apm-agent-plugins/apm-jmx-plugin/src/main/java/co/elastic/apm/agent/jmx/JmxMetricTracker.java

+        }
+    }
+
+    private List<JmxMetricRegistration> registerJmxMetrics(List<JmxMetric> jmxMetrics) {


This action is not really registering (ie adding to the registry), so maybe something like:

Suggested change

private List<JmxMetricRegistration> registerJmxMetrics(List<JmxMetric> jmxMetrics) {

private List<JmxMetricRegistration> compileJmxMetricRegistrationList(List<JmxMetric> jmxMetrics) {

eyalkoren · 2019-09-18T08:46:59Z

apm-agent-plugins/apm-jmx-plugin/src/main/java/co/elastic/apm/agent/jmx/JmxMetricTracker.java

+        return registrations;
+    }
+
+    private void registerJmxMetric(final JmxMetric jmxMetric, List<JmxMetricRegistration> registrations) throws JMException {


Suggested change

private void registerJmxMetric(final JmxMetric jmxMetric, List<JmxMetricRegistration> registrations) throws JMException {

private void addJmxMetricRegistration(final JmxMetric jmxMetric, List<JmxMetricRegistration> registrations) throws JMException {

Add capture_jmx_metrics option

ba239fa

closes elastic#469

felixbarny requested a review from eyalkoren August 20, 2019 20:21

felixbarny added the [zube]: In Review label Aug 20, 2019

felixbarny requested a review from bmorelli25 August 20, 2019 20:26

bmorelli25 approved these changes Aug 21, 2019

View reviewed changes

Make Java 7 compiler happy

ec1f5c1

eyalkoren assigned felixbarny Aug 21, 2019

felixbarny added 6 commits August 21, 2019 21:21

Add attribute[attr:metric_name=metric] syntax

8c3a173

- uses ObjectName syntax, leverages javax.management.ObjectName for parsing - allow multiple attribute[] elements - more tests, including negative tests - make MapsTokenScanner more resilient against IndexOutOfBoundsExceptions

Fix ES example documents

bc65b11

Merge remote-tracking branch 'origin/master' into jmx

0d221da

Update version

7a96bb1

Merge remote-tracking branch 'origin/master' into jmx

5dcc115

Update parent version of jmx plugin to 1.9.1-SNAPSHOT

9c48cb2

eyalkoren reviewed Aug 25, 2019

View reviewed changes

felixbarny added 5 commits September 11, 2019 16:07

Add ability to remove JMX metrics at runtime

5865420

Merge remote-tracking branch 'origin/master' into jmx

1ca42ff

Throw exception on unknown attributes

2e040d8

Make Java 7 compiler happy

02d9e86

Make things package private

e457031

eyalkoren approved these changes Sep 18, 2019

View reviewed changes

felixbarny added 2 commits September 23, 2019 15:13

Merge remote-tracking branch 'origin/master' into jmx

4c61398

Rename methods

9ef428f

felixbarny merged commit 8abcf4e into elastic:master Sep 24, 2019

zube bot added [zube]: Done and removed [zube]: In Review labels Sep 24, 2019

felixbarny deleted the jmx branch September 24, 2019 09:23

lreuven removed the [zube]: Done label Nov 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add capture_jmx_metrics option#801

Add capture_jmx_metrics option#801
felixbarny merged 15 commits intoelastic:masterfrom
felixbarny:jmx

felixbarny commented Aug 20, 2019 •

edited

Loading

Uh oh!

bmorelli25 left a comment

Uh oh!

codecov-io commented Aug 24, 2019 •

edited

Loading

Uh oh!

eyalkoren left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixbarny commented Aug 26, 2019

Uh oh!

eyalkoren commented Sep 1, 2019 •

edited

Loading

Uh oh!

felixbarny commented Sep 12, 2019 •

edited

Loading

Uh oh!

eyalkoren left a comment

Uh oh!

eyalkoren Sep 18, 2019

Uh oh!

eyalkoren Sep 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	private List<JmxMetricRegistration> registerJmxMetrics(List<JmxMetric> jmxMetrics) {
	private List<JmxMetricRegistration> compileJmxMetricRegistrationList(List<JmxMetric> jmxMetrics) {

	private void registerJmxMetric(final JmxMetric jmxMetric, List<JmxMetricRegistration> registrations) throws JMException {
	private void addJmxMetricRegistration(final JmxMetric jmxMetric, List<JmxMetricRegistration> registrations) throws JMException {

Conversation

felixbarny commented Aug 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmorelli25 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Aug 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

eyalkoren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixbarny commented Aug 26, 2019

Uh oh!

eyalkoren commented Sep 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixbarny commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eyalkoren left a comment

Choose a reason for hiding this comment

Uh oh!

eyalkoren Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

eyalkoren Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

felixbarny commented Aug 20, 2019 •

edited

Loading

codecov-io commented Aug 24, 2019 •

edited

Loading

eyalkoren commented Sep 1, 2019 •

edited

Loading

felixbarny commented Sep 12, 2019 •

edited

Loading