[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.27225% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.58%. Comparing base (c7b64ec) to head (ca544c2).

Files with missing lines	Patch %	Lines
exporters/otlp/src/otlp_json_converter.cc	67.86%	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.69%   86.58%   -0.11%     
==========================================
  Files         527      532       +5     
  Lines       20902    20577     -325     
==========================================
- Hits        18119    17814     -305     
+ Misses       2783     2763      -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.34%)`	⬆️
... 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: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%

… backend

…iter backend

Corrections to the review fixes above, kept together because they share files and none of them is independently reviewable. Two would have failed CI on Bazel. The three OTLP/JSON exporter tests include otlp_stub_json_writer.h, and the trace one otlp_stub_json_reader.h, but their cc_test srcs listed only the .cc, which a sandboxed build cannot resolve; the headers join srcs the way open-telemetry#4556's targets already list the writer stub. And otel_cc_benchmark takes (name, srcs, deps, tags) with no kwargs, so the target_compatible_with on the new benchmark would have failed the whole BUILD file at load time -- dropped, matching otlp_file_client_json_benchmark, which carries no guard either. A null factory passed to the five-argument exporter constructors was stored and then dereferenced on export. Null already means "use the default" on the runtime options, so it means the same there now, and a test covers it. The installed-export test could not compile at all: this branch's additions to it ate the closing brace of the OtlpHttpBuilderUtils case, so every test after it parsed as a nested class. Closing it exposed the rest -- with no JSON writer backend installed, every case that resolves a default factory terminates. That is this branch's three factory-less OTLP/JSON exporter cases and its three builder cases, and upstream's three protobuf builder cases, which ask for encoding: json. All nine now sit behind ENABLE_JSON_WRITER_NLOHMANN, which the installed writer target carries as a PUBLIC definition and which therefore states exactly whether an install has a default backend. It is the same guard open-telemetry#4556 applied to its own default-writer tests. The component test's CMake keeps its TARGET check, which is about linking rather than about the source. An install without a backend is usable through the constructors that take both factories, which the unit tests cover; what the installed export can check there is that the targets link, and it now runs to completion and passes instead of aborting. The benchmark was missing <chrono> and opentelemetry/common/timestamp.h, which include-what-you-use would have reported. The rest is cmake-format over the build files this branch re-indented, and one garbled comment in the nlohmann reader. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYY7esG61tBypp7VfYZiyv

ltowarek · 2026-10-19T20:28:37Z

@lalitb is there anything else required from me before merging this change?

open-telemetry#4715 replaced the local HexEncode helper in the OTLP HTTP and file clients with common::detail::LowerHexDigit. This branch had already moved that code into otlp_json_converter.cc, so the conflicting hunks resolve to this branch's deletion and the same replacement is applied to the moved copy, which keeps open-telemetry#4715's change rather than reverting it.

Corrections to the review fixes above, kept together because they share files and none of them is independently reviewable. Two would have failed CI on Bazel. The three OTLP/JSON exporter tests include otlp_stub_json_writer.h, and the trace one otlp_stub_json_reader.h, but their cc_test srcs listed only the .cc, which a sandboxed build cannot resolve; the headers join srcs the way open-telemetry#4556's targets already list the writer stub. And otel_cc_benchmark takes (name, srcs, deps, tags) with no kwargs, so the target_compatible_with on the new benchmark would have failed the whole BUILD file at load time -- dropped, matching otlp_file_client_json_benchmark, which carries no guard either. A null factory passed to the five-argument exporter constructors was stored and then dereferenced on export. Null already means "use the default" on the runtime options, so it means the same there now, and a test covers it. The installed-export test could not compile at all: this branch's additions to it ate the closing brace of the OtlpHttpBuilderUtils case, so every test after it parsed as a nested class. Closing it exposed the rest -- with no JSON writer backend installed, every case that resolves a default factory terminates. That is this branch's three factory-less OTLP/JSON exporter cases and its three builder cases, and upstream's three protobuf builder cases, which ask for encoding: json. All nine now sit behind ENABLE_JSON_WRITER_NLOHMANN, which the installed writer target carries as a PUBLIC definition and which therefore states exactly whether an install has a default backend. It is the same guard open-telemetry#4556 applied to its own default-writer tests. The component test's CMake keeps its TARGET check, which is about linking rather than about the source. An install without a backend is usable through the constructors that take both factories, which the unit tests cover; what the installed export can check there is that the targets link, and it now runs to completion and passes instead of aborting. The benchmark was missing <chrono> and opentelemetry/common/timestamp.h, which include-what-you-use would have reported. The rest is cmake-format over the build files this branch re-indented, and one garbled comment in the nlohmann reader.

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