[elastic-agent] initial instrumentation #29031

stuartnelson3 · 2021-11-18T10:58:53Z

What does this PR do?

Add some initial tracing to elastic-agent. I basically just looked through the code and was looking at lowhanging fruit for wrapping http handlers and clients. I'd love to have some of the EA dev's point out places I've missed that I could add more tracing, and ways to trigger these traces to view them in the UI.

todo:

more instrumenting
tests
how to configure apm output? currently apm-agent-go just using default values and sending to the apm-server integration.

Why is it important?

This will help in troubleshooting issues as more users start running EA.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
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 elastic-agent off this branch, start it under fleet, add the APM integration. the agent should appear as an apm service.

Related issues

closes #28895

Use cases

troubleshooting mis-behaving elastic-agents

Screenshots

(traces are coming from a metricbeat grabbing proxied data from apm-server)

mergify · 2021-11-18T10:59:10Z

This pull request does not have a backport label. Could you fix it @stuartnelson3? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

mergify · 2021-11-18T10:59:18Z

This pull request does not have a backport label. Could you fix it @stuartnelson3? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

x-pack/elastic-agent/pkg/agent/application/upgrade/step_download.go

elasticmachine · 2021-11-18T11:20:47Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Reason: null
Start Time: 2022-01-31T17:07:59.836+0000
Duration: 272 min 52 sec
Commit: c0b4b9e

Test stats 🧪

Test	Results
Failed	0
Passed	48027
Skipped	4292
Total	52319

💚 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!)

michel-laterman · 2021-11-18T17:37:40Z

Haven't reviewed anything yet, but this PR is exciting! It also covers some diagnostics work that's on my backlog.
I can make time to help out if needed

axw

Looking good so far!

Not too sure about the span names. Typically I'd name after higher level operations, rather than method names. Maybe that's just my bias though.

x-pack/elastic-agent/pkg/agent/application/upgrade/step_download.go

x-pack/elastic-agent/pkg/agent/cmd/run.go

x-pack/elastic-agent/pkg/agent/application/upgrade/step_download.go

stuartnelson3 · 2021-11-19T07:36:01Z

That would be much appreciated! I'm sort of flying blind at the moment as I'm not super familiar with the codebase, so any help would be great.

ruflin · 2021-11-19T14:38:33Z

This is really exciting! Thanks for taking this on.

One thing I'm curious in the context of diagnostics: Assuming someone is not shipping the apm data to ES, would it still be possible to fetch some of it as part of the diagnostics call? Does that even make sense? @axw @stuartnelson3

michel-laterman · 2021-11-19T20:44:37Z

@ruflin, I'm not entirely sure how the apm client works, I think that integration with the diagnostics command should be done as a separate PR.
I don't think the apm client has an option to write to a local file, in-memory buffer, etc. so we'll likely have to figure out how we want collect this information for diagnostics later.

axw · 2021-11-22T03:01:22Z

One thing I'm curious in the context of diagnostics: Assuming someone is not shipping the apm data to ES, would it still be possible to fetch some of it as part of the diagnostics call? Does that even make sense?

@ruflin not out of the box, but it would be possible to do something like this by implementing a custom transport. e.g. we have a transport which decodes events and stores them in memory for testing purposes: https://github.com/elastic/apm-agent-go/blob/master/transport/transporttest/recorder.go

ruflin · 2021-11-23T10:01:00Z

Very interesting @axw . Is this something that could be enabled "on-demand"? What I have in mind is something like:

User triggers elastic-agent diagnostic command
Recorder is enabled for x seconds
Recording is read out and stored on disk
Recorder is disabled again
Diagnostics finished

@michel-laterman Agree in any case to separate it out in future PR's.

simitt · 2021-11-23T13:07:51Z

@ruflin would you mind creating a dedicated issue for this kind of conversation. I believe we can isolate it and don't think we should block making progress with initial instrumentation by this.

axw · 2021-11-24T00:33:13Z

@ruflin please ping me on said issue and I'll reply there so the answers don't get lost.

ruflin · 2021-11-24T13:52:12Z

@axw elastic/elastic-agent#112

stuartnelson3 · 2022-01-24T09:00:19Z

@ph i've addressed some comments and written some follow-up questions, can you take a look when you have a chance?

also, I'm not sure what the e2e test failure is, it's apparently something with kubernetes (kubernetes_autodiscover_elastic-agent). I merged in latest master and will see what happens...

ph

This looks good to me, @stuartnelson3 @blakerouse can you have an eye on it?

stuartnelson3 · 2022-01-26T12:41:31Z

any idea what's up with the packaging failures?

[2022-01-25T19:23:45.582Z] ERRO[2022-01-25T19:23:45Z] Could not download the binary for the agent   arch=x86_64 artifact=elastic-agent error="GET request failed with 404" extension=tar.gz os=linux version=8.1.0-dbc834fd-SNAPSHOT
[2022-01-25T19:23:45.582Z] ERRO[2022-01-25T19:23:45Z] Could not download the binary for the agent   arch=x86_64 artifact=elastic-agent error="GET request failed with 404" extension=tar.gz os=linux version=8.1.0-dbc834fd-SNAPSHOT

blakerouse

Overall the change looks very mechanical, which is good to see. Made it easier to review such a large branch.

I reviewed the initTracer as that is probably the most important part. Looks good and provides unit test coverage for it.

cmacknz · 2022-01-27T14:34:12Z

any idea what's up with the packaging failures?

[2022-01-25T19:23:45.582Z] ERRO[2022-01-25T19:23:45Z] Could not download the binary for the agent   arch=x86_64 artifact=elastic-agent error="GET request failed with 404" extension=tar.gz os=linux version=8.1.0-dbc834fd-SNAPSHOT
[2022-01-25T19:23:45.582Z] ERRO[2022-01-25T19:23:45Z] Could not download the binary for the agent   arch=x86_64 artifact=elastic-agent error="GET request failed with 404" extension=tar.gz os=linux version=8.1.0-dbc834fd-SNAPSHOT

I think this might have been broken by an unrelated change on master, see #30007. I think there was a follow up fix if it's the same issue I was seeing on other PRs.

stuartnelson3 · 2022-01-27T18:28:00Z

/test

stuartnelson3 · 2022-01-28T08:18:10Z

/test

stuartnelson3 · 2022-01-28T12:58:44Z

/test

ruflin · 2022-01-31T09:37:49Z

/test

stuartnelson3 · 2022-01-31T12:33:05Z

/test

mdelapenya · 2022-02-01T08:59:12Z

e2e are failing but not caused by this PR. We @elastic/observablt-robots are working on the permanent fix (very probably here elastic/e2e-testing#2064), so I'd say this PR is fine

👍

This revert the code of the APM Instrumentation of the Elastic Agent. To unblock the build of and the CI for other team. This would require more investigation to really understand the problem. Fixes elastic/fleet-server#1129

* Revert #29031 This revert the code of the APM Instrumentation of the Elastic Agent. To unblock the build of and the CI for other team. This would require more investigation to really understand the problem. Fixes elastic/fleet-server#1129 * fix make update * fix linter

* Revert #29031 This revert the code of the APM Instrumentation of the Elastic Agent. To unblock the build of and the CI for other team. This would require more investigation to really understand the problem. Fixes elastic/fleet-server#1129 * fix make update * fix linter (cherry picked from commit 718c923)

Co-authored-by: Pier-Hugues Pellerin <[email protected]>

This reverts commit 718c923.

This revert the code of the APM Instrumentation of the Elastic Agent. To unblock the build of and the CI for other team. This would require more investigation to really understand the problem. Fixes elastic/fleet-server#1129

stuartnelson3 added 2 commits November 16, 2021 15:44

add some initial instrumentation

53cbb1b

update tests

9f960a2

stuartnelson3 added the enhancement label Nov 18, 2021

stuartnelson3 requested review from a team November 18, 2021 10:58

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 18, 2021

stuartnelson3 added the Team:Elastic-Agent Label for the Agent team label Nov 18, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 18, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 18, 2021

stuartnelson3 removed the backport-skip Skip notification from the automated backport with mergify label Nov 18, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 18, 2021

stuartnelson3 commented Nov 18, 2021

View reviewed changes

x-pack/elastic-agent/pkg/agent/application/upgrade/step_download.go Show resolved Hide resolved

stuartnelson3 added 2 commits November 18, 2021 13:38

update NOTICE.txt

97f5658

make update

234ab4e

axw reviewed Nov 19, 2021

View reviewed changes

fix order capturing the error/ending the span

8bf85e9

ruflin mentioned this pull request Mar 7, 2022

[Elastic Agent] Read out apm data for diagnostics purpose elastic/elastic-agent#112

Open

close the default tracer

1303efc

ph approved these changes Jan 25, 2022

View reviewed changes

Merge branch 'master' into instrument-elastic-agent

8d9603a

blakerouse approved these changes Jan 27, 2022

View reviewed changes

Merge branch 'master' into instrument-elastic-agent

d303e3d

Merge branch 'master' into instrument-elastic-agent

92a87e2

Merge branch 'master' into instrument-elastic-agent

c0b4b9e

stuartnelson3 merged commit 395ee91 into elastic:master Feb 1, 2022

stuartnelson3 added backport-v8.1.0 Automated backport with mergify v8.1.0 and removed backport-v8.0.0 Automated backport with mergify labels Feb 1, 2022

mtojek mentioned this pull request Feb 3, 2022

[8.1.0-SNAPSHOT] Fleet Server can't enroll: FAILED: Missed two check-ins elastic/fleet-server#1129

Closed

ph mentioned this pull request Feb 3, 2022

Revert #29031 #30197

Merged

6 tasks

ph added a commit that referenced this pull request Feb 9, 2022

[8.1](backport #30197) Revert #29031 (#30209)

706c11f

Co-authored-by: Pier-Hugues Pellerin <[email protected]>

stuartnelson3 added a commit to stuartnelson3/beats that referenced this pull request Feb 14, 2022

Revert "Revert elastic#29031 (elastic#30197)"

619d143

This reverts commit 718c923.

stuartnelson3 added a commit to stuartnelson3/beats that referenced this pull request Feb 14, 2022

Revert "Revert elastic#29031 (elastic#30197)"

466b03d

This reverts commit 718c923.

stuartnelson3 added a commit to stuartnelson3/beats that referenced this pull request Feb 14, 2022

Revert "Revert elastic#29031 (elastic#30197)"

73734ed

This reverts commit 718c923.

stuartnelson3 mentioned this pull request Feb 14, 2022

[elastic-agent] Re-introduce instrumentation #30385

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[elastic-agent] initial instrumentation #29031

[elastic-agent] initial instrumentation #29031

stuartnelson3 commented Nov 18, 2021

mergify bot commented Nov 18, 2021

mergify bot commented Nov 18, 2021

elasticmachine commented Nov 18, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

michel-laterman commented Nov 18, 2021

axw left a comment

stuartnelson3 commented Nov 19, 2021

ruflin commented Nov 19, 2021

michel-laterman commented Nov 19, 2021

axw commented Nov 22, 2021

ruflin commented Nov 23, 2021

simitt commented Nov 23, 2021

axw commented Nov 24, 2021

ruflin commented Nov 24, 2021

stuartnelson3 commented Jan 24, 2022

ph left a comment

stuartnelson3 commented Jan 26, 2022

blakerouse left a comment

cmacknz commented Jan 27, 2022

stuartnelson3 commented Jan 27, 2022

stuartnelson3 commented Jan 28, 2022

stuartnelson3 commented Jan 28, 2022

ruflin commented Jan 31, 2022

stuartnelson3 commented Jan 31, 2022

mdelapenya commented Feb 1, 2022

[elastic-agent] initial instrumentation #29031

[elastic-agent] initial instrumentation #29031

Conversation

stuartnelson3 commented Nov 18, 2021

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

mergify bot commented Nov 18, 2021

mergify bot commented Nov 18, 2021

elasticmachine commented Nov 18, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

michel-laterman commented Nov 18, 2021

axw left a comment

Choose a reason for hiding this comment

stuartnelson3 commented Nov 19, 2021

ruflin commented Nov 19, 2021

michel-laterman commented Nov 19, 2021

axw commented Nov 22, 2021

ruflin commented Nov 23, 2021

simitt commented Nov 23, 2021

axw commented Nov 24, 2021

ruflin commented Nov 24, 2021

stuartnelson3 commented Jan 24, 2022

ph left a comment

Choose a reason for hiding this comment

stuartnelson3 commented Jan 26, 2022

blakerouse left a comment

Choose a reason for hiding this comment

cmacknz commented Jan 27, 2022

stuartnelson3 commented Jan 27, 2022

stuartnelson3 commented Jan 28, 2022

stuartnelson3 commented Jan 28, 2022

ruflin commented Jan 31, 2022

stuartnelson3 commented Jan 31, 2022

mdelapenya commented Feb 1, 2022

elasticmachine commented Nov 18, 2021 •

edited by jenkins-beats-ci bot

Loading