[EXPORTER] Add a JsonWriter interface for OTLP/JSON serialization by ltowarek · Pull Request #4556 · open-telemetry/opentelemetry-cpp

ltowarek · 2026-09-10T21:39:25Z

The OTLP HTTP and file clients each carried their own copy of the same reflection-based protobuf-to-JSON converter. Replace both with a single converter that emits through a JsonWriter interface, and let callers supply their own writer through a JsonWriterFactory, as they already can for the HTTP client. The emitted JSON is unchanged.

Fixes #2541

Changes

The factory is injected as a json_writer_factory field on the existing
runtime options, rather than as new constructor and factory overloads - those
would have added roughly 50 parallel entry points across the exporters for one
injectable. No existing signature changes; a null factory selects the default
nlohmann backend.

OTELCPP_WITH_JSON_WRITER_NLOHMANN controls whether that backend is compiled.
OTELCPP_WITH_OTLP_HTTP and OTELCPP_WITH_OTLP_FILE no longer force
nlohmann-json on, so a consumer supplying their own writer can drop it from the
build entirely.

Golden-body tests pin the complete request body for traces, metrics and logs on
both exporters, so the unchanged-output claim rests on the emitted bytes.

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

The OTLP HTTP and file clients each carried their own copy of the same reflection-based protobuf-to-JSON converter. Replace both with a single converter that emits through a JsonWriter interface, and let callers supply their own writer through a JsonWriterFactory. The emitted JSON is unchanged. The factory is injected the way each family already injects thread_instrumentation: as a field on OtlpHttpClientOptions for the HTTP exporters, and on OtlpFileClientRuntimeOptions for the file ones. No existing constructor or factory signature changes, and a null factory selects the default backend.

codecov · 2026-09-10T22:49:53Z

Codecov Report

❌ Patch coverage is 78.55297% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.38%. Comparing base (eb92442) to head (8ea7620).

Files with missing lines	Patch %	Lines
exporters/otlp/src/otlp_json_converter.cc	68.79%	54 Missing ⚠️
exporters/otlp/src/otlp_json_writer_nlohmann.cc	72.90%	29 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4556      +/-   ##
==========================================
- Coverage   86.50%   86.38%   -0.12%     
==========================================
  Files         525      530       +5     
  Lines       20465    20135     -330     
==========================================
- Hits        17701    17391     -310     
+ Misses       2764     2744      -20

Files with missing lines	Coverage Δ
...xporters/otlp/detail/default_json_writer_factory.h	`100.00% <100.00%> (ø)`
...de/opentelemetry/exporters/otlp/otlp_file_client.h	`100.00% <ø> (ø)`
.../exporters/otlp/otlp_file_client_runtime_options.h	`100.00% <ø> (ø)`
...de/opentelemetry/exporters/otlp/otlp_http_client.h	`100.00% <ø> (ø)`
...xporters/otlp/otlp_http_exporter_runtime_options.h	`100.00% <ø> (ø)`
...lp/otlp_http_log_record_exporter_runtime_options.h	`100.00% <ø> (ø)`
...s/otlp/otlp_http_metric_exporter_runtime_options.h	`100.00% <ø> (ø)`
...de/opentelemetry/exporters/otlp/otlp_json_writer.h	`100.00% <100.00%> (ø)`
...elemetry/exporters/otlp/otlp_json_writer_factory.h	`100.00% <100.00%> (ø)`
exporters/otlp/src/otlp_file_client.cc	`72.84% <100.00%> (+2.13%)`	⬆️
... and 6 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ction Advances third_party/opentelemetry-cpp to ltowarek/opentelemetry-cpp@2fb8632c, otlp-json-no-protobuf rebased onto the current json-writer-seam (open-telemetry/opentelemetry-cpp#4556). The previous pin, 4a8e7bf4, is no longer on any fork branch. PR1 now takes the JsonWriterFactory as a json_writer_factory field on the exporters' runtime options instead of constructor overloads, so the JTAG file exporters and the protobuf-encoding OTLP/HTTP exporters set it there through CjsonRuntimeOptions<T>(). The OTLP/JSON exporters still take both factories as constructor arguments and are unchanged. Not yet built with ESP-IDF. Part of #102 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FvAjrPuAUXJLv8ZpUxh96

lalitb · 2026-09-12T20:09:12Z


-    std::string post_body_json =
-        json_request.dump(-1, ' ', false, nlohmann::detail::error_handler_t::replace);
+    std::string post_body_json = json_writer->ToString();


Could we check ok() after ToString() too? That call can fail, and we currently continue with an empty body. The file client needs the same check. Also better to have a test with a writer that fails during ToString() for both.

Done in 1a9780e: both clients now fail the export when ok() is false after ToString() or when Create() returns nullptr, with tests for each case in both clients.

lalitb · 2026-09-12T20:10:07Z

    : is_shutdown_(false),
      options_(std::move(options)),
      http_client_(std::move(http_client)),
+      json_writer_factory_(options_.json_writer_factory ? options_.json_writer_factory


Could we resolve the default JSON writer only when the content type is JSON? With nlohmann disabled and no custom writer, this currently calls std::terminate() even for binary HTTP export. Please also cover binary export without a JSON backend in a test.

Done in fc0f530: the default factory is resolved only for kJson content, and BinaryExportDoesNotNeedAJsonWriterFactory covers binary export.

lalitb · 2026-09-12T20:11:56Z

+  opentelemetry_add_pkgconfig(
+    exporter_otlp_json_converter
+    "OpenTelemetry OTLP - JSON Converter"
+    "Shared reflection-based protobuf-to-JSON converter for OTLP/JSON exporters."
+    "opentelemetry_sdk opentelemetry_common opentelemetry_exporter_otlp_json_writer"


Could we update the full pkg-config dependency chain for these new libraries? The HTTP/file client packages do not include the converter, the writer package does not include the enabled nlohmann backend, and opentelemetry_sdk has no .pc file. An installed static consumer should be able to link using pkg-config with the backend enabled or disabled.

Done in 803fcce: the client packages now require the writer and converter, the writer requires the nlohmann backend when enabled, the converter requires opentelemetry_proto instead of opentelemetry_sdk, and ci/verify_packages.sh checks the new packages.

lalitb · 2026-09-12T20:12:54Z

+    if (nlohmann::json *slot = ClaimSlot())
+    {
+      *slot = opentelemetry::sdk::common::Base64Escape(
+          std::string(reinterpret_cast<const char *>(data), size));


Could we pass a nostd::string_view directly to Base64Escape? It already accepts one, so creating a string here adds a full copy of every bytes value before encoding it.

Done in 30f1738, thanks.

lalitb · 2026-09-12T20:13:54Z

+      Fail("Key() called twice without an intervening value");
+      return;
+    }
+    pending_key_     = std::string(key.data(), key.size());


Could we use pending_key_.assign(key.data(), key.size()) here? That lets us reuse the existing capacity instead of constructing another string for every key. Longer field names can otherwise allocate on each call.

Applied in 7eec588, then replaced in b9530db by inserting the member in Key() and writing the value in place, which removes the key buffer entirely.

lalitb · 2026-09-12T20:16:32Z

+  virtual void BeginArray() noexcept  = 0;
+  virtual void EndArray() noexcept    = 0;
+
+  virtual void Key(nostd::string_view key) noexcept = 0;


Could we document the expectations for custom writers here? In particular, string views must be consumed or copied before the call returns, WriteBytes produces base64, and callers need to know how ok() and ToString() behave after a failure.

The factory should also say whether Create() can return null or throw, and whether calls may happen concurrently.

Done in 6a869fc, with the whole documented contract covered by a reusable test suite in 5c4e118.

lalitb · 2026-09-12T20:19:11Z

Thanks for the PR. The approach looks find at initial glance. Could we add a benchmark comparing JSON serialization before and after this change? It would help check whether the new writer adds any overhead for users keeping the default nlohmann backend. Ideally, measure serialization without HTTP or file I/O.

…s null

…P client

…ON writer

…verter

ltowarek · 2026-09-13T15:23:54Z

Thanks for the PR. The approach looks find at initial glance. Could we add a benchmark comparing JSON serialization before and after this change? It would help check whether the new writer adds any overhead for users keeping the default nlohmann backend. Ideally, measure serialization without HTTP or file I/O.

Added in f35605b (serialization only, no I/O); after the optimizations in b9530db, 94b2f64 and ff92b5b, the PR is on par with main for 1 span and faster for larger batches (median of 8 interleaved runs, -O3, i5-4590):

Spans	main (`1143e55`)	this PR	Difference
1	14.74 µs	14.76 µs	+0.1%
10	112.78 µs	109.94 µs	−2.5%
100	1,309.96 µs	1,286.79 µs	−1.8%
1000	15,074.13 µs	14,592.15 µs	−3.2%

ltowarek requested a review from a team as a code owner September 10, 2026 21:39

Fix Bazel build

7937ce2

Fix CI

dd3964b

ltowarek mentioned this pull request Sep 11, 2026

Token-based JSON serialization interface for protobuf-free OTLP/JSON ltowarek/esp-opentelemetry-cpp#102

Open

lalitb reviewed Sep 12, 2026

View reviewed changes

ltowarek added 11 commits September 13, 2026 12:21

Pass a string_view to Base64Escape in the nlohmann JSON writer

30f1738

Reuse the pending key buffer in the nlohmann JSON writer

7eec588

Fail the OTLP/JSON export when ToString() fails or the factory return…

1a9780e

…s null

Resolve the default JSON writer only for JSON content in the OTLP HTT…

fc0f530

…P client

Complete the pkg-config dependency chain of the OTLP JSON libraries

803fcce

Document the JsonWriter and JsonWriterFactory contracts

6a869fc

Test the JsonWriter contract with a suite shared by writer backends

5c4e118

Add a benchmark for OTLP/JSON serialization without I/O

f35605b

Avoid key copies and initializer_list construction in the nlohmann JS…

b9530db

…ON writer

Format hex IDs and 64-bit integers into stack buffers in the JSON con…

94b2f64

…verter

Fetch reflection once per message in the JSON converter

ff92b5b

ltowarek added 4 commits September 13, 2026 17:24

Merge main into json-writer-seam

5fdd156

Make the concurrency contract test constants static for MSVC

07b30c8

Fix clang-tidy and include-what-you-use findings in the OTLP JSON tests

a07346a

Merge main into json-writer-seam

8ea7620

Uh oh!

Conversation

ltowarek commented Sep 10, 2026

Changes

Uh oh!

codecov Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb commented Sep 12, 2026

Uh oh!

ltowarek commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 10, 2026 •

edited

Loading