Use fetch content for otel cpp - #653
thompson-tomo wants to merge 43 commits into
Conversation
5e111ff to
448cdf8
Compare
74873ed to
9257d33
Compare
9257d33 to
5194d9f
Compare
| PROPERTIES EXPORT_NAME opentelemetry_exporter_geneva_logs) | ||
|
|
||
| if(MAIN_PROJECT) | ||
| target_link_libraries(opentelemetry_exporter_geneva_logs |
There was a problem hiding this comment.
Both the FetchContent and find_package paths import the public opentelemetry-cpp::* CMake targets and the call to target_link_libraries( ... opentelemetry-cpp::api) should be unconditional.
| set_file_path_if_valid_ = {[](LogRecord *, const std::string &) {}, | ||
| [](LogRecord *log_record, const std::string &file_name) { | ||
| log_record->SetAttribute(kCodeFilepath, file_name); | ||
| log_record->SetAttribute("code.filepath", file_name); |
There was a problem hiding this comment.
Please use the auto generated code attribute constants from <opentelemetry/semconv/code_attributes.h>
There was a problem hiding this comment.
I can't these are the deprecated constants which no longer exist and, changing it to the new constant would be a breaking change.
There was a problem hiding this comment.
I missed that these are the deprecated attributes.
The opentelemetry-cpp api target includes the <opentelemetry/semconv/incubating/code_attributes.h> header with the deprecated versions. Can these be used?
There was a problem hiding this comment.
Thanks, i had missed that the deprecated were still in the incubating conventions. Have switched them but i am more interested in what is needed to fix ci after making the previously requested reverts.
| ) | ||
|
|
||
| if(MAIN_PROJECT) | ||
| target_link_libraries(${this_target} PUBLIC |
There was a problem hiding this comment.
The link here should be unconditional
This goes through and removes the otel cpp repo checkout and local build steps and instead leverage a fetchcontent in the cmake build process.
It is easier to review this pr hiding whitespace aka https://github.com/open-telemetry/opentelemetry-cpp-contrib/pull/653/changes?w=1#diff-afedc88ba9243d94695a1f609b7a87dab38fc0dd1a86a789eaba4ddffe4780bf