If nlohmann_json is missing in customer's build system, then install it from submodule

[maxgolov]

Fixes #843

Problem Description

Previous fix ( #865 ) works well for Linux and Mac. But missed one important detail for Windows: nlohmann::json package is unconditionally required by ETW exporter on Windows, because json.hpp header is included in public ETW exporter headers. And because it is added as INTERFACE package. When it is added as INTERFACE (not PRIVATE), means it must to be installed on customer machine into CMake cache. Otherwise CMake won't let the build to proceed. Actually the same is true for zPages, but for zPages we just did not enforce it via target_link_libraries ... INTERFACE nlohmann_json - so it was "passing" without a hard-check.

There are at least two workarounds available:

• use vcpkg install nlohmann_json to install the package before the buiild ; OR
• pass -DJSON_Install=ON to CMake when compiling for Windows. That'll install the nlohmann_json before building the rest of code.

With the fix, the workaround of passing -DJSON_Install=ON is no longer needed.

Changes

If nlohmann::json is not installed on customer system, then we use our instance of it from our submodule. And we install it. Because it is unconditionally required by ETW exporter, zPages, both part of default build. I also improved the logic to check that submodule directory exists. We discussed it in a previous related PR. If submodule does not exist and the package is not installed from elsewhere, then the build fails with a recommendation / instructions how to fix it by recursively cloning with submodules :

Customer reporting the issue confirmed that the fix / idea behind the fix is working well:

UPDATE: also when we package the source tarball, allow the API-only build to avoid including nlohmann_jsonhpp library.

[codecov]

Codecov Report

Merging #870 (8c8c960) into main (c0b459e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #870   +/-   ##
=======================================
  Coverage   95.50%   95.50%           
=======================================
  Files         156      156           
  Lines        6620     6620           
=======================================
  Hits         6322     6322           
  Misses        298      298           

[maxgolov]

@lidavidm - I hope that should fix the build for API-only build.

[lidavidm]

I can confirm this fixes the API-only build, thanks!

It would be nice if we could still build the rest of the library that doesn't require json without this dependency, though (e.g. the SDK), though I can understand if that would get complicated.

[maxgolov]

@lidavidm - it happens that in the majority of cases (80% scenarios I'd say), we do actually require nlohmann/json.hpp, e.g. for ETW on Windows, for zPages (internal monitor of SDK statistics), for Zipkin exporter, as well as for OTLP-HTTP exporter. Plus for all tests, and generally for examples too.

Since we got you covered well - the case when the build is done only for API and nothing else, json is now optional for that scenario.. I think it's gonna be too much of a hassle to make json optional for the rest. As you noticed, it gets complicated - given that most components require it. And we give two alternatives - either consume json library from user-provided deps. Or use the one we cloned from recursive submodules, both options now work cross-plat.

We can probably consider further improvements in a follow-up PR? But as low priority. Since all immediate scenarios have been addressed.

[lalitb]

I agree on having further cleanup in a follow-up PR. I think the most common usage scenario would be instrumentation libraries devs using otel-cpp api, and application developers using otel-gRPC exporter. And both these scenarios don't depend on JSON and so shouldn't fail. Though I do see the hassle involved here to work it cross-platform.

Thanks for fixing the API only build :)

[maxgolov]

@lalitb - My worry is that OTLP-HTTP exporter may also be common. This one does require JSON. Same as Zipkin. Also I think the zPages project is kinda common across all languages. The idea is we use zPages for debugging OpenTelemetry in-proc.. that build flavor of final SDK dll / shared object bit, with zPages in it, would require json.hpp

While I agree we may not need it for OTLP-gRPC only build without zPages, in most other configurations we end up needing json.hpp anyways.

[lidavidm]

I totally understand. I think the biggest thing is just that the release tarballs don't include the submodules, but that's a Github issue.