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

fix: Add wrapper around libbeat instrumentation to propagate missing tracer config #13653

Merged
merged 8 commits into from
Jul 11, 2024

Conversation

1pkg
Copy link
Member

@1pkg 1pkg commented Jul 9, 2024

Motivation/summary

This PR addresses the issue by adding a thin wrapper around libbeat instrumentation to propagate missing tracer config from elastic-agent via environmental variables.

Checklist

For functional changes, consider:

  • Is it observable through the addition of either logging or metrics?
  • Is its use being published in telemetry to enable product improvement?
  • Have system tests been added to avoid regression?

How to test these changes

Follow instructions in Building an Elastic Agent container image with a locally built APM Server and Running an Elastic Agent container with a locally built APM Server section in TESTING.md, update the relevant agent.monitoring sections in elastic-agent.yml to see the config are propagated down to tracing correctly.

Related issues

#13597
#11381
#13514

@1pkg 1pkg self-assigned this Jul 9, 2024
@1pkg 1pkg requested a review from a team as a code owner July 9, 2024 22:38
Copy link
Contributor

mergify bot commented Jul 9, 2024

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

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

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

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Jul 9, 2024
@1pkg 1pkg force-pushed the missing-apm-tracer-config-from-elastic-agent branch from f2ae5e9 to cf2d85f Compare July 9, 2024 23:55
axw
axw previously approved these changes Jul 10, 2024
Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

LGTM. Would be good (but perhaps not critical) to have a functional test for this. WDYT? Maybe open an issue to add a system test?

internal/beater/beater.go Show resolved Hide resolved
@1pkg 1pkg requested review from marclop and axw July 10, 2024 21:31
marclop
marclop previously approved these changes Jul 11, 2024
Copy link
Contributor

@marclop marclop left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for adding the test.

changelogs/8.15.asciidoc Outdated Show resolved Hide resolved
@carsonip carsonip added backport-8.15 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify labels Jul 11, 2024
changelogs/8.15.asciidoc Outdated Show resolved Hide resolved
@1pkg 1pkg enabled auto-merge (squash) July 11, 2024 16:40
@1pkg 1pkg requested a review from marclop July 11, 2024 16:40
@1pkg 1pkg changed the title feat: add wrapper to libbeat instrumentation to propagate missing tracer config via env fix: Add wrapper around libbeat instrumentation to propagate missing tracer config Jul 11, 2024
@1pkg 1pkg disabled auto-merge July 11, 2024 16:42
@1pkg 1pkg enabled auto-merge (squash) July 11, 2024 16:43
carsonip
carsonip previously approved these changes Jul 11, 2024
Copy link
Member

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

thanks for the unit test! left a few nits

changelogs/8.15.asciidoc Outdated Show resolved Hide resolved
changelogs/8.15.asciidoc Outdated Show resolved Hide resolved
internal/beater/beater.go Outdated Show resolved Hide resolved
@carsonip carsonip disabled auto-merge July 11, 2024 16:47
@1pkg 1pkg merged commit 0a57bbe into main Jul 11, 2024
14 checks passed
@1pkg 1pkg deleted the missing-apm-tracer-config-from-elastic-agent branch July 11, 2024 17:14
mergify bot pushed a commit that referenced this pull request Jul 11, 2024
…tracer config (#13653)

Add wrapper to libbeat instrumentation to propagate missing tracer
config via apm environmental variables.

(cherry picked from commit 0a57bbe)
mergify bot added a commit that referenced this pull request Jul 11, 2024
…tracer config (#13653) (#13675)

Add wrapper to libbeat instrumentation to propagate missing tracer
config via apm environmental variables.

(cherry picked from commit 0a57bbe)

Co-authored-by: Kostiantyn Masliuk <1pkg@protonmail.com>
@1pkg 1pkg mentioned this pull request Jul 11, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.15 Automated backport with mergify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants