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

[Elastic Agent] Keep debug symbols, no inline and no optimisations for dev builds #29961

Merged
merged 1 commit into from
Jan 26, 2022

Conversation

AndersonQ
Copy link
Member

What does this PR do?

When the agent is built for development it keeps the debug symbols, do not inline nor optimize so it's possible to debug using delve.

It also fixes a typo.

Why is it important?

Improves developer experience by making it ready for debug when building for development.

Checklist

  • [ x] My code follows the style guidelines of this project
  • [ x] I have commented my code, particularly in hard-to-understand areas
  • [ x] I have made corresponding changes to the documentation
  • [ x] I have made corresponding change to the default configuration files
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Build with DEV=true, then run it with delve and connect to remote debug

Using GoLand, you can run the elastic-agent as below and use the Go Remote to Run/Debug the agent.

dlv --listen=:2345 --headless=true --api-version=2 --accept-multiclient exec ./elastic-agent

Related issues

@AndersonQ AndersonQ added enhancement backport-skip Skip notification from the automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.1.0 labels Jan 24, 2022
@AndersonQ AndersonQ self-assigned this Jan 24, 2022
@AndersonQ AndersonQ requested a review from a team as a code owner January 24, 2022 10:08
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jan 24, 2022
@jlind23 jlind23 requested a review from a team January 25, 2022 10:11
@jlind23
Copy link
Collaborator

jlind23 commented Jan 25, 2022

@AndersonQ is this one good to be reviewed?

@AndersonQ
Copy link
Member Author

/test metricbeat-windows-2016-windows-2016

@AndersonQ
Copy link
Member Author

@jlind23 yes, it's ready for review. The failing tests seem to be unrelated to my changes. But I'm looking at them now

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jan 25, 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-01-25T15:55:32.916+0000

  • Duration: 31 min 27 sec

  • Commit: 7a03a0c

Test stats 🧪

Test Results
Failed 0
Passed 706
Skipped 241
Total 947

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@ph ph self-requested a review January 25, 2022 13:58
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

Changes LGTM, I've looked at the test suite and this looks like an issue with the python environment on windows. Two possible reasons:

  1. The Windows images changed ? @v1v
  2. We are missing the venv python module in our requirements https://docs.python.org/3/library/venv.html

Maybe both are true?

@v1v
Copy link
Member

v1v commented Jan 25, 2022

The runtime configuration script to prepare the context failed but it didn't exit 1, therefore it ran the mage build/test

[2022-01-25T11:08:13.948Z] Failures
[2022-01-25T11:08:13.948Z]  - python (exited 1) - python not installed. An error occurred during installation:
[2022-01-25T11:08:13.948Z]  The operation has timed out

https://beats-ci.elastic.co/blue/organizations/jenkins/Beats%2Fbeats%2FPR-29961/detail/PR-29961/2/pipeline/#step-3696-log-39

The behaviour then fallbacks to the current installed software in the Worker, and it seems python2 is the default one.

For the moment, just type /test again, and I'll raise an issue to track the error and then figure out why it didn't return error 1 but kept running

@ph
Copy link
Contributor

ph commented Jan 25, 2022

/test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify enhancement Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants