Add BPFTRACE_STACK_MODE env variable #2586

jordalgo · 2023-05-03T00:12:51Z

Summary:
This is so users can more easily set the global output for their stacks rather than repeating the same string for every invocation of ustack or kstack

Test Plan:
Tested locally.

BPFTRACE_STACK_MODE=tomato sudo ./src/bpftrace -e 'kprobe:do_nanosleep {
printf("Jordan\n%s\n", ustack());  }'
ERROR: Env var 'BPFTRACE_STACK_MODE' did not contain a valid StackMode:
tomato

BPFTRACE_STACK_MODE=perf sudo ./src/bpftrace -e 'kprobe:do_nanosleep {
printf("Jordan\n%s\n", ustack());  }'
Attaching 1 probe...
Jordan

	7f088cae85b3 __clock_nanosleep+99
(/usr/local/fbcode/platform010-compat/lib/libc.so.6)
	7f088caed253 __nanosleep+19
(/usr/local/fbcode/platform010-compat/lib/libc.so.6)
	1f3418b
scribe::common::sink::QueueSink::sendLoop(scribe::common::sink::detail::BatchQueue*,
int)+2971 (/usr/local/fbprojects/scribe/bin/scribed)
	6187e12 folly::ThreadedExecutor::work(folly::Function<void
()>&)+82 (/usr/local/fbprojects/scribe/bin/scribed)
	7f088cedf2e5 execute_native_thread_routine+21
(/usr/local/fbcode/platform010-compat/lib/libstdc++.so.6.0.29)
	7f088ca979bf start_thread+559
(/usr/local/fbcode/platform010-compat/lib/libc.so.6)
	7f088cb288ac __GI___clone3+44
(/usr/local/fbcode/platform010-compat/lib/libc.so.6)

Reviewers:

Subscribers:

Tasks:

Tags:

Checklist

Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
User-visible and non-trivial changes updated in CHANGELOG.md
The new behaviour is covered by tests

jordalgo · 2023-05-03T00:13:12Z

Not sure where to add unit tests for this.

src/main.cpp

viktormalik · 2023-05-03T06:01:06Z

Not sure where to add unit tests for this.

I think that runtime tests (e.g. in tests/runtime/call) would be sufficient.

danobi · 2023-05-03T16:14:56Z

docs/reference_guide.md

@@ -553,6 +554,11 @@ Default: `..`

 Trailer to add to strings that were truncated. Set to empty string to disable truncation trailers.

+### 9.11 `BPFTRACE_STACK_MODE`


Please also update bpftrace.adoc . (We really have to unify it one of these days)

danobi · 2023-05-03T16:15:22Z

docs/reference_guide.md

+
+Default: bpftrace
+
+Output format for ustack and kstack builtins. Available modes/formats: `bpftrace`, `perf`, and `raw`.


Perhaps also mention that it's still override-able at call sites?

jordalgo · 2023-05-04T10:51:43Z

Not sure where to add unit tests for this.

I think that runtime tests (e.g. in tests/runtime/call) would be sufficient.

I don't see a way to verify the ustack or kstack output easily. The current ustack test just checks that the probe is attached. I could add something like this just to ensure bpftrace doesn't vomit but I don't know how useful it is:

NAME ustack_stack_mode_env
PROG k:do_nanosleep { printf("%s\n%s\n", ustack(), ustack(1)); exit(); }
ENV BPFTRACE_STACK_MODE=raw
EXPECT Attaching 1 probe
TIMEOUT 5
AFTER ./testprogs/syscall nanosleep  1e8

viktormalik · 2023-05-04T11:02:29Z

I don't see a way to verify the ustack or kstack output easily. The current ustack test just checks that the probe is attached. I could add something like this just to ensure bpftrace doesn't vomit but I don't know how useful it is:

Could you verify that any kstack/ustack was printed in the correct format?

# bpftrace format
EXPECT ^\s+[a-zA-Z0-9_]+
# perf format
EXPECT ^\s+[0-9a-f]+ [a-zA-Z0-9_]+
# raw format
EXPECT ^\s+[0-9a-f]+$

jordalgo · 2023-05-04T13:55:57Z

Could you verify that any kstack/ustack was printed in the correct format?
# bpftrace format
EXPECT ^\s+[a-zA-Z0-9_]+
# perf format
EXPECT ^\s+[0-9a-f]+ [a-zA-Z0-9_]+
# raw format
EXPECT ^\s+[0-9a-f]+$

I worry that the output might change slightly and then the regex would fail e.g. here is the output for this:

NAME ustack_stack_mode_env
PROG k:do_nanosleep { printf("%s", ustack(1)); exit(); }
ENV BPFTRACE_STACK_MODE=perf
EXPECT ^\s+[0-9a-f]+$
TIMEOUT 5
AFTER ./testprogs/syscall nanosleep  1e8

Attaching 1 probe...\n__BPFTRACE_NOTIFY_PROBES_ATTACHED\n\n\t7f088cae85b3 __clock_nanosleep+99 (/usr/local/fbcode/platform010-compat/lib/libc.so.6)\n\n\n

viktormalik · 2023-05-04T14:00:06Z

Could you verify that any kstack/ustack was printed in the correct format?
# bpftrace format
EXPECT ^\s+[a-zA-Z0-9_]+
# perf format
EXPECT ^\s+[0-9a-f]+ [a-zA-Z0-9_]+
# raw format
EXPECT ^\s+[0-9a-f]+$

I worry that the output might change slightly and then the regex would fail e.g. here is the output for this:

NAME ustack_stack_mode_env
PROG k:do_nanosleep { printf("%s", ustack(1)); exit(); }
ENV BPFTRACE_STACK_MODE=perf
EXPECT ^\s+[0-9a-f]+$
TIMEOUT 5
AFTER ./testprogs/syscall nanosleep  1e8

Attaching 1 probe...\n__BPFTRACE_NOTIFY_PROBES_ATTACHED\n\n\t7f088cae85b3 __clock_nanosleep+99 (/usr/local/fbcode/platform010-compat/lib/libc.so.6)\n\n\n

But you're using the raw regex for perf format.

jordalgo · 2023-05-04T14:15:40Z

But you're using the raw regex for perf format.

I was just using that as an example of the output. As the regexes for all would need to start with something that looked like this

^Attaching 1 probe...\\n__BPFTRACE_NOTIFY_PROBES_ATTACHED[\\n\\t]*

Feels fragile

viktormalik · 2023-05-04T14:34:39Z

I don't think so, the regex should be matched against every line of the output.

jordalgo · 2023-05-04T14:59:14Z

I don't think so, the regex should be matched against every line of the output.

Well, I don't quite understand HOW it passes but it does :) E.g. why doesn't the check fail for the line "Attaching 1 probe..." against this regex ^\s+[0-9a-f]+$. Maybe it just needs to match against at least ONE line in the output?

danobi · 2023-05-07T17:43:51Z

I think the regex only has to match a single line:

https://github.com/iovisor/bpftrace/blob/8e9853fb015e62968cde853dc8c1cdca8c68fec8/tests/runtime/engine/runner.py#L251
https://docs.python.org/3/library/re.html#re.search

which would match my experience (IIRC)

So I’m not sure having perf/raw/bpftrace regex is correct.

jordalgo · 2023-05-08T15:35:03Z

So I’m not sure having perf/raw/bpftrace regex is correct.

I guess it depends how much extra noise we get outside of the single stack trace that we think might pollute the results. The tests I added seem to do the right thing and fail if we change the StackMode...

viktormalik · 2023-05-09T08:25:32Z

So I’m not sure having perf/raw/bpftrace regex is correct.

I guess it depends how much extra noise we get outside of the single stack trace that we think might pollute the results. The tests I added seem to do the right thing and fail if we change the StackMode...

Agreed with @jordalgo, the added regexes are specific enough to avoid matching anything outside of the expected stack trace.

Summary: This is so users can more easily set the global output for their stacks rather than repeating the same string for every invocation of ustack or kstack Test Plan: Tested locally. ``` BPFTRACE_STACK_MODE=tomato sudo ./src/bpftrace -e 'kprobe:do_nanosleep { printf("Jordan\n%s\n", ustack()); }' ERROR: Env var 'BPFTRACE_STACK_MODE' did not contain a valid StackMode: tomato ``` ``` BPFTRACE_STACK_MODE=perf sudo ./src/bpftrace -e 'kprobe:do_nanosleep { printf("Jordan\n%s\n", ustack()); }' Attaching 1 probe... Jordan 7f088cae85b3 __clock_nanosleep+99 (/usr/local/fbcode/platform010-compat/lib/libc.so.6) 7f088caed253 __nanosleep+19 (/usr/local/fbcode/platform010-compat/lib/libc.so.6) 1f3418b scribe::common::sink::QueueSink::sendLoop(scribe::common::sink::detail::BatchQueue*, int)+2971 (/usr/local/fbprojects/scribe/bin/scribed) 6187e12 folly::ThreadedExecutor::work(folly::Function<void ()>&)+82 (/usr/local/fbprojects/scribe/bin/scribed) 7f088cedf2e5 execute_native_thread_routine+21 (/usr/local/fbcode/platform010-compat/lib/libstdc++.so.6.0.29) 7f088ca979bf start_thread+559 (/usr/local/fbcode/platform010-compat/lib/libc.so.6) 7f088cb288ac __GI___clone3+44 (/usr/local/fbcode/platform010-compat/lib/libc.so.6) ``` Reviewers: Subscribers: Tasks: Tags:

jordalgo requested review from ajor, viktormalik, danobi and fbs as code owners May 3, 2023 00:12

viktormalik reviewed May 3, 2023

View reviewed changes

src/main.cpp Outdated Show resolved Hide resolved

danobi reviewed May 3, 2023

View reviewed changes

jordalgo force-pushed the master branch 3 times, most recently from fe7c0bd to cf31b7d Compare May 4, 2023 17:23

viktormalik approved these changes May 9, 2023

View reviewed changes

jordalgo force-pushed the master branch from cf31b7d to ef7d16b Compare May 9, 2023 10:30

danobi approved these changes May 9, 2023

View reviewed changes

danobi merged commit 491089d into bpftrace:master May 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BPFTRACE_STACK_MODE env variable #2586

Add BPFTRACE_STACK_MODE env variable #2586

jordalgo commented May 3, 2023 •

edited

Loading

jordalgo commented May 3, 2023

viktormalik commented May 3, 2023

danobi May 3, 2023

danobi May 3, 2023

jordalgo commented May 4, 2023

viktormalik commented May 4, 2023

jordalgo commented May 4, 2023

viktormalik commented May 4, 2023

jordalgo commented May 4, 2023 •

edited

Loading

viktormalik commented May 4, 2023

jordalgo commented May 4, 2023

danobi commented May 7, 2023

jordalgo commented May 8, 2023

viktormalik commented May 9, 2023

		@@ -553,6 +554,11 @@ Default: `..`

		Trailer to add to strings that were truncated. Set to empty string to disable truncation trailers.

		### 9.11 `BPFTRACE_STACK_MODE`


		Default: bpftrace

		Output format for ustack and kstack builtins. Available modes/formats: `bpftrace`, `perf`, and `raw`.

Add BPFTRACE_STACK_MODE env variable #2586

Add BPFTRACE_STACK_MODE env variable #2586

Conversation

jordalgo commented May 3, 2023 • edited Loading

Checklist

jordalgo commented May 3, 2023

viktormalik commented May 3, 2023

danobi May 3, 2023

Choose a reason for hiding this comment

danobi May 3, 2023

Choose a reason for hiding this comment

jordalgo commented May 4, 2023

viktormalik commented May 4, 2023

jordalgo commented May 4, 2023

viktormalik commented May 4, 2023

jordalgo commented May 4, 2023 • edited Loading

viktormalik commented May 4, 2023

jordalgo commented May 4, 2023

danobi commented May 7, 2023

jordalgo commented May 8, 2023

viktormalik commented May 9, 2023

jordalgo commented May 3, 2023 •

edited

Loading

jordalgo commented May 4, 2023 •

edited

Loading