Provide a CMake target for prometheus_exporter_utils #2566
Conversation
| target_link_libraries( | ||
| opentelemetry_exporter_prometheus_utils | ||
| PUBLIC opentelemetry_metrics prometheus-cpp::core) | ||
|
|
There was a problem hiding this comment.
Targets are also added in cmake config -
But I am not sure of the implications in that case - as the symbols from exporter_utils.cc would be part of two target belonging to same packageOPENTELEMETRY_CPP_LIBRARIES. Will let @owent comment on that :)
There was a problem hiding this comment.
I don't think that should be an issue, resolving it. Sorry for confusion :)
There was a problem hiding this comment.
I think we should remove src/exporter_utils.cc from and link opentelemetry_exporter_prometheus_utils into opentelemetry_exporter_prometheus if we want to split this file into a standalone target.Or there may be conflict when some components link both opentelemetry_exporter_prometheus_utils and opentelemetry_exporter_prometheus.
BTW: The documents and the component name for OPENTELEMETRY_CPP_LIBRARIES should also be added into cmake/opentelemetry-cpp-config.cmake.in.
|
@lalitb Does the patch look okay apart from your original comment? |
|
The PR should not break existing builds for the users and this has to be investigated before merge. |
|
Thanks for the PR. Currently, opentelemetry-cpp maintains the following builds:
On top of the build systems supported by opentelemetry-cpp,
Maintaining all this ecosystem, and making sure various combinations of existing features are working properly is already a challenge for opentelemetry-cpp, and a major drain on development resources. What this PR asks for makes sense, but the problem is that this new build combination would not be tested and covered in the existing Adding even more complexity to an already difficult to maintain area is not desirable at this point. Given how this combination is not covered in our CI, there is a higher risk of this build to be broken by later changes or refactoring done in the opentelemetry-cpp make files, and the break can go unnoticed. As a result, if we (opentelemetry-cpp) accept this PR, we would just be setting expectations (to support it and maintain it) that realistically will not be met. Closing this PR. Overall, the need to cut the dependency from the prometheus exporter to a given web server is valid. The best way to achieve this is to extend the prometheus exporter to support alternate web servers instead, but this is a much bigger feature and not just a makefile change to split a library. |
Fixes #2565