-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Enhanced Tracing in libcurl and curl #11421
Conversation
We'll need to update --- a/libcurl.def
+++ b/libcurl.def
@@ -28,6 +28,7 @@ curl_getenv
curl_global_cleanup
curl_global_init
curl_global_init_mem
+curl_global_log_config
curl_global_sslset
curl_maprintf
curl_mfprintf |
Can't we generate that list? Or at worst make a CI job that checks that the list is up-to-date? |
@bagder PR ready for checking it: #11570. I can't imagine a way to dynamically generate this from both CMake and |
Added this in the recent push. Thanks for noticing |
Some local measurement on my macOS dev machine. I see no real difference in performance between non-debug builds on master and this PR. Meaning the guarded log statements have no impact.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about how this can be tested, at least rudimentary?
.\" ************************************************************************** | ||
.TH curl_global_log_config 3 "01 August 2023" "libcurl" "libcurl" | ||
.SH NAME | ||
curl_global_log_config - Global libcurl logging configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of naming it curl_global_trace()
? It is shorter and I think that is slightly more fitting name that avoids the word "log" which might have a negative connotation. We also use the word trace for this for the command line option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in recent push.
The list of component names is not part of curl's public API. Names may | ||
be added or disappear in future versions of libcurl. Since unknown names | ||
are silently ignored, outdated log configurations will not error when | ||
upgrading libcurl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we still need to document the existing available component names here, don't we? Otherwise it seems very hard to use this function!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a trade off with this, of course. Do we want to document these names as part of the public API? Do we keep the names then in future versions?
Added test cases in pytest. |
- WIP: app init method is missing. CURL_DEBUG env var only parsed on debug builds. - add format attributes to infof() and failf() declarations - protect `infof()` execution by verbosity check, saving some cycles when not verbose - change patterns `DEBUGF(LOG_CF(...))` to `CURL_LOG_CF(...)`
- added curl_log_configure(config) to set log configuration explicitly - log config extended - 'all' for all known components - +/- prefix to enable/disable
- curl_global_log_config() for setting log configration in libcurl - --trace-config command line option for curl - man pages - test adjustments for new function
- curl_log.[ch] is not curl_trc.[ch] - curl_global_log_config() is now curl_global_trace() - CURL_LOG_CF() is now CURL_TRC_CF() etc.
Ready from my side. |
Add --trace-config to curl Add curl_global_trace() to libcurl Closes curl#11421
- enable symbol hiding manually to match cmake/autotools. - enable variadic macros. These macros became required in curl 8.3.0 with curl/curl#11421. These macros will no longer be needed with curl 8.5.0 after curl/curl#12167. - set `HAVE_SNPRINTF`. curl's `config-win32.h` missed to set it, causing a minor difference in the generated binaries compared to cmake/autotools builds. Ref: curl/curl#12325 With these changes GNU Make builds are in sync again with cmake/autotools.
Making detailed logging availabel in libcurl and curl:
- --trace-config
command line option for curlDEBUGF(LOG_CF(..))
code toCURL_LOG_CF()
that is available in non-debug buildsinfof()
execution by verbosity check, saving some cycles when not verboseExamples:
Add HTTP/2 details to verbose output:
Add HTTP/2, SSL and id details to verbose output:
Enable all details in verbose output, including times and ids and all components in libcurl:
Enable all details, but not about SSL or TCP: