Skip to content

Conversation

@jaggederest
Copy link
Contributor

@jaggederest jaggederest commented Jun 2, 2022

What does this pull request do?

This adds a version method to the Config class, which internally checks the server for the value of it's version in the configuration data returned from the root url

Why is it important?

This will be used to gate some features based on server version, most directly elastic/apm#545

Checklist

  • I have signed the Contributor License Agreement.
  • My code follows the style guidelines of this project (See .rubocop.yml)
  • I have rebased my changes on top of the latest main branch
  • 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

Related issues

Closes #1274

@ghost
Copy link

ghost commented Jun 2, 2022

💚 Build Succeeded

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

Expand to view the summary

Build stats

  • Start Time: 2022-06-03T09:19:12.110+0000

  • Duration: 32 min 14 sec

Test stats 🧪

Test Results
Failed 0
Passed 44393
Skipped 80
Total 44473

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@ghost
Copy link

ghost commented Jun 2, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 99.213% (126/127) 👍 0.006
Classes 99.213% (126/127) 👍 0.006
Lines 59.678% (2519/4221) 👎 -0.027
Conditionals 100.0% (0/0) 💚
Copy link
Contributor

@estolfo estolfo left a comment

Choose a reason for hiding this comment

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

Looks good!
We should think about whether we want to get the version lazily or explicitly at agent start.

@estolfo
Copy link
Contributor

estolfo commented Jun 3, 2022

And just a reminder to put this into non-Draft mode to signal that your work is complete on it.

@jaggederest jaggederest marked this pull request as ready for review June 3, 2022 09:19
@jaggederest jaggederest enabled auto-merge (squash) June 3, 2022 09:19
@felixbarny
Copy link
Member

We should think about whether we want to get the version lazily or explicitly at agent start.

Ideally, querying the version should not block both the application startup and the agent startup. In other words, it's fine if the agent starts creating transactions and we only know about the server version some time later.

@jaggederest jaggederest merged commit 892a0ce into main Jun 3, 2022
@jaggederest jaggederest deleted the 1274-query-apm-server-for-version-and-details-about-supported-features branch June 3, 2022 09:51
estolfo pushed a commit that referenced this pull request Mar 2, 2023
) * Initial pass at server version checking * Testing server version checking * Update changelog for #1278
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants