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

Make additional OTel Collector components available in Agent running in OTel mode #4623

Closed
wants to merge 14 commits into from

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Apr 25, 2024

What does this PR do?

This PR makes the following additional OpenTelemetry Collector components available when Elastic Agent is running in otel mode:

Why is it important?

To allow users of Elastic Agent running as as OpenTelemetry Collector to configure pipelines containing the aforementioned components.

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

@ycombinator ycombinator requested a review from a team as a code owner April 25, 2024 21:35
@ycombinator ycombinator added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team Team:Elastic-Agent Label for the Agent team labels Apr 25, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@ycombinator ycombinator added backport-v8.14.0 Automated backport with mergify enhancement New feature or request labels Apr 25, 2024
@ycombinator
Copy link
Contributor Author

As requested by @strawgate for 8.15.0.

@strawgate
Copy link
Contributor

strawgate commented Apr 26, 2024

We may also want to add resourcedetection and filter

@ycombinator ycombinator changed the title Make elasticsearch exporter available in Agent running in OTel mode Make additional OTel components available in Agent running in OTel mode Apr 26, 2024
@ycombinator ycombinator marked this pull request as draft April 26, 2024 15:00
@ycombinator ycombinator changed the title Make additional OTel components available in Agent running in OTel mode Make additional OTel Collector components available in Agent running in OTel mode Apr 26, 2024
@ycombinator ycombinator marked this pull request as ready for review April 26, 2024 15:31
@michalpristas
Copy link
Contributor

maybe not in this PR but we will need to replace memorylimiterprocessor with memorylimiterextension.
difference is behavior is that processors drops the events and extensions acts as a pushback mechanism stopping/pausing processing on receiver side

go 1.21
go 1.21.0

toolchain go1.21.8
Copy link
Contributor

@michalpristas michalpristas Apr 29, 2024

Choose a reason for hiding this comment

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

go version is specified as 1.21.9 maybe we should match and update "bump go version" to update go.mod as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed (manually) in 176df27.

Put up #4636 to automatically keep the toolchain version here in sync with the contents of .go-version.

@ycombinator
Copy link
Contributor Author

maybe not in this PR but we will need to replace memorylimiterprocessor with memorylimiterextension. difference is behavior is that processors drops the events and extensions acts as a pushback mechanism stopping/pausing processing on receiver side

@strawgate thoughts on this? Are you okay with us switching to the memorylimiterextension?

go.mod Outdated
github.com/open-telemetry/opentelemetry-collector-contrib/exporter/fileexporter v0.98.0
github.com/open-telemetry/opentelemetry-collector-contrib/processor/attributesprocessor v0.98.0
github.com/open-telemetry/opentelemetry-collector-contrib/processor/filterprocessor v0.99.0
Copy link
Contributor

Choose a reason for hiding this comment

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

there will be a conflict: #4638

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's wait to get #4638 merged first since that's a more important change IMO and then I'll rebase this PR on main.

Copy link
Contributor

mergify bot commented May 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 otel-mode-es-exporter upstream/otel-mode-es-exporter
git merge upstream/main
git push upstream otel-mode-es-exporter

@strawgate
Copy link
Contributor

maybe not in this PR but we will need to replace memorylimiterprocessor with memorylimiterextension. difference is behavior is that processors drops the events and extensions acts as a pushback mechanism stopping/pausing processing on receiver side

@strawgate thoughts on this? Are you okay with us switching to the memorylimiterextension?

@ycombinator I don't think i have any concerns so if you dont have any concerns then let's proceed

@strawgate
Copy link
Contributor

strawgate commented May 2, 2024

If we can get hostmetrics and elasticsearch exporter for 8.14, that's a win in my book.

So 2 with a big hope that it's not one of those two

@ycombinator
Copy link
Contributor Author

ycombinator commented May 3, 2024

Thanks @strawgate. Trying out just hostmetricsreceiver and elasticsearchexporter in a separate PR: #4664. Not seeing the dependency conflict in go.mod with just these two 👍.

Will layer on the other components from this PR one by one so we can get as many in as possible without the dependency conflict.

@ycombinator
Copy link
Contributor Author

maybe not in this PR but we will need to replace memorylimiterprocessor with memorylimiterextension. difference is behavior is that processors drops the events and extensions acts as a pushback mechanism stopping/pausing processing on receiver side

Filed #4665 to track this change.

@ycombinator
Copy link
Contributor Author

It was resourcedetectionprocessor that was causing the k8s dependencies conflicts with the github.com/elastic/elastic-agent-autodiscover dependency. I've removed that from this PR now.

@ycombinator ycombinator force-pushed the otel-mode-es-exporter branch from 08f6900 to 50f1dc2 Compare May 6, 2024 19:21
@ycombinator
Copy link
Contributor Author

We have flaky integration tests failing on Windows: #4686 and #4687. @jlind23 @cmacknz since we're trying to get this change into 8.14.0 and the failing flaky tests are unrelated to the changes in this PR, are you okay to merge this PR as-is?

@ycombinator
Copy link
Contributor Author

ycombinator commented May 8, 2024

@cmacknz pointed out (in Slack) that the "The service did not respond to the start or control request in a timely fashion" error in CI isn't happening on other branches so we need to be sure one of the changes in this PR isn't causing this error.

I'm going to keep this PR open for now but create three additional PRs, one for each of the OTel Collector components being added to Agent. That way we can hopefully narrow down which component is causing the error and also get the other components in.

@ycombinator
Copy link
Contributor Author

ycombinator commented May 8, 2024

I'm going to keep this PR open for now but create three additional PRs, one for each of the OTel Collector components being added to Agent. That way we can hopefully narrow down which component is causing the error and also get the other components in.

@ycombinator
Copy link
Contributor Author

Looks like it might be the hostmetricsreceiver that's the cause for the CI errors (isolated via #4706).

Copy link
Contributor

mergify bot commented May 8, 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 otel-mode-es-exporter upstream/otel-mode-es-exporter
git merge upstream/main
git push upstream otel-mode-es-exporter

@ycombinator
Copy link
Contributor Author

Closing this PR in favor of individual PRs for the three components being added in this PR:

@ycombinator ycombinator closed this May 9, 2024
@ycombinator ycombinator deleted the otel-mode-es-exporter branch May 9, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.14.0 Automated backport with mergify enhancement New feature or request Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants