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 support for skipping Go tracers #451

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Conversation

grcevski
Copy link
Contributor

During some refactoring for multi-process support we had broken the debug option BEYLA_SKIP_GO_SPECIFIC_TRACERS, which can come in handy when debugging issues with Go instrumentation. For example, we may fail to parse something or work with older Go version that is unsupported.

This fixes the support for the option and adds an integration test so we don't accidentally break it again.

@grcevski grcevski requested a review from mariomac as a code owner November 17, 2023 15:41
@codecov-commenter
Copy link

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e2c0c51) 78.26% compared to head (1c21a19) 65.89%.
Report is 1 commits behind head on main.

Files Patch % Lines
pkg/internal/discover/attacher.go 40.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #451       +/-   ##
===========================================
- Coverage   78.26%   65.89%   -12.38%     
===========================================
  Files          61       58        -3     
  Lines        5028     4937       -91     
===========================================
- Hits         3935     3253      -682     
- Misses        882     1418      +536     
- Partials      211      266       +55     
Flag Coverage Δ
integration-test 65.89% <40.00%> (+0.14%) ⬆️
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mariomac mariomac left a comment

Choose a reason for hiding this comment

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

Hmmm strange. I thought that skipping go specific tracers in the previous pipeline stage (typer) would be enough.

Thanks for finding the issue and fixing it!

@mariomac
Copy link
Contributor

I'd also backport the issue to release-1.0.

@grcevski
Copy link
Contributor Author

I'd also backport the issue to release-1.0.

Sounds good!

@grcevski grcevski merged commit 1b094ce into grafana:main Nov 20, 2023
3 checks passed
@grcevski grcevski deleted the skip_go_tracers branch November 20, 2023 14:56
grcevski added a commit to grcevski/ebpf-autoinstrument that referenced this pull request Nov 20, 2023
grcevski added a commit that referenced this pull request Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants