Replace protobuf with protox by alco · Pull Request #30 · electric-sql/elixir-otel-metric-exporter

alco · 2026-03-19T17:26:53Z

Summary

Replaces the protobuf dependency with protox to eliminate the duplicate Google.Protobuf.* module conflict described in Unify protobuf libraries to remove protobuf version pin electric#4018
Regenerated all protobuf message modules using mix protox.generate from the vendored OpenTelemetry .proto files (submodule)
All 44 existing tests pass, compilation is clean with --warnings-as-errors
Version bumped to 0.5.0

Key changes

mix.exs: {:protobuf, "~> 0.15"} → {:protox, "~> 2.0"}
Encoding: Protobuf.encode_to_iodata/1 → Protox.encode!/1 (returns {iodata, size})
Decoding: Protobuf.decode/2 → Protox.decode!/2, Module.decode/1 → Module.decode!/1
Enums: SeverityNumber.value(:ATOM) → just :ATOM (protox enum fields accept atoms directly)
Error handling: Protobuf.EncodeError → Protox.EncodingError
Strictness: protox is stricter about string field defaults (nil → ""), nil metric values → 0/0.0

Generated code

The 6 old .pb.ex files (584 lines) were replaced with a single opentelemetry_proto.ex (10,739 lines) generated by mix protox.generate. The generated file includes all OpenTelemetry proto modules (metrics, logs, common, resource, collector services). All module names are preserved with the OtelMetricExporter.Opentelemetry.Proto.* namespace.

Test plan

mix compile --warnings-as-errors passes
All 44 tests pass
Manual verification in a downstream project (electric/stratovolt) that the protobuf version pin can be removed

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Generated using mix protox.generate with --namespace=OtelMetricExporter. All module names are preserved (e.g. OtelMetricExporter.Opentelemetry.Proto.Common.V1.AnyValue). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- otel_api.ex: Protobuf.encode_to_iodata -> Protox.encode! (returns {iodata, size}) - protocol.ex: Use atoms directly for severity numbers instead of SeverityNumber.value/1 - protocol.ex: Default event_name to "" instead of nil (protox validates string fields) - metric_store.ex: Default unit to "" and description to "" when nil - metric_store.ex: Default nil metric values to 0/0.0 instead of nil - test files: Protobuf.decode -> Protox.decode!, Module.decode -> Module.decode! Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Minor version bump for protobuf -> protox migration. Public API unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5ca070e11

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-19T17:34:26Z

+  defp convert_value(nil, :int), do: {:as_int, 0}
+  defp convert_value(nil, :double), do: {:as_double, 0.0}


Skip nil measurements instead of coercing them to zero

If a Telemetry.Metrics.last_value/2 uses an optional measurement key or a measurement callback returns nil, handle_metric/4 passes that nil through to convert_value/2. Before this migration the OTLP value oneof was left unset, but these new clauses now emit a real 0/0.0 datapoint instead, so missing measurements are silently reported as zero-valued gauges. That corrupts exported metrics in exactly the cases this comment says we are trying to preserve.

Useful? React with 👍 / 👎.

This needs to be addressed: we must avoid emitting data points using default values where the source measurement didn't have any datapoints to begin with.

chatgpt-codex-connector · 2026-03-19T17:34:27Z

+      @spec decode(binary()) :: {:ok, t()} | {:error, any()}
+      def decode(bytes) do
+        {:ok, decode!(bytes)}


Preserve decode/1 return shape on public proto modules

The generated OtelMetricExporter.Opentelemetry.Proto.* modules are exposed under this library's namespace, and the previous use Protobuf implementation returned the decoded struct directly from decode/1. After regeneration, ExportMetricsServiceRequest.decode/1 now returns {:ok, struct} instead, so any downstream code still doing what our pre-migration call sites did—pipe bytes into decode/1 and pattern-match on %ExportMetricsServiceRequest{}—will break at runtime after upgrading. If this release is meant to be source-compatible, these modules need a shim or the old API preserved.

Useful? React with 👍 / 👎.

The change in the return value shape looks unjustified. Revert it to keep API compatibility with existing users of the library.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

P1: Skip nil measurements instead of coercing to zero. When extract_measurement returns nil (e.g. measurement key not in event), handle_metric now skips writing to ETS rather than emitting a datapoint with a zero default value. The convert_value(nil, ...) fallback clauses are removed since nil values no longer reach that code path. P2: Restore decode/1 to return the struct directly instead of {:ok, struct}, preserving API compatibility with the previous protobuf-based implementation. P3: Remove 8 unused protox-generated modules (LogRecordFlags, DataPointFlags, ExportLogsPartialSuccess, ExportLogsServiceResponse, ExportMetricsPartialSuccess, ExportMetricsServiceResponse, LogsData, MetricsData) reducing the generated file from ~10.7K to ~9.2K lines. Modules referenced transitively by used code (Exemplar, ExponentialHistogram*, Summary*, etc.) are retained. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

alco · 2026-03-20T13:53:39Z

Follow-up addressing review feedback

Pushed 3 commits addressing the review feedback:

P1 (nil measurements): handle_metric/4 now skips writing when the extracted measurement value is nil, instead of emitting zero-valued datapoints. The convert_value(nil, ...) fallback clauses are removed. A new test verifies this behavior.

P2 (decode/1 return shape): All decode/1 functions now return the struct directly, restoring API compatibility with the old protobuf library.

P3 (large generated file): Removed 8 unused modules (~1,460 lines, 13.8% reduction). The file went from ~10.7K to ~9.2K lines. Transitive dependency analysis determined that 7 additional modules (Exemplar, ExponentialHistogram, ExponentialHistogramDataPoint, ExponentialHistogramDataPoint.Buckets, Summary, SummaryDataPoint, SummaryDataPoint.ValueAtQuantile) must be retained because they are referenced by used modules -- for example, Exemplar is referenced by NumberDataPoint and HistogramDataPoint, and ExponentialHistogram/Summary are oneof variants in the Metric struct. Removing these would require modifying the protobuf decode/encode logic in the used modules, which could break wire compatibility.

Justification for the single-file approach: Protox generates all modules for a given set of .proto files into a single file by default. This is the standard protox workflow (as opposed to protobuf which generated one file per .proto). The single file ensures consistent compilation order and avoids issues with cross-module references between proto definitions. The 6 old .pb.ex files totaled 584 lines, but they relied on the protobuf library doing heavy lifting at compile time via use Protobuf. The protox-generated code is self-contained, which accounts for the size increase.

All 45 tests pass.

alco and others added 7 commits March 19, 2026 17:37

Add agent task files for protox migration

0fa9e92

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace protobuf dep with protox in mix.exs

d95c8ec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace protobuf .pb.ex files with protox-generated modules

af20e8d

Generated using mix protox.generate with --namespace=OtelMetricExporter. All module names are preserved (e.g. OtelMetricExporter.Opentelemetry.Proto.Common.V1.AnyValue). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Bump version to 0.5.0

6a7b42a

Minor version bump for protobuf -> protox migration. Public API unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Update progress log with implementation details

e5ca070

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove stale protobuf entry from mix.lock

df9b530

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed Mar 19, 2026

View reviewed changes

erik-the-implementer and others added 3 commits March 20, 2026 14:47

Add agent task files for PR #30 review follow-up

6348fcd

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update progress log with implementation details

3f09e1a

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

alco mentioned this pull request Mar 23, 2026

Remove protobuf pin electric-sql/electric#4016

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace protobuf with protox#30

Replace protobuf with protox#30
alco wants to merge 10 commits intomainfrom
protox-migration

alco commented Mar 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 19, 2026

Uh oh!

alco Mar 20, 2026

Uh oh!

chatgpt-codex-connector Bot Mar 19, 2026

Uh oh!

alco Mar 20, 2026

Uh oh!

alco commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		defp convert_value(nil, :int), do: {:as_int, 0}
		defp convert_value(nil, :double), do: {:as_double, 0.0}

Conversation

alco commented Mar 19, 2026

Summary

Key changes

Generated code

Test plan

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

alco Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

alco Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

alco commented Mar 20, 2026

Follow-up addressing review feedback

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants