GH-98894: Fix `function__return` and `function__entry` dTrace probe missing after `GH-103083`
Fix #98894
- Issue: gh-98894
After #103083, the function__return and function__entry dTrace probe is missing. In the production environment, the developer may combine the dtrace probe with other tools like eBPF to trace the internal state of the CPython like
sudo bpftrace -e '
usdt:/home/manjusaka/Documents/projects/cpython/python:python:function__return {
printf("filename: %s, funcname:%s, lineno:%d\n",str(arg0),str(arg1),arg2);
}
' -p 291832
And the function__return and function__entry part is still in the official document of the CPython for the Dtrace module.
So I think we should keep the codebase same with the document or we need to update the document if we confirm that we don't need the dtrace feature any more.
The build errors:
/tmp/tmpwuomaa14/_RETURN_VALUE.c:124:13: error: call to undeclared function 'PyDTrace_FUNCTION_RETURN_ENABLED'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 124 | DTRACE_FUNCTION_EXIT();
Something is probably missing somewhere.
The build errors:
/tmp/tmpwuomaa14/_RETURN_VALUE.c:124:13: error: call to undeclared function 'PyDTrace_FUNCTION_RETURN_ENABLED'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 124 | DTRACE_FUNCTION_EXIT();
Something is probably missing somewhere.
I have noticed this, I'm working on the CI. Thanks for the tips. Draft this PR first
DTRACE_FUNCTION_ENTRY() and DTRACE_FUNCTION_EXIT() are static instrumentation points for dtrace, are they not?
IIUC, this means that they will not work with the JIT. Even if the instrumentation points are compiled into the templates, dtrace will not be able to find them.
How does dtrace handle jit-in-time compiled code?
DTRACE_FUNCTION_ENTRY()andDTRACE_FUNCTION_EXIT()are static instrumentation points for dtrace, are they not?
Yes, it's static
IIUC, this means that they will not work with the JIT. Even if the instrumentation points are compiled into the templates, dtrace will not be able to find them.
How does dtrace handle jit-in-time compiled code?
The DTRACE_FUNCTION_ENTRY() and DTRACE_FUNCTION_EXIT() is not working in JIT mode. For now I just compile some empty function into the template to avoid the compile issue.
For now, the JIT is still an experimental feature. I think DTRACE_FUNCTION_ENTRY() and DTRACE_FUNCTION_EXIT() are still useful for normal code.
For the future, I think we may need extra dtrace point for JIT module
For the future, I think we may need extra dtrace point for JIT module
How? There's no point in adding back dtrace support for 3.14, if it stops working again in 3.15.
How?
That would be some different dtrace points, I'm not sure we need to discuss it here.
There's no point in adding back dtrace support for 3.14, if it stops working again in 3.15.
I'm not sure about the JIT roadmap. if here's more than five years before we make the JIT default release, I think it still is worthed adding the dtrace point back.
Otherwise, we need to clean up the docs FYI https://docs.python.org/3/howto/instrumentation.html
@markshannon ping~
Also, merging it as it is facilitates backporting.
@markshannon ping~
I'm not sure about the JIT roadmap. if here's more than five years before we make the JIT default release, I think it still is worthed adding the dtrace point back.
The JIT will be included in 3.14, but probably off by default. It will almost certainly on by default for 3.15.
I'm not opposed to having dtrace hooks, but I don't see much value in them unless they
- have tests (which means they built in by default, so CI can run the tests)
- will work with the JIT
- have very low overhead when not in use (should be true for the interpreter, but might not be for the JIT)
I'm not opposed to having dtrace hooks, but I don't see much value in them unless they
- have tests (which means they built in by default, so CI can run the tests)
- will work with the JIT
- have very low overhead when not in use (should be true for the interpreter, but might not be for the JIT)
OK, I got it. 1 and 3 would not be a big issue, but I need more time about 2. So how about we update https://docs.python.org/3/howto/instrumentation.html and remove the function__return and function__entry part first?
+1 (bpf|d)trace instrumentation is an important feature for us. Is someone still working on it? If not, please add a note in the documentation that USDT probes are broken after python3.10, and users should avoid upgrading.
@Zheaoli Can you resolve the conflicts?
@Zheaoli Can you resolve the conflicts?
Yes, But I think we need a final call here.
@markshannon Should we recover the USDT probe or we just need to remove it from documentation?
If the Python JIT can be enabled/disabled without a recompilation of Python, then to me it seems worth it to include this instrumentation for non-JIT codepaths at least.
That way, people who want instrumentation can disable the JIT to get it as needed. It can be documented that the probe points won't work when the JIT is enabled (rather than the alternative of them not working at all!)
If the Python JIT can be enabled/disabled without a recompilation of Python, then to me it seems worth it to include this instrumentation for non-JIT codepaths at least.
That way, people who want instrumentation can disable the JIT to get it as needed. It can be documented that the probe points won't work when the JIT is enabled (rather than the alternative of them not working at all!)
This seems like the easiest path to move forward with, but it defeats the greatest power of Dtrace - intrumenting running processes (with low overhead). If you need to restart a process to instrument it (disabling the jit), the probes become much less useful. That said, this is still much preferable to removing the probes :)
I'm also confident that I can get eyes on the general DTrace vs JIT issue in the future, just at the moment, the team working on that is busy with some more fundamental bits on the DTrace side. That is, I don't think this is delaying the inevitable or anything (and therefore PR should ideally go in for now).
I think there are many people still need USDT for non-JIT build. I think this PR is still worth to push forward. I resolve the code conflict
close this PR