open-telemetry / open-telemetry/opentelemetry-cpp

Generate a configure-time config.h, included in all other header/.cpp file

Open
#3,085 3 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

do-not-stale triage/accepted
Dominant language
C++
Stars
1.4k
Forks
632
Avg merge
1d 13h
Merged PRs (30d)
75

Description

My request is done in the context of people using none of CMake or Bazel build system. It's the case in our company. We do build some open source libraries, and "convert it" to the delivery format expected by our internal build system. The way most open source library works (historically, the ones using autotools) is be generating at configure time some opentelemetry/config.h header file (note: you have already one such file, but it's hardcoded, not generated). This way any non default option (like the WITH_STL version, or things like -DENABLE_ASYNC_EXPORT (mentioned in #3084) are directly available in the installed headers, instead of having to pass these configuration macros to the compiler. For me these macros are more or less part of the ABI, as changing them seems to add/remove/change some symbols found in the generated binaries, so it's more logical (according to me), that the generated headers reflect these configuration options. Today we have to dig a big inside the generated CMake files, or read/understand the CMakeList.txt files, looking at implementation details, and find which macros someone NOT using CMake should define when including these opentelemetry headers.

Doing that means:

  • CMake logic is updated a bit to generate a new config file at configure time
  • CMake logic is changed to stop defining all these macros as compilation flags
  • All files (public headers and private ones) are updated to include this new config file first, so that all see a consistant "configuration".

So it's definitely a change involving small changes in many files.

The added value here is to ease the life of people building the lib with CMake, but somehow packaging it as a deliverable for another build system. I think even Linux distros who package this as an RPM would need that, as people using the rpm have no idea how the pre-built library itself was configured (though it seems for now there is no package in fedora for the opentelemetry C++ library).

Since you seem to try to target a big audience for this lib, even allowing to build without a proper recent >= C++11 STL library, I propose this as it seems something you might accept.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the CMake logic and the existing hardcoded opentelemetry/config.h. Trace how configuration macros are currently passed as compilation flags and identify the public and private headers that must include the generated config first. Done means configure-time generation works, those flags are no longer required from consumers, and all headers see a consistent configuration.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.