Add preview feature-flag/macro for metrics

[lalitb]

Changes

The metrics implementation is not complete ( and may need a complete rewrite ), so it needs to be behind the feature-flag as we are planning for 1.0.0 RC ( 1.0.0 release is only for tracing signal, not metrics and logs).

As per the feature flag policy defined in Versioning.md - This PR just wraps all the metrics code (api, sdk, tests, exporter, example ) under ENABLE_METRICS_PREVIEW hashdef.

#ifdef ENABLE_METRICS_PREVIEW
..
..
#endif

The changes look huge as the clang-format does the indentation of preprocessor directives within this wrapper.

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

[codecov]

Codecov Report

Merging #745 (0d7ab3e) into main (42e9330) will increase coverage by 1.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #745      +/-   ##
==========================================
+ Coverage   94.75%   95.95%   +1.20%     
==========================================
  Files         217      174      -43     
  Lines        9919     7127    -2792     
==========================================
- Hits         9399     6839    -2560     
+ Misses        520      288     -232     

Impacted Files	Coverage Δ
...ude/opentelemetry/common/key_value_iterable_view.h	88.88% <0.00%> (-11.12%)	⬇️
api/include/opentelemetry/nostd/mpark/variant.h	97.31% <0.00%> (-0.04%)	⬇️

[maxgolov]

I'm OK with this change in general. But instead of changing 51 files - it could have also been handled in a more elegant fashion by a relatively small 3-4 lines feature flag in CMake, i.e. NOT including any of these files as part of the build. It would have been a single option and a relatively isolated change in CMakeLists.txt, without needing to change the source code at all (because you are excluding files entirely). I would have done this differently for CMake..

I approve this only because I do not know how to handle the exclusion of directory in Bazel in more flexible way, i.e. how to handle build combos in Bazel. Simplest for Bazel would have been to add the entirety of metrics to .bazelignore. Because it is subject to rework anyways, i.e.

contents of .bazelignore

third_party
tools
out
api/include/opentelemetry/metrics
api/test/metrics
sdk/include/opentelemetry/sdk/metrics
sdk/src/metrics
sdk/test/metrics

to .bazelignore, and a corresponding WITH_METRICS flag to the root level CMakeLists.txt... Plus, corresponding removal - commenting out of prometheus-cpp from installation scripts. Because building and installing it takes a lot of time wasted for nothing, since we are excluding metrics by-default anyways. Rather remove that other stuff too.

[maxgolov]

Yeah, you wouldn't have needed it if you excluded the entire folder. It would've been just 2-3 build infra files changed instead of 51 source code files.

[lalitb]

@maxgolov Yes it would be definitely easy to ignore entire 'metrics' related folders in CMake build and also probably some way to do through bazel. But our releases are source archives, and idea is to add preview flag explicitly in source to ensure that metrics is not part of build irrespective of whatever build system is used for this released source (not just bazel or cmake).

[pyohannes]

Could we maybe additionally ignore folders related to metrics? Otherwise we end up with empty targets for metrics, which might be confusing for users.

[pyohannes]

Also, we should think about enabling ENABLE_METRIC_PREVIEW for our CI builds. Otherwise we won't catch regressions in the metric code.

[lalitb]

Could we maybe additionally ignore folders related to metrics? Otherwise we end up with empty targets for metrics, which might be confusing for users.

Yes Johannes, I did realize that after raising the PR. As you mentioned, better would be enabling metrics preview for our CI builds.