Skip to content

Conversation

@marcuss
Copy link
Contributor

@marcuss marcuss commented Jan 30, 2021

No description provided.

@kgilpin
Copy link
Contributor

kgilpin commented Feb 2, 2021

Is there a spec for this @ptrdvrk ? My understanding was that we wanted to have the maven plugin add the Java agent to the jvm args. A command to make a recording could be interesting but the primary means for making recordings is running tests.

@kgilpin
Copy link
Contributor

kgilpin commented Feb 2, 2021

@marcuss It seems that a part of this PR would be to publish to maven central? Or a related PR.

@ptrdvrk
Copy link
Contributor

ptrdvrk commented Feb 2, 2021

@kgilpin the functional spec is here: https://docs.google.com/document/d/147EDYpBh1xdEw6KUaC1L2W3qmemC0mJPE5GgrK_iwmo/edit#heading=h.14ya0r1ubmbk
You are correct. The plugin is expected to enable instrumentation, not to start/stop recording.
@marcuss let's review that. The plugin needs to load the appmap.jar, which will trigger the instrumentation to be started via the premain static method (see the jar manifest); and pass the required parameters to it.

@marcuss
Copy link
Contributor Author

marcuss commented Feb 2, 2021

@ptrdvrk this is still a work in progress, I will check the functional spec I had lost the link to it and make the necessary adjustments.

@marcuss
Copy link
Contributor Author

marcuss commented Feb 2, 2021

@kgilpin @ptrdvrk
Is it ok for me to keep this draft PR open while I add the missing functionality? I ask because I don't want this to look like the current state is the way I'm going to deliver the project, for me is just a way to show some progress since there is a small amount of time allocated each day for the task.

@marcuss
Copy link
Contributor Author

marcuss commented Feb 2, 2021

@marcuss
Copy link
Contributor Author

marcuss commented Feb 3, 2021

First working version using console commands to load the agent. (needs a lot of refactoring)

@marcuss marcuss marked this pull request as ready for review February 5, 2021 05:16
@marcuss
Copy link
Contributor Author

marcuss commented Feb 5, 2021

As per yesterday, I changed the project to work without any EPL portion of code and made all the options work as expected including one for skip execution of the agent.

Fixed issue with preexistent command line.
Minor Refactor.
@marcuss marcuss changed the title Maven plugin for appmap java recorder [Ready to Merge] Maven plugin for appmap java recorder Feb 21, 2021
@marcuss
Copy link
Contributor Author

marcuss commented Feb 22, 2021

Added requested changes on xbootclasspath issue.

Copy link
Collaborator

@dustinbyrne dustinbyrne left a comment

Choose a reason for hiding this comment

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

This looks great @marcuss, thank you for the hard work you've put into this. I've added a few naming related comments, then we're good to merge!

@marcuss marcuss changed the title [Ready to Merge] Maven plugin for appmap java recorder [Changes Requested] Maven plugin for appmap java recorder Feb 25, 2021
@marcuss
Copy link
Contributor Author

marcuss commented Feb 25, 2021

@dustinbyrne is a bigger change, but... should I also change the folder name from
appmap-java-maven-plugin>

to

appmap-agent-maven-plugin ?

@marcuss marcuss force-pushed the maven-plugin-appmap-java branch from 854fecf to aded373 Compare February 25, 2021 03:29
Copy link
Contributor Author

@marcuss marcuss left a comment

Choose a reason for hiding this comment

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

I went down the rabbit hole for 2 hours, still not even compiling in my machine.
researching could be related to java 8 and adding new dependencies to the pom
the fact that I could use it during all this time may be related to maven repo cache..

I will continue to work on this tomorrow.

xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.appland</groupId>
<artifactId>appmap-java-maven-plugin</artifactId>
Copy link
Contributor Author

@marcuss marcuss Feb 25, 2021

Choose a reason for hiding this comment

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

@dustinbyrne appmap-maven-plugin ?

removed java 8 dependencies, now the project can be compiled with java 1.7
Copy link
Collaborator

@dustinbyrne dustinbyrne left a comment

Choose a reason for hiding this comment

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

💯

@dustinbyrne dustinbyrne merged commit 1798df2 into getappmap:master Feb 25, 2021
marcuss added a commit to marcuss/appmap-java that referenced this pull request Feb 26, 2021
appland-release pushed a commit that referenced this pull request Mar 16, 2021
# 1.0.0 (2021-03-16)

### Features

* AppMap Maven plugin ([#46](#46)) ([1798df2](1798df2))
* appmap.yml errors are logged to stderr ([e746253](e746253))
appland-release pushed a commit that referenced this pull request Mar 17, 2021
# 1.0.0 (2021-03-17)

### Features

* AppMap Maven plugin ([#46](#46)) ([1798df2](1798df2))
* appmap.yml errors are logged to stderr ([e746253](e746253))
appland-release pushed a commit that referenced this pull request Mar 17, 2021
# 1.0.0 (2021-03-17)

### Features

* AppMap Maven plugin ([#46](#46)) ([1798df2](1798df2))
* appmap.yml errors are logged to stderr ([e746253](e746253))
appland-release pushed a commit that referenced this pull request Mar 17, 2021
# 1.0.0 (2021-03-17)

### Features

* AppMap Maven plugin ([#46](#46)) ([1798df2](1798df2))
* appmap.yml errors are logged to stderr ([e746253](e746253))
@appland-release
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

hleb-rubanau pushed a commit to hleb-rubanau/appmap-java that referenced this pull request Apr 13, 2021
# 1.0.0 (2021-04-13)

### Bug Fixes

* **gradle:** add debugging output ([8b63cfa](8b63cfa))
* **gradle:** add debugging output ([e577d30](e577d30))
* **gradle:** alternative way of specifying nexus credentials ([2df78dd](2df78dd))
* **gradle:** alternative way of specifying nexus credentials ([a018510](a018510))
* **gradle:** broken gradle directives ([b529ea2](b529ea2))
* **gradle:** fix typo ([7d11ffa](7d11ffa))
* **gradle:** troubleshoot testenv ([80e7b29](80e7b29))
* **gradle:** troubleshoot testenv ([5c16f70](5c16f70))
* Allow classes compiled without locals to be hooked ([0e0a0d3](0e0a0d3))
* Capture exceptions thrown from SQL interfaces ([9d1e66f](9d1e66f))
* disable http client requests ([getappmap#60](https://github.com/hleb-rubanau/appmap-java/issues/60)) ([2131d82](2131d82))
* Don't append System path to class pools ([681d74e](681d74e))
* improve path and package resolution ([getappmap#62](https://github.com/hleb-rubanau/appmap-java/issues/62)) ([c3ba3df](c3ba3df))
* Provide better error message when encountering an unknown event ([c69a877](c69a877))

### Features

* **docs:** improve CI documentation and also trigger the relase ([e6b5a8e](e6b5a8e))
* AppMap Maven plugin ([getappmap#46](https://github.com/hleb-rubanau/appmap-java/issues/46)) ([1798df2](1798df2))
* appmap.yml errors are logged to stderr ([e746253](e746253))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants