Skip to content
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

[package] opentelemetry-cpp/1.8 1: option to enable Logs support with ENABLE_LOGS_PREVIEW #15024

Closed
MrSparc opened this issue Dec 31, 2022 · 2 comments · Fixed by #15198
Closed
Labels
bug Something isn't working

Comments

@MrSparc
Copy link
Contributor

MrSparc commented Dec 31, 2022

Description

This is not a bug actually but a request to improvement for the current recipe because this library support this setting.

The Logs feature in opentelemetry-cpp 1.8.1 is still experimental but can be enabled via the define ENABLE_LOGS_PREVIEW that is handled by CMake WITH_LOGS_PREVIEW.
I would like to be able set this via an recipe option.
This will allow us to enable this support in our conanfile.txt like this:

[requires]
opentelemetry-cpp/1.8.1

[generators]
cmake

[options]
opentelemetry-cpp:with_logs_preview=True

This option is required in order to build the includes and libs that support ostream, OTLP HTTP and gRPC exporters for logs especification.

Here is my proposal of PR for the package maintainer:
MrSparc@88c94d0

Package and Environment Details

  • Package Name/Version: opentelemetry-cpp/1.8.1
  • Operating System+version: Linux Ubuntu 22.04
  • Compiler+version: GCC 11.3
  • Conan version: conan 1.53.0

Conan profile

N/A

Steps to reproduce

N/A.

Logs

N/A.

@justinmcbride
Copy link
Contributor

justinmcbride commented Jan 10, 2023

@MrSparc I took your changes and simply opened a PR with them (from your repo): #15198

Unfortunately, I was hoping it would be easy, but it looks like opening the PR with the changes from your repository would require you to sign the CLA (mentioned by the CLA Bot in the PR). Would you want to sign that? If not, I'll close the PR, and someone else will need to come along to implement the request.

@MrSparc
Copy link
Contributor Author

MrSparc commented Jan 11, 2023

@justinmcbride I signed now the CLA.
But I think that is not enough to be merged since it requires LGTM from 3 reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants