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

Enable APM tracing by default for stack up #1076

Closed
wants to merge 1 commit into from

Conversation

joshdover
Copy link

@joshdover joshdover commented Dec 17, 2022

Turns on tracing by default for all components

image

TODO:

  • Figure out how to enable tracing for managed Agent
    • Setting agent.monitoring.tracing and agent.monitoring.apm settings in elastic-agent.yml didn't work
  • Cleanup Kibana configs, prefer env vars over yaml if possible
  • Log correlation for Fleet Server and Agent logs (which are already collected)
  • Setup certs & https for APM for RUM support

Things that could be improved in future PRs:

  • Agent and Fleet Server should allow enabling tracing via env vars, rather than depending on yaml or policy configuration
  • Trace context propagation from Agent to Fleet Server
  • Log collection and correlation for Kibana, Elasticsearch, and Package Registry

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 17, 2022

💔 Build Failed

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-12-17T00:46:47.083+0000

  • Duration: 40 min 30 sec

Test stats 🧪

Test Results
Failed 0
Passed 560
Skipped 0
Total 560

Steps errors 4

Expand to view the steps failures

Check
  • Took 4 min 38 sec . View more details here
  • Description: make install test-check-packages-with-kind check-git-clean
Check
  • Took 31 min 5 sec . View more details here
  • Description: make install test-check-packages-with-custom-agent check-git-clean
Build elastic-package
  • Took 3 min 59 sec . View more details here
  • Description: make PACKAGE_UNDER_TEST=aws test-check-packages-parallel
Build elastic-package
  • Took 3 min 55 sec . View more details here
  • Description: make PACKAGE_UNDER_TEST=nginx test-check-packages-parallel

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Collaborator

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (31/31) 💚
Files 67.48% (83/123) 👍 0.292
Classes 62.011% (111/179) 👍 0.055
Methods 46.176% (326/706) 👎 -2.529
Lines 29.556% (2999/10147) 👎 -2.37
Conditionals 100.0% (0/0) 💚

@ruflin
Copy link
Member

ruflin commented Dec 20, 2022

++ on having apm enabled in elastic-package by default. I wonder if it would be cleaned to run a separate elastic-agent with the apm-integration.

@jsoriano
Copy link
Member

I wonder if we should make this somehow optional. This is not required for package development, but adds a source of flakiness.

+1 to do it with a separate agent, during testing elastic-package is going to replace the policies used by the default agent, so APM will be likely disabled.

@joshdover
Copy link
Author

Agree on using a separate agent and making it optional via the existing --services parameter. I think this separate agent should also run the Kibana and Elasticsearch integrations with shared volumes to capture logs from ES and Kibana. Any opposition to that?

@jsoriano
Copy link
Member

jsoriano commented Dec 20, 2022

via the existing --services parameter.

We don't have any other optional service now (everything is started if the flag is not provided), but this could be an option, yes.

I think this separate agent should also run the Kibana and Elasticsearch integrations with shared volumes to capture logs from ES and Kibana. Any opposition to that?

Sounds good, if this is also optional, and not used to test the Kibana and Elasticsearch integrations.

@florianl
Copy link
Member

florianl commented Feb 6, 2023

elastic/elastic-package comes in handy for full system integration and for local development. Having an option to easily enable APM would really come handy.

@sharbuz
Copy link
Contributor

sharbuz commented Jul 10, 2023

Hi everyone,
Please update the branch because we merged an important change to the main branch: #1347

@graphaelli
Copy link
Member

superseded by #1598

@graphaelli graphaelli closed this Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants