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

Add filebeat receiver to otel mode #5672

Merged
merged 7 commits into from
Oct 3, 2024

Conversation

leehinman
Copy link
Contributor

What does this PR do?

Adds filebeat receiver as an otel receiver.

Why is it important?

This is the first beat receiver, and this will allow us to test the feasibility of running beats as otel receivers.

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/fragments using the changelog tool
  • I have added an integration test or an E2E test

Disruptive User Impact

should be 0 user impact of existing configurations

How to test this PR locally

Use something like the following as otel.yml, and run elastic-agent otel --config otel.yml
This will run the beat receiver, which will produce one event.
The event should be written to the path you specify in the file/no_rotation section.

receivers:
  filebeatreceiver:
    filebeat:
      inputs:
        - type: benchmark
          enabled: true
          count: 1
          message: "The quick brown fox jumped over the lazy dog"
    output:
      otelconsumer:
    logging:
      level: info
      selectors:
        - '*'
    path:
      home: /tmp/<state dir>
exporters:
  file/no_rotation:
    path: /tmp/<output file>
service:
  pipelines:
    logs:
      receivers: [filebeatreceiver]
      exporters: [file/norotation]

Related issues

Questions to ask yourself

  • How are we going to support this in production?
  • How are we going to measure its adoption?
  • How are we going to debug this?
  • What are the metrics I should take care of?
  • ...

@leehinman leehinman added enhancement New feature or request Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team backport-8.x Automated backport to the 8.x branch with mergify labels Oct 2, 2024
@leehinman leehinman requested a review from a team as a code owner October 2, 2024 18:56
@elasticmachine
Copy link
Contributor

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

@leehinman leehinman changed the title 3489 beat receiver2 Add filebeat receiver to otel mode Oct 2, 2024
Copy link
Contributor

mergify bot commented Oct 2, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 3489_beat_receiver2 upstream/3489_beat_receiver2
git merge upstream/main
git push upstream 3489_beat_receiver2

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Seems we where doing double work on this one #5666

You got the integration test added before me, so I am going to close mine in favor of yours.

@@ -464,3 +579,5 @@ replace (
// See https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/12d41f40b0d408b0167633d8095160d3343d46ac/go.mod#L38
github.com/openshift/api v3.9.0+incompatible => github.com/openshift/api v0.0.0-20180801171038-322a19404e37
)

replace github.com/dop251/goja => github.com/andrewkroh/goja v0.0.0-20190128172624-dd2ac4456e20
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

@mauri870 mauri870 Oct 3, 2024

Choose a reason for hiding this comment

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

I don't think relying on a fork from an employee's personal GitHub account is the right approach. I'd prefer to have these under Elastic's organization, as it would be easier to manage and maintain in the long run. According to elastic/beats#41084, we don't want to keep temporary replacements around for too long. We should make our best effort to upstream any fixes or switch to/maintain another up-to-date fork.

Copy link
Contributor

Choose a reason for hiding this comment

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

100% true. But blocking this PR for something that is already true for beats, doesn't provide forward progress. Lets push for progress and make adjustments a long the way.

Copy link
Contributor Author

@leehinman leehinman Oct 3, 2024

Choose a reason for hiding this comment

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

The sarama one was already there. Added fsnotify, we won't need it until we bring in Auditbeat, but be might as well start the pain now so we have more motivation to get those changes pushed upstream.

Copy link
Contributor

mergify bot commented Oct 3, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 3489_beat_receiver2 upstream/3489_beat_receiver2
git merge upstream/main
git push upstream 3489_beat_receiver2

@leehinman leehinman force-pushed the 3489_beat_receiver2 branch from 2b120db to 6ad88c4 Compare October 3, 2024 14:27
@leehinman
Copy link
Contributor Author

Seems we where doing double work on this one #5666

You got the integration test added before me, so I am going to close mine in favor of yours.

Sorry, didn't see that you had started. The integration test took more time than the code changes :-)

@blakerouse
Copy link
Contributor

@leehinman No big deal. You will need to add this change to your PR as well - https://github.com/elastic/elastic-agent/pull/5666/files#diff-d002edd1b34ea944ff5d1278c97e55f0b42a7240299fa6e349843687b4d1144cR21

@leehinman leehinman force-pushed the 3489_beat_receiver2 branch 2 times, most recently from 5180b3e to c3e3738 Compare October 3, 2024 18:50
@leehinman leehinman enabled auto-merge (squash) October 3, 2024 19:46
@leehinman leehinman force-pushed the 3489_beat_receiver2 branch from c3e3738 to 1f1bd01 Compare October 3, 2024 20:50
@leehinman leehinman merged commit bd219d6 into elastic:main Oct 3, 2024
14 checks passed
@blakerouse
Copy link
Contributor

🎉

mergify bot pushed a commit that referenced this pull request Oct 3, 2024
* Add filebeatreceiver to otel components

(cherry picked from commit bd219d6)

# Conflicts:
#	NOTICE.txt
#	go.mod
#	go.sum
ycombinator added a commit to ycombinator/elastic-agent that referenced this pull request Oct 4, 2024
jlind23 pushed a commit that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.x Automated backport to the 8.x branch with mergify enhancement New feature or request 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.

4 participants