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

Reapply single-PID adapation for metricbeat's system/process, revert update to go-systemd #39730

Merged
merged 4 commits into from
May 31, 2024

Conversation

fearful-symmetry
Copy link
Contributor

Proposed commit message

This reverts #39714

Which in turn reverted #39620

That original PR somehow ended up containing an update to coreos/go-systemd

That newer version of go-systemd contains this file: https://github.com/coreos/go-systemd/blob/v22.5.0/internal/dlopen/dlopen_example.go

Which is copied from coreos/pkg: https://github.com/coreos/pkg/blob/bbd7aa9bf6fb51acc905bd45a5363ebecf065f30/dlopen/dlopen_example.go

When the original PR was merged into beats, this caused a linker error in agentbeat, as agentbeat will import both the go-systemd and coreos/pkg libraries, resulting in two definitions of my_strlen from those two identical files.

For now, this just reverts the update to go-systemd, which isn't actually needed.

What I'm still not sure about is why this wasn't caught my CI. Do we not have anything in CI that packages agentbeat? Note that hitting this bug requires a few extra args during the build process:

args.ExtraFlags = append(args.ExtraFlags, "-tags=agentbeat,withjournald")

Going to manually test this with agent/agentbeat.

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.

@fearful-symmetry fearful-symmetry added the Team:Elastic-Agent Label for the Agent team label May 24, 2024
@fearful-symmetry fearful-symmetry self-assigned this May 24, 2024
@fearful-symmetry fearful-symmetry requested review from a team as code owners May 24, 2024 15:44
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

@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 May 24, 2024
Copy link
Contributor

mergify bot commented May 24, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @fearful-symmetry? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@cmacknz
Copy link
Member

cmacknz commented May 24, 2024

What I'm still not sure about is why this wasn't caught my CI. Do we not have anything in CI that packages agentbeat? Note that hitting this bug requires a few extra args during the build process:

It might be the case that the conditional build triggers don't properly account for agentbeat.

@fearful-symmetry
Copy link
Contributor Author

Alright, confirmed agentbeat builds with "-tags=agentbeat,withjournald" on this branch.

@ycombinator ycombinator added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label May 24, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@pierrehilbert pierrehilbert requested review from leehinman and removed request for AndersonQ May 25, 2024 16:08
@cmacknz
Copy link
Member

cmacknz commented May 27, 2024

At first glance it doesn't look like CI is actually building agentbeat with these changes.

I think we need to update CI to compile x-pack/agentbeat when any of the individual Beat pipelines is triggered. That is likely why this error was missed the first time.

@fearful-symmetry
Copy link
Contributor Author

@cmacknz do we want to rope CI changes into this PR?

@fearful-symmetry fearful-symmetry merged commit d77596a into elastic:main May 31, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants