Properly support setting DD_TRACE_TRANSPORT to "none"#172
Properly support setting DD_TRACE_TRANSPORT to "none"#172dmehala merged 1 commit intoDataDog:mainfrom
Conversation
445f621 to
7f7acd1
Compare
|
Fantastic @kristjanvalur. |
dmehala
left a comment
There was a problem hiding this comment.
LGTM!
If I recall correctly, when no HTTP client is provided, the library defaults to default_http_client. This could potentially lead to a crash since the library assumes default_http_client is always a valid instance. While it functions, some refactoring would be beneficial to help users avoid unintended issues.
I think the tradeoff is ok, as long as it help you progress on your plugin :)
Thank you!
|
I could add a check in the config builder for a nullptr, but since this is an integration issue and not a configuration issue, in my view assuming a non-null ptr (and thus a correctly written integration) is ok. |
Description
Omit the 'curl' library if DD_TRACE_TRANSPORT is set to "none"
Motivation
There is clearly the intent to be able to not include a default transport, by setting
DD_TRACE_TRANSPORT=none.However, two things were missing:
default_http_client_curl.cppwithdefault_http_client_null.cppIntegrations that provide their own transport may not want to build and link a whole unused library, along with any runtime library dependencies it makes.
Additional Notes