-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Consolidate Elastic APM dependencies #162686
Conversation
Pinging @elastic/apm-ui (Team:APM) |
Pinging @elastic/kibana-operations (Team:Operations) |
It was incompatible with the newest version of the APM agent
Was there some indication of what that compat issue was? I see sorenlouv/backport#465 that locked the elastic-apm-node dep version for backport, but no indication of the issue. I suppose I could try it myself. :) |
packages/kbn-apm-synthtrace/src/lib/apm/client/elastic_apm_http_client.d.ts
Outdated
Show resolved
Hide resolved
I've opened sorenlouv/backport#467 to have it support the latest elastic-apm-node version. |
@trentm Søren is on PTO, so I'll prefer merging this one now and then just make a follow up PR once he's back and can address your PR. @elastic/apm-ui Can I please get a review from one of you with the recent changes? |
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @watson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APM changes LGTM 👍🏼
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
The backports is turning out to be too complicated and multiple PRs needs to be backported as one unit. I'll do them manually instead and remove the label |
APM Synthtrace added a direct dependency to
elastic-apm-http-client
in #136530. This wasn't kept up to date with the client used by the mainelastic-apm-node
dependency when that was recently upgraded.This PR fixes that and also consolidates theelastic-apm-node
dependency to only have one version (the other was depended upon by thebacktrace
dependency, which luckily is only used in development).Update 1: There were a compatibility issue with the newest version of
elastic-apm-node
andbackport
, so I had to slightly walk this one back. I've updatedbackport
as well to meet half way.Update 2: After the discussion below, I've removed the APM Synthtrace dependency on
elastic-apm-http-client
as it was no longer usedThe only breaking change in theelastic-apm-http-client
upgrade is that thehostname
config property is now namedconfiguredHostname
according to theCHANGELOG.md
. So I think the change I made is sufficient. But we should probably get someone with knowledge of APM Synthtrace to review.