Make macros.h available for all source files via version.h

[ThomsonTan]

Changes

As the macros defined in macros.h is general for all sources, including it in version.h to make it available for all sources (no need for separate #include "opentelemetry/common/macros.h" statement.

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #1918 (58f3bfc) into main (0305ad8) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1918   +/-   ##
=======================================
  Coverage   86.92%   86.92%           
=======================================
  Files         165      165           
  Lines        4594     4594           
=======================================
  Hits         3993     3993           
  Misses        601      601           

Impacted Files	Coverage Δ
api/include/opentelemetry/trace/trace_state.h	97.60% <ø> (ø)
...include/opentelemetry/sdk/metrics/view/predicate.h	72.73% <ø> (ø)
sdk/src/metrics/instrument_metadata_validator.cc	100.00% <ø> (ø)

[marcalff]

Making this change will expose:

• the cstdint header file
• the regex headerfile
• the HAVE_WORKING_REGEX symbol, which is not using an OPENTELEMETRY_ prefix and therefore pollutes the build

for any user of any api header file.

I would prefer to apply the include-what-you-use principles, and only include macro.h in the few places where it is needed.

The API header files are stable by now, so there is no real maintenance burden and breaks caused by missing includes on macro.h

What is the motivation or rationale for this change ?

[ThomsonTan]

Making this change will expose:

• the cstdint header file
• the regex headerfile
• the HAVE_WORKING_REGEX symbol, which is not using an OPENTELEMETRY_ prefix and therefore pollutes the build

for any user of any api header file.

I would prefer to apply the include-what-you-use principles, and only include macro.h in the few places where it is needed.

The API header files are stable by now, so there is no real maintenance burden and breaks caused by missing includes on macro.h

What is the motivation or rationale for this change ?

Perhaps we should remove cstdint from macro here? HAVE_WORKING_REGEX is like any definition in config.h which is fine and convenient to be included for every source file under the repo?

The intention was the make the all the generic macros in a common header file for all source files. I am adding some declspec macro for building DLL, so I'd like to avoid including it for every header (there could be many of them).

[ThomsonTan]

Perhaps move #include <regex> to the source file which need it instead of putting into this generic header file?

[owent]

LGTM

[marcalff]

LGTM with the following changes, please consider to:

• rename HAVE_WORKING_REGEX to OPENTELEMETRY_HAVE_WORKING_REGEX, everywhere (including in the SDK)
• only set OPENTELEMETRY_HAVE_WORKING_REGEX based on the GCC version, but do not include regex.h when setting the flag
• include (conditionally) regex.h when using the flag
• remove the code duplication from trace_state.h

With this, all uses should look like:

#if OPENTELEMETRY_HAVE_WORKING_REGEX
#  include <regex>
#endif

I know this is an existing issue, but cleaning it up before exposing macro.h is needed, to avoid causing side effects on user code.

No strong opinion on whether to put the code to set OPENTELEMETRY_HAVE_WORKING_REGEX in macro.h or config.h

Thanks for the fix.

[marcalff]

Perhaps move #include <regex> to the source file which need it instead of putting into this generic header file?

Good idea, see my latest review comments.