Hi, this is a feedback for the faf design notes currently located at https://github.com/abrt/faf/wiki
In general I like the idea of having the functionality split into several types of modules. Some minor suggestions, mostly regarding the ureport format:
* The next points follow the guideline that if something can be computed on the server then let's do it there. We have greater control over the server side than the clients (and use higher-level language => faster development).
* I'm not sure whether the list(s) of packages should be considered part of the OS or the problem (or split among both) but in any way the metadata available for a package depend on the OS. Personally, I don't see an advantage in assigning roles (interpreter, package, related package) to the packages on the client side - this can probably be done on the server.
So my suggestion is to have single list of packages in the report. What packages are included in the report is determined by the OS type (e.g. for fedora, always include the kernel and selinux-policy; for debian, include the suggested/recommended packages) and the problem (e.g. for ccpp crash include packages of all the libraries involved; for python crashes include packages of all the modules involved and the python interpreter, ...).
* I'd suggest representing the packages as dictionaries with keys dependent on the OS as opposed to single string. There's some code in satyr for checking RPM integrity (whether packaged files were modified) so I guess it may be useful for us in fedora? We may have to keep track from which repo the package comes in distributions that allow custom repositories.
* Regarding the module interfaces, I guess it's probably quite early to design definitive versions, I suppose we'll find the actual required methods during implementation:) I can imagine that OS plugin method for getting files in a package and a problem plugin method for finding a package responsible for the crash could be useful.
Random thought regarding retracing: stack frame from report may expand to multiple source code level stack frames due to function inlining (or do we do that already?).
* Random thought regarding ccpp crashes: allow reports without stack traces, certain crashes may damage stack so it cannot be unwound and I guess we are interested in those crashes as well.
Anyway, below is my proposal of what a ccpp (haven't given much thought to other crash types) ureport2 can look like and further below is a ureport that satyr currently produces. I've added some comments inline. Most changes are cosmetic and should be treated as opinion regarding the bike shed color [1] than anything else:) But then again, if you design nice and flexible data structures, it's easier to build good code around them ...
== proposed example of ureport 2 ==
{ "ureport_version": 2, "reporter": { "name": "ABRT", "version": "2.0.20" } "reason": "Killed by SIGSEGV", # is this useful? probably not on the server but we should # present some human-readable string on the client side
# i'd remove the "custom" key and move its contents to the top level, it's not useful imho
"os": { "type": "fedora", # this key (as well as problem.type) can be moved to the top level # doesn't really matter "version": "16", "arch": "x86_64", "packages": [ { "name": "kernel", "version": ..., "release": ..., "architecture": ..., "epoch": ..., "install_time": ..., "modified": false, # package integrity check? }, { "name": "glibc", ... }, { "name": "selinux-policy", ... }, { "name": "coreutils", ...} ], "selinux-mode": "enforcing" }
"problem": { "type": "ccpp", # or core/binary/whatever "core_stacktrace": { # or just stacktrace "signal": 11, # or "SEGV" "executable": "/bin/sleep", "threads": [ # thread numbers not useful, imo ... neither their ordering { "frames": [ # we may have other per-thread info (signal?) { "build_id": "fedcba", .... }, # frame numbers not important, only the ordering { "build_id": "789456", .... } # for suggested frame contents, see the satyr report below ], "crash_thread": true } ] } } }
== what satyr currently generates ==
{ "ureport_version": 2 , "report_type": "core" , "reporter": { "name": "satyr" , "version": "0.1" } , "user_root": false # not sure if os-specific or problem-specific , "user_local": true # but probably useful , "os": { "name": "Fedora" , "version": "17" , "architecture": "x86_64" } , "component": "will-crash" # redundant, can be determined on the server # based on core stacktrace executable , "rpm_packages": [ { "name": "glibc" , "epoch": 0 , "version": "2.15" , "release": "58.fc17" , "architecture": "x86_64" , "install_time": 1349344996 } , { "name": "will-crash" , "epoch": 0 , "version": "0.2" , "release": "1.fc17" , "architecture": "x86_64" , "install_time": 1349344996 } ] , "core_stacktrace": { "signal": 0 , "crash_thread": 0 , "executable": "/usr/bin/will_segfault" , "threads": [ { "frames": [ { "address": 4195358 , "build_id": "b5965f39c22d9d6ae1cc6b64db43c19d959f318e" , "build_id_offset": 1054 , "file_name": "/usr/bin/will_segfault" , "fingerprint": "27fec48d69579748149aa1a5456ea2d906ff5744" } , { "address": 237190125365 , "build_id": "cc10c72da62c93033e227ffbe2670f2c4fbbde1a" , "build_id_offset": 137013 , "function_name": "__libc_start_main" , "file_name": "/usr/lib64/libc-2.15.so" , "fingerprint": "075acda5d3230e115cf7c88597eaba416bdaa6bb" } ] } ] } }
Hope some of this makes sense, Martin