Skip to content

Separate Agent MBean from Output#62

Merged
marchof merged 2 commits into
masterfrom
issue-62
Jan 9, 2013
Merged

Separate Agent MBean from Output#62
marchof merged 2 commits into
masterfrom
issue-62

Conversation

@marchof

@marchof marchof commented Jan 8, 2013

Copy link
Copy Markdown
Member

The JaCoCo agent should register an MBean separately to the output mode. This allows to use the MBean in combination with an output mode (e.g. trigger dump to file via MBean). The MBean will then have the same API as the runtime API defined in #61:

  • Get agent version
  • Set/get session id
  • reset execution data
  • Dump (and optionally reset) execution data via configured output
  • Dump (and optionally reset) execution data to byte[]

The MBean is registered by default. This behavior can be modified with the new boolean agent option mbean (e.g. mbean=false).

@Godin

Godin commented Dec 29, 2012

Copy link
Copy Markdown
Member

If MBean registered by default, then what to do with an existing output mode "mbean" ? Possible options - fully drop it, or it will behave as "none".

@Godin Godin mentioned this pull request Dec 29, 2012
@Godin

Godin commented Dec 29, 2012

Copy link
Copy Markdown
Member

Also should be noted that as mentioned in #61 package "javax.management" not available on Android.
And BTW package "java.lang.instrument" also not available on Android, which is not fatal, but leads to messages in log like:

W/ClassPathPackageInfoSource(  775): java.lang.ClassNotFoundException: org.jacoco.agent.rt.CoverageTransformer
W/ClassPathPackageInfoSource(  775):    at java.lang.Class.classForName(Native Method)
W/ClassPathPackageInfoSource(  775):    at java.lang.Class.forName(Class.java:217)
W/ClassPathPackageInfoSource(  775):    at android.test.ClassPathPackageInfoSource.createPackageInfo(ClassPathPackageInfoSource.java:88)

And I don't know how to suppress those messages without removal of classes.

@marchof

marchof commented Dec 29, 2012

Copy link
Copy Markdown
Member Author

I don't think mbean output is widely used, so we can drop this and clearly document this in the change log. On the other hand if enable it by default this might cause issues in some scenarios. So maybe the default should be no JMX.

Regarding missing APIs on Android the only solution is probably a different JaCoCo runtime packaging without JMX and Java agent support. As the error log is not a show stopper I wouldn't try to solve this right now.

@marchof marchof closed this Dec 29, 2012
@Godin Godin reopened this Jan 4, 2013
@marchof

marchof commented Jan 8, 2013

Copy link
Copy Markdown
Member Author

@Godin Hi Evgeny, can you please verify that this version also works on Android? I'm asking because the current implementation has direct javax.management references from the Agent class. If this causes problems I we need to pull JMX operations into a separate class (like MBeanController was before).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "exposes" should be replaced by "expose".

@marchof

marchof commented Jan 8, 2013

Copy link
Copy Markdown
Member Author

@Godin Thanks for reviewing, I fixed the typos. Good to merge?

@Godin

Godin commented Jan 8, 2013

Copy link
Copy Markdown
Member

@marchof I'll test on Android tomorrow morning and will come back to you about merge.

@Godin

Godin commented Jan 9, 2013

Copy link
Copy Markdown
Member

@marchof It works fine on Android, since the code to create MBean (i.e. which uses non-existent package) not executed by default. So we can safely merge this!

marchof added a commit that referenced this pull request Jan 9, 2013
Separate Agent MBean from Output
@marchof marchof merged commit b71f404 into master Jan 9, 2013
@marchof marchof deleted the issue-62 branch January 9, 2013 09:48
@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants