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

Allow wildcards for tracepoint categories #1445

Merged
merged 1 commit into from
Aug 8, 2020

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Jul 30, 2020

It is now possible to run e.g. this:

bpftrace -e 'tracepoint:kvm*:* { @[probe] = count(); }'

Resolves #1212.

Checklist
  • Language changes are updated in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md

@viktormalik
Copy link
Contributor Author

Embedded build failed on a uprobe test (cannot open mount ns), which shouldn't be affected by this PR. Not sure why the error occurred, it passed on my fork's branch build.

Also there are clang-format fails in a new test that I copied from an existing one (which contains the same format errors, too).

@fbs
Copy link
Contributor

fbs commented Jul 30, 2020

Embedded build failed on a uprobe test (cannot open mount ns), which shouldn't be affected by this PR. Not sure why the error occurred, it passed on my fork's branch build.

Some tests are still somewhat flaky and fail sporadically, rerunning it usually fixes it

Also there are clang-format fails in a new test that I copied from an existing one (which contains the same format errors, too).

We try to incrementally fix the formatting errors. So if you touch code you format it. This way we can keep git blame useful.

@danobi
Copy link
Member

danobi commented Jul 31, 2020

I kicked off another round of embedded tests. Let's see if it passes this time.

We try to incrementally fix the formatting errors. So if you touch code you format it. This way we can keep git blame useful.

+1

It is now possible to run e.g. this:

    bpftrace -e 'tracepoint:kvm*:* { @[probe] = count(); }'

Changes were done to wildcard matching: prefix is no longer used (since
no probe type uses a fixed prefix), the tracepoint category is made a
part of the function pattern. Also, find_wildcard_matches now returns
tracepoint matches in the form "category:event", which must be handled
appropriately in the calling code.

Resolves iovisor#1212.

New unit tests for codegen and bpftrace are added.
@viktormalik
Copy link
Contributor Author

I kicked off another round of embedded tests. Let's see if it passes this time.

Thanks, they all passed now.

We try to incrementally fix the formatting errors. So if you touch code you format it. This way we can keep git blame useful.

Sounds good, ok, fixed. I'll keep this in mind.

Copy link
Member

@danobi danobi left a comment

Choose a reason for hiding this comment

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

This entire codepath is pretty gnarly and is in great need of a good refactor. This PR doesn't make it materially worse (in fact it might even be a little better).

LGTM but every time anyone touches this code path I get scared something's gonna break but have no real way of knowing.

@fbs fbs merged commit 2b362c5 into bpftrace:master Aug 8, 2020
@viktormalik viktormalik deleted the tp-category-wildcards branch August 10, 2020 06:09
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.

Allow wildcards to be used in tracepoint categories
3 participants