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

Don't fail if only PROMETHEUS_PORT is set #135

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

mariomac
Copy link
Contributor

@mariomac mariomac commented Jun 7, 2023

Prometheus export is a valid export node itself, so the instrumenter should init even if no other exports are provided.

It also fixes a unit test not compiling in Darwin.

@mariomac mariomac requested a review from grcevski as a code owner June 7, 2023 12:53
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2023

Codecov Report

Merging #135 (d168f33) into main (7bbfd46) will increase coverage by 0.01%.
The diff coverage is 69.44%.

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   78.87%   78.88%   +0.01%     
==========================================
  Files          30       31       +1     
  Lines        2111     2112       +1     
==========================================
+ Hits         1665     1666       +1     
  Misses        331      331              
  Partials      115      115              
Flag Coverage Δ
integration-test 66.76% <63.88%> (+0.01%) ⬆️
unittests 40.36% <8.33%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
pkg/ebpf/httpfltr/httpfltr.go 85.43% <0.00%> (+2.91%) ⬆️
pkg/ebpf/httpfltr/httpfltr_linux.go 68.75% <68.75%> (ø)
pkg/pipe/config.go 56.41% <100.00%> (+1.14%) ⬆️

Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

Looks good to me, I just had a question about returning 0 instead of 1 on Darwin. Thanks!


func findNamespace(_ int32) (uint32, error) {
// convenience method to allow unit tests compiling in Darwin
return 1, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we return 0, so that we don't set anything? 1 will be the init process, so we might be instrumenting a wrong process id?

@mariomac mariomac merged commit 40e14a6 into grafana:main Jun 7, 2023
@mariomac mariomac deleted the fix-prom branch June 7, 2023 14:53
mattdurham pushed a commit to mattdurham/beyla that referenced this pull request Jan 22, 2025
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