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

Added support for setting destination.address/port on JMS (client) spans who use ActiveMQ Artemis as underlying technology #1905

Closed
wants to merge 3 commits into from

Conversation

tobiasstadler
Copy link
Contributor

@tobiasstadler tobiasstadler commented Jul 8, 2021

What does this PR do?

Fixes #1904

Checklist

  • This is a new plugin
    • I have updated CHANGELOG.asciidoc
    • My code follows the style guidelines of this project
    • I have made corresponding changes to the documentation
    • x] I have added tests that prove my fix is effective or that my feature works
    • New and existing unit tests pass locally with my changes
    • I have updated supported-technologies.asciidoc
    • Added an API method or config option? Document in which version this will be introduced
    • Added an instrumentation plugin? Describe how you made sure that old, non-supported versions are not instrumented by accident.

…ans who use ActiveMQ Artemis as underlying technology (elastic#1904)
@tobiasstadler
Copy link
Contributor Author

I don't think this litte enhancement is worth an entry in supported-technologies.asciidoc

@apmmachine
Copy link
Contributor

apmmachine commented Jul 8, 2021

❕ Build Aborted

The PR is not allowed to run in the CI yet

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Reason: The PR is not allowed to run in the CI yet

  • Start Time: 2021-07-12T05:58:02.528+0000

  • Duration: 3 min 56 sec

  • Commit: 60b1a10

Trends 🧪

Image of Build Times

Steps errors 2

Expand to view the steps failures

Load a resource file from a shared library
  • Took 0 min 0 sec . View more details on here
  • Description: approval-list/elastic/apm-agent-java.yml
Error signal
  • Took 0 min 0 sec . View more details on here
  • Description: githubPrCheckApproved: The PR is not allowed to run in the CI yet. (Only users with write permissions can do so.)

Log output

Expand to view the last 100 lines of log output

[2021-07-12T06:00:33.599Z] Pruning obsolete local branches
[2021-07-12T06:00:32.700Z] Using reference repository: /var/lib/jenkins/.git-references/apm-agent-java.git
[2021-07-12T06:00:32.705Z] Fetching upstream changes from https://github.com/elastic/apm-agent-java.git
[2021-07-12T06:00:32.706Z]  > git --version # timeout=10
[2021-07-12T06:00:32.721Z]  > git --version # 'git version 2.17.1'
[2021-07-12T06:00:32.721Z] using GIT_ASKPASS to set credentials GitHub user @apmmachine User + Personal Access Token
[2021-07-12T06:00:32.724Z]  > git fetch --no-tags --progress -- https://github.com/elastic/apm-agent-java.git +refs/heads/*:refs/remotes/origin/* # timeout=100
[2021-07-12T06:00:33.570Z]  > git config remote.origin.url https://github.com/elastic/apm-agent-java.git # timeout=10
[2021-07-12T06:00:33.576Z]  > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10
[2021-07-12T06:00:33.587Z]  > git config remote.origin.url https://github.com/elastic/apm-agent-java.git # timeout=10
[2021-07-12T06:00:33.605Z] Fetching upstream changes from https://github.com/elastic/apm-agent-java.git
[2021-07-12T06:00:33.605Z] using GIT_ASKPASS to set credentials GitHub user @apmmachine User + Personal Access Token
[2021-07-12T06:00:33.606Z]  > git fetch --no-tags --progress --prune -- https://github.com/elastic/apm-agent-java.git +refs/pull/1905/head:refs/remotes/origin/PR-1905 +refs/heads/master:refs/remotes/origin/master # timeout=100
[2021-07-12T06:00:34.082Z] Merging remotes/origin/master commit e49290672ebb8ed56e80e3ddc076327477299ab2 into PR head commit 60b1a10a3d946336c9790cc285a76f7b8cde8f60
[2021-07-12T06:00:34.357Z] Merge succeeded, producing 60b1a10a3d946336c9790cc285a76f7b8cde8f60
[2021-07-12T06:00:34.357Z] Checking out Revision 60b1a10a3d946336c9790cc285a76f7b8cde8f60 (PR-1905)
[2021-07-12T06:00:34.089Z]  > git config core.sparsecheckout # timeout=10
[2021-07-12T06:00:34.095Z]  > git checkout -f 60b1a10a3d946336c9790cc285a76f7b8cde8f60 # timeout=100
[2021-07-12T06:00:34.320Z]  > git remote # timeout=10
[2021-07-12T06:00:34.330Z]  > git config --get remote.origin.url # timeout=10
[2021-07-12T06:00:34.336Z] using GIT_ASKPASS to set credentials GitHub user @apmmachine User + Personal Access Token
[2021-07-12T06:00:34.337Z]  > git merge e49290672ebb8ed56e80e3ddc076327477299ab2 # timeout=10
[2021-07-12T06:00:34.347Z]  > git rev-parse HEAD^{commit} # timeout=10
[2021-07-12T06:00:34.362Z]  > git config core.sparsecheckout # timeout=10
[2021-07-12T06:00:34.377Z]  > git checkout -f 60b1a10a3d946336c9790cc285a76f7b8cde8f60 # timeout=100
[2021-07-12T06:00:38.365Z] Commit message: "Merge branch 'master' into fix-1904"
[2021-07-12T06:00:38.369Z]  > git rev-list --no-walk 1ee8f1121c995b7fb51f94ea425e31012aba5b36 # timeout=10
[2021-07-12T06:00:39.020Z] Masking supported pattern matches of $JOB_GCS_BUCKET or $NOTIFY_TO
[2021-07-12T06:00:39.061Z] Timeout set to expire in 1 hr 0 min
[2021-07-12T06:00:39.074Z] The timestamps step is unnecessary when timestamps are enabled for all Pipeline builds.
[2021-07-12T06:00:39.317Z] [INFO] Number of builds to be searched 10
[2021-07-12T06:00:39.625Z] [INFO] 'shallow' is forced to be disabled when running on PullRequests
[2021-07-12T06:00:39.639Z] Running in /var/lib/jenkins/workspace/_java_apm-agent-java-mbp_PR-1905/src/github.com/elastic/apm-agent-java
[2021-07-12T06:00:39.658Z] [INFO] gitCheckout: Checkout SCM PR-1905 with some customisation.
[2021-07-12T06:00:39.682Z] [INFO] Override default checkout
[2021-07-12T06:00:39.718Z] Sleeping for 10 sec
[2021-07-12T06:00:49.728Z] The recommended git tool is: git
[2021-07-12T06:00:49.841Z] using credential 2a9602aa-ab9f-4e52-baf3-b71ca88469c7-UserAndToken
[2021-07-12T06:00:49.853Z] Cloning the remote Git repository
[2021-07-12T06:00:49.867Z] Cloning repository https://github.com/elastic/apm-agent-java.git
[2021-07-12T06:00:49.892Z]  > git init /var/lib/jenkins/workspace/_java_apm-agent-java-mbp_PR-1905/src/github.com/elastic/apm-agent-java # timeout=10
[2021-07-12T06:00:49.905Z] Using reference repository: /var/lib/jenkins/.git-references/apm-agent-java.git
[2021-07-12T06:00:49.905Z] Fetching upstream changes from https://github.com/elastic/apm-agent-java.git
[2021-07-12T06:00:49.905Z]  > git --version # timeout=10
[2021-07-12T06:00:49.912Z]  > git --version # 'git version 2.17.1'
[2021-07-12T06:00:49.913Z] using GIT_ASKPASS to set credentials GitHub user @apmmachine User + Personal Access Token
[2021-07-12T06:00:49.914Z]  > git fetch --tags --progress -- https://github.com/elastic/apm-agent-java.git +refs/heads/*:refs/remotes/origin/* # timeout=10
[2021-07-12T06:00:50.525Z]  > git config remote.origin.url https://github.com/elastic/apm-agent-java.git # timeout=10
[2021-07-12T06:00:50.528Z]  > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10
[2021-07-12T06:00:50.535Z]  > git config remote.origin.url https://github.com/elastic/apm-agent-java.git # timeout=10
[2021-07-12T06:00:50.542Z] Fetching upstream changes from https://github.com/elastic/apm-agent-java.git
[2021-07-12T06:00:50.543Z] using GIT_ASKPASS to set credentials GitHub user @apmmachine User + Personal Access Token
[2021-07-12T06:00:50.544Z]  > git fetch --tags --progress -- https://github.com/elastic/apm-agent-java.git +refs/pull/1905/head:refs/remotes/origin/PR-1905 +refs/heads/master:refs/remotes/origin/master # timeout=10
[2021-07-12T06:00:51.083Z] Checking out Revision 60b1a10a3d946336c9790cc285a76f7b8cde8f60 (origin/PR-1905)
[2021-07-12T06:00:51.270Z] Commit message: "Merge branch 'master' into fix-1904"
[2021-07-12T06:00:51.078Z]  > git rev-parse origin/PR-1905^{commit} # timeout=10
[2021-07-12T06:00:51.086Z]  > git config core.sparsecheckout # timeout=10
[2021-07-12T06:00:51.090Z]  > git checkout -f 60b1a10a3d946336c9790cc285a76f7b8cde8f60 # timeout=10
[2021-07-12T06:00:52.017Z] Masking supported pattern matches of $GIT_USERNAME or $GIT_PASSWORD
[2021-07-12T06:00:52.480Z] + git fetch https://****:****@github.com/elastic/apm-agent-java.git +refs/pull/*/head:refs/remotes/origin/pr/*
[2021-07-12T06:00:53.470Z] Running in /var/lib/jenkins/workspace/_java_apm-agent-java-mbp_PR-1905/src/github.com/elastic/apm-agent-java/.git
[2021-07-12T06:00:53.528Z] Archiving artifacts
[2021-07-12T06:00:54.180Z] + git rev-parse HEAD
[2021-07-12T06:00:54.504Z] + git rev-parse HEAD
[2021-07-12T06:00:54.811Z] + git rev-parse origin/pr/1905
[2021-07-12T06:00:54.852Z] [INFO] githubEnv: Found Git Build Cause: pr
[2021-07-12T06:00:55.066Z] Masking supported pattern matches of $GITHUB_TOKEN
[2021-07-12T06:00:55.811Z] [WARN] githubApiCall: The REST API call https://api.github.com/repos/elastic/apm-agent-java/pulls/1905/reviews return 0 elements
[2021-07-12T06:00:55.841Z] [INFO] githubPrCheckApproved: Title: Added support for setting destination.address/port on JMS (client) spans who use ActiveMQ Artemis as underlying technology - User: tobiasstadler - Author Association: CONTRIBUTOR
[2021-07-12T06:00:56.076Z] ERROR: githubPrCheckApproved: The PR is not allowed to run in the CI yet
[2021-07-12T06:00:56.076Z] ERROR: githubPrCheckApproved: The PR is not allowed to run in the CI yet. (Only users with write permissions can do so.)
[2021-07-12T06:00:56.109Z] [INFO] Let's stop build #2. The PR is not allowed to run in the CI yet
[2021-07-12T06:00:56.123Z] Sleeping for 5 sec
[2021-07-12T06:00:57.045Z] Stage "Build" skipped due to earlier failure(s)
[2021-07-12T06:00:57.117Z] Stage "Tests" skipped due to earlier failure(s)
[2021-07-12T06:00:57.172Z] Stage "Unit Tests" skipped due to earlier failure(s)
[2021-07-12T06:00:57.174Z] Stage "Smoke Tests 01" skipped due to earlier failure(s)
[2021-07-12T06:00:57.175Z] Stage "Smoke Tests 02" skipped due to earlier failure(s)
[2021-07-12T06:00:57.176Z] Stage "Benchmarks" skipped due to earlier failure(s)
[2021-07-12T06:00:57.177Z] Stage "Javadoc" skipped due to earlier failure(s)
[2021-07-12T06:00:57.235Z] Failed in branch Unit Tests
[2021-07-12T06:00:57.236Z] Failed in branch Smoke Tests 01
[2021-07-12T06:00:57.236Z] Failed in branch Smoke Tests 02
[2021-07-12T06:00:57.237Z] Failed in branch Benchmarks
[2021-07-12T06:00:57.238Z] Failed in branch Javadoc
[2021-07-12T06:00:57.291Z] Stage "Integration Tests" skipped due to earlier failure(s)
[2021-07-12T06:00:57.323Z] Stage "Stable" skipped due to earlier failure(s)
[2021-07-12T06:00:57.357Z] Stage "AfterRelease" skipped due to earlier failure(s)
[2021-07-12T06:00:57.374Z] Stage "AfterRelease" skipped due to earlier failure(s)
[2021-07-12T06:00:57.675Z] Running on worker-1095690 in /var/lib/jenkins/workspace/_java_apm-agent-java-mbp_PR-1905
[2021-07-12T06:00:57.769Z] [INFO] getVaultSecret: Getting secrets
[2021-07-12T06:00:57.810Z] Masking supported pattern matches of $VAULT_ADDR or $VAULT_ROLE_ID or $VAULT_SECRET_ID
[2021-07-12T06:00:59.819Z] + chmod 755 generate-build-data.sh
[2021-07-12T06:00:59.819Z] + ./generate-build-data.sh https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-java/apm-agent-java-mbp/PR-1905/ https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-java/apm-agent-java-mbp/PR-1905/runs/2 ABORTED 175902
[2021-07-12T06:00:59.819Z] INFO: curl https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-java/apm-agent-java-mbp/PR-1905/runs/2/steps/?limit=10000 -o steps-info.json
[2021-07-12T06:01:00.517Z] INFO: curl https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-java/apm-agent-java-mbp/PR-1905/runs/2/tests/?status=FAILED -o tests-errors.json
[2021-07-12T06:01:01.218Z] Retry 1/3 exited 22, retrying in 1 seconds...
[2021-07-12T06:01:02.667Z] Retry 2/3 exited 22, retrying in 2 seconds...
[2021-07-12T06:01:04.921Z] Retry 3/3 exited 22, no more retries left.
[2021-07-12T06:01:04.921Z] INFO: curl https://apm-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/apm-agent-java/apm-agent-java-mbp/PR-1905/runs/2/log/ -o pipeline-log.txt

@eyalkoren
Copy link
Contributor

Thanks a lot for your awesome contributions @tobiasstadler !!
FYI, we very recently extended our public API to enable setting destination.address and destination.port (#1788), which would make this PR another great candidate for an external plugin. Would you like to convert this one as well?
This addition was not yet released, but in the mean time you can test it by using the latest snapshot.

@eyalkoren eyalkoren closed this Jul 11, 2021
@eyalkoren eyalkoren reopened this Jul 11, 2021
@eyalkoren
Copy link
Contributor

(didn't mean to close - wrong button 🤦 )

@tobiasstadler
Copy link
Contributor Author

@eyalkoren Personally, I prefer having this in tree. But I am also fine with me maintaining this as an external plugin, if you prefer that.

@tobiasstadler
Copy link
Contributor Author

BTW. Do you already know when 1.25.0 will be released?

@eyalkoren
Copy link
Contributor

Personally, I prefer having this in tree.

Why? What's the benefit for you? I'm interested because we want to make our external-plugin contribution experience at least as easy to use/maintain, so any feedback that can help us improve would be great

I am also fine with me maintaining this as an external plugin, if you prefer that

We do prefer that in this case. Note that these external plugins can still be publicly available to anyone interested and we highly encourage that.
Keep your external plugins on a dedicated clean GitHub repo and we'll make sure to find a proper way to publish them. In the mean time, you can link them from https://github.com/elastic/apm-contrib/tree/master/apm-agent-java/frameworks, so that we can take it from there once we decide on a proper location in our main repo and/or docs.

BTW. Do you already know when 1.25.0 will be released?

Not yet, not this week, but hopefully in the next week or two.

Thanks again for your awesome contributions and feedback!

@tobiasstadler
Copy link
Contributor Author

The are two reasons why I prefer having it in tree:

  • If it is in your repo you will do the maintenance (if necessary) ;-)
  • Without co.elastic.apm.agent.AbstractInstrumentationTest (wich is in elastic-apm-core and not published to Maven Central) testing the plugin is way more effort

@tobiasstadler
Copy link
Contributor Author

Ok, I will create an external plugin.

@tobiasstadler tobiasstadler deleted the fix-1904 branch July 12, 2021 07:36
@eyalkoren
Copy link
Contributor

Without co.elastic.apm.agent.AbstractInstrumentationTest (wich is in elastic-apm-core and not published to Maven Central) testing the plugin is way more effort

In case you are not familiar with it already, check out our external plugin example, it does show how to use our tests utilities. In this plugin module there is an example for a unit test, and if you go one level up you can see how we do an integration test for this demo plugin.
I hope this can assist.

Worth noticing: the external plugin mechanism is still experimental, so things may change. However, the intention is that any external plugin you develop now will continue working with future releases, even if we change the API or move to a different approach.

@tobiasstadler
Copy link
Contributor Author

The external plugin uses apm-agent-core as a test dependency. As apm-agent-core is not on maven central (at least I didn't find it there) I have to build it manually first (with the right version) before I can build the plugin. This makes it kinda hard to use CI.

@eyalkoren
Copy link
Contributor

It can still be developed within your fork of the Java agent.
What would be the ideal infrastructure that facilitates external plugin development in your view?

@tobiasstadler
Copy link
Contributor Author

FYI: You can find the external plugin here: https://github.com/tobiasstadler/apm-activemq-artemis-destination-plugin

@felixbarny
Copy link
Member

apm-agent-core is not on maven central

Good catch, I wasn't aware of that. Sounds like we should publish it by removing this line:

<maven-deploy-plugin.skip>true</maven-deploy-plugin.skip>

@tobiasstadler
Copy link
Contributor Author

Although publishing apm-agent-core would solve my problem, I don't know if it is a good idea to do so (from your perspective ). Publishing apm-agent-core also means "exposing" the internal implementation to plugin developers.

At the moment I am using an apm-server that is mocked by mock-server (similar to your integration-tests) to capture and validate the created transactions, spans and errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for destination.address/port in ActiveMQ Artemis Client
4 participants