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.

Sign up for GitHub

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

Jump to bottom

example: demonstrate tracepoint usage #190

Merged
merged 2 commits into from
Mar 1, 2021
Merged

example: demonstrate tracepoint usage #190

merged 2 commits into from
Mar 1, 2021

Conversation

florianl
Copy link
Contributor

Signed-off-by: Florian Lehner dev@der-flo.net

This PR adds an example on how a third party package can be used to attach an eBPF program to a tracepoint.

@florianl florianl added the documentation Improvements or additions to documentation label Dec 19, 2020
@florianl florianl requested a review from lmb December 19, 2020 15:34
@florianl florianl mentioned this pull request Dec 19, 2020
Closed
lmb
lmb reviewed Jan 13, 2021
View reviewed changes
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

LGTM, should we remove the other example?

example_tracepoint_test.go Outdated Show resolved Hide resolved
example_tracepoint_test.go Outdated Show resolved Hide resolved
example_tracepoint_test.go Outdated
panic(fmt.Errorf("failed to attach eBPF to tracepoint: %v", err))
}

<-ctx.Done()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the program ever print any output? Since the goroutine is locked to the thread, nothing else can run on it and invoke open, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes - right. It's the very same output/setup as in example_program_test.go before.

example_tracepoint_test.go Outdated
"github.com/cilium/ebpf/asm"
ringbuffer "github.com/cilium/ebpf/perf"

"github.com/elastic/go-perf"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this means the package will be included in the go.mod? Probably acceptable.

Copy link
Contributor Author

@florianl florianl Jan 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

yes. It adds github.com/elastic/go-perf in go.mod. Added this as well now.

@florianl florianl force-pushed the flo-elastic-tracepoint-example branch 2 times, most recently from dd0d9cf to df7b9a7 Compare January 14, 2021 20:15
@lmb
Copy link
Collaborator

lmb commented Jan 18, 2021

Sorry for the long radio silence! I've discovered one unfortunate side-effect: go mod vendor includes go-perf now :( I need to figure out whether it's possible to avoid that.

@florianl
Copy link
Contributor Author

Good poind :+1 https://blog.golang.org/examples states that examples are executed as part of go test. Unless they are in package xyz_test. And as xyz_test is independent of package xyz it should not influence the modules files of xyz.
I will check this again.

@florianl
Copy link
Contributor Author

ok - I checked go help test and there it states 'Go test' recompiles each package along with any files with names matching the file pattern "*_test.go".
So I guess there is only the option to close this PR in order not to introduce the new dependency. What do you think?

@lmb
Copy link
Collaborator

lmb commented Jan 28, 2021

I think that would be very sad :( Maybe golang/go#36460 will help, do you mind if we keep this open until 1.16 lands and we try again, with an updated go.mod? The alternative would be to set up an examples repository.

@florianl
Copy link
Contributor Author

Keeping the PR open for the moment is fine for me :)

@lmb
Copy link
Collaborator

lmb commented Feb 24, 2021

@florianl I had an idea: maybe we can use a sub-Go-Module in an examples folder? That would have its own go.mod / go.sum so it wouldn't pollute the main module IIUC.

@florianl florianl force-pushed the flo-elastic-tracepoint-example branch from df7b9a7 to 5d66b59 Compare February 24, 2021 19:20
@florianl
Copy link
Contributor Author

Good idea 👍
I have updated the PR and moved the example to its own sub module.

@lmb
Copy link
Collaborator

lmb commented Feb 26, 2021 •
edited
Loading

One of the downsides of the nested module is that we have to manually bump the cilium/ebpf version every once in a while, it doesn't automatically follow the rest of the code in the repo. I'm not sure how much of an issue this is in practice. On one hand the API should be fairly stable now, on the other side it's going to introduce some bit-rot. Anyways, I've pushed a commit to only use a single examples submodule so that we only have to do this step once if we decide to go forward.

Another downside is that invoking go build ./... in the project root now completely ignores the examples. I think we should at least go build ./examples/... on CI to make sure we don't commit completely broken code. Not sure what else we can do.

Finally, I think it should be convention that examples only ever contains package main. This means they are installable via go get but other projects can't depend on the examples package in any way. That leaves us a way to get rid of the nested module if it becomes feasible in the future.

WDYT?

@lmb
Copy link
Collaborator

lmb commented Feb 26, 2021

cc @ti-mo I think you had ideas for examples as well?

@florianl florianl force-pushed the flo-elastic-tracepoint-example branch from e4437a2 to 6610ed0 Compare February 26, 2021 18:43
@lmb lmb force-pushed the flo-elastic-tracepoint-example branch from 5915a41 to 20cfa81 Compare March 1, 2021 11:06
@florianl @lmb
example: demonstrate tracepoint usage
67c69df 
Signed-off-by: Florian Lehner <dev@der-flo.net>
@lmb lmb force-pushed the flo-elastic-tracepoint-example branch from 20cfa81 to a8dbc7a Compare March 1, 2021 11:10
@lmb lmb force-pushed the flo-elastic-tracepoint-example branch from a8dbc7a to a8964d2 Compare March 1, 2021 11:37
@lmb lmb merged commit 5066a42 into master Mar 1, 2021
@lmb lmb deleted the flo-elastic-tracepoint-example branch March 1, 2021 11:59
@ti-mo
Copy link
Collaborator

ti-mo commented Mar 1, 2021

cc @ti-mo I think you had ideas for examples as well?

Sorry, missed this cc. This submodule thing actually looks like it could be a good approach for including (both userspace and kernelspace) samples in the lib without polluting the main go.mod! Haven't played with this kind of structure, but indeed we need to figure out how we're going to prevent these samples from rotting.

ti-mo
ti-mo reviewed Mar 1, 2021
View reviewed changes
examples/tracepoint/main.go
panic(fmt.Errorf("failed to restore original rlimit: %v", err))
}

runtime.LockOSThread()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is a sample, documenting why we need to lock an OS thread would be nice. Is this required?

@lmb lmb mentioned this pull request Mar 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmb lmb lmb left review comments

@ti-mo ti-mo ti-mo left review comments

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@florianl @lmb @ti-mo