fix: use protobuf exporters for http/protobuf and add option http/json to keep old behavior

[thomasbonenfant]

Description

As noted in Issue #50 opentelemetry http exporters were sending traces as application/json. This results in 415 status code from Arize Phoenix.

This PR makes http/protobuf use the opentelemetry/exporter-*-otlp-proto.

In order to keep the old behavior with JSON payloads a "http/json" configuration option was added that uses the opentelemetry/exporter-*-http exporters.

Tests

I tested this on Arize Phoenix. The request is no longer rejected with a 415 Unsupported Media Type and the trace is correctly added.

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Chore (dependency updates, etc.)

Checklist

• I have read the CONTRIBUTING.md document
• My code follows the style guidelines of this project
• bun run lint passes with no errors
• bun run check:jsdoc-coverage passes with no errors
• bun run typecheck passes with no errors
• bun test passes with no errors
• I have added tests that prove my fix is effective or that my feature works
• I have updated the documentation accordingly
• My commits follow the Conventional Commits specification

Related issues

Fixes #50

Additional context

No additional context

Summary by CodeRabbit

• New Features

  • Added HTTP/JSON protocol support for OpenTelemetry exporters alongside gRPC and HTTP/Protobuf.
  • HTTP protocol endpoints now automatically append signal-specific paths for traces, metrics, and logs.

• Documentation

  • Updated configuration docs to include HTTP/JSON guidance and endpoint usage details.

• Chores

  • Added OTLP proto exporters for logs, metrics, and traces to dependencies.

• Tests

  • Added tests covering the new HTTP/JSON protocol behavior.

[coderabbitai]

Warning

Rate limit exceeded

@thomasbonenfant has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 31 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 09e4d0d1-440c-443d-88b6-e45d1707e5f6

📥 Commits

Reviewing files that changed from the base of the PR and between 4f344d0 and bc615ff.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock, !**/*.lock

📒 Files selected for processing (6)

• README.md
• package.json
• src/config.ts
• src/otel.ts
• tests/config.test.ts
• tests/otel.test.ts

📝 Walkthrough

Walkthrough

This PR adds http/json as an OTLP transport option, updates config mapping for OPENCODE_OTLP_PROTOCOL, imports proto HTTP exporters, and updates setupOtel() to choose exporters and HTTP /v1/{traces,metrics,logs} paths for HTTP modes; tests and README are updated accordingly.

Changes

OTLP Protocol Support

Layer / File(s)	Summary
Protocol configuration type and loading src/config.ts, tests/config.test.ts, README.md	PluginConfig.protocol type expanded to `"grpc"
OTLP exporter dependencies and protocol-based selection package.json, src/otel.ts, tests/otel.test.ts, README.md	Added proto exporter dependencies (@opentelemetry/exporter-logs-otlp-proto, @opentelemetry/exporter-metrics-otlp-proto, @opentelemetry/exporter-trace-otlp-proto at ^0.213.0); setupOtel() imports proto and HTTP exporter variants and selects the correct exporter type and /v1/{signal} URL path for http/protobuf and http/json, preserving gRPC exporter behavior for grpc; README endpoint guidance updated for HTTP modes; tests assert exporter types for both HTTP modes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• DEVtheOPS/opencode-plugin-otel#23: Both PRs modify OTLP protocol plumbing in src/config.ts and src/otel.ts (HTTP vs gRPC exporter selection).
• DEVtheOPS/opencode-plugin-otel#39: Both modify src/otel.ts's exporter creation/switch logic and may intersect in exporter selection.
• DEVtheOPS/opencode-plugin-otel#46: Both touch setupOtel() telemetry setup and exporter configuration.

Suggested reviewers

• dialupdisaster

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: fixing http/protobuf to use protobuf exporters and adding http/json option for backward compatibility.
Linked Issues check	✅ Passed	The PR comprehensively addresses issue #50 by switching http/protobuf to use protobuf exporters and adding http/json support, with regression tests validating the exporter selection logic.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the OTLP protocol handling and supporting protobuf exporters; no unrelated modifications were introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dialupdisaster]

Thanks for the fix here. The exporter swap for http/protobuf looks right, and I’m fine with the added http/json option as well.

Could you add a regression test for the actual behavior change in src/otel.ts though?

Right now the new test only covers config parsing, but the bug in #50 was specifically that http/protobuf selected the JSON HTTP exporters instead of the protobuf ones. I’d like to see at least one test that validates exporter selection for http/protobuf, and ideally another that confirms http/json still routes to the existing HTTP/JSON exporters.

That would make this much safer to merge and would prevent this protocol mapping from drifting again later.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/otel.test.ts`:
- Around line 72-76: The afterEach teardown in the test uses sequential await
calls on providers?.tracerProvider.shutdown(),
providers?.loggerProvider.shutdown(), and providers?.meterProvider.shutdown()
which stops remaining shutdowns if one rejects; update the afterEach for
resilience by invoking all three shutdown promises concurrently with
Promise.allSettled (guarding against undefined
providers/tracerProvider/loggerProvider/meterProvider), wait for allSettled to
complete, optionally log or ignore individual rejections, and then set providers
= undefined; modify the afterEach function and references to
tracerProvider.shutdown, loggerProvider.shutdown, and meterProvider.shutdown
accordingly.
- Around line 24-26: The test currently dereferences internals directly
(meterProvider._sharedState.metricCollectors[0]!._metricReader._exporter,
loggerProvider._sharedState.activeProcessor.processors[0]!._exporter,
tracerProvider._activeSpanProcessor._spanProcessors[0]!._exporter) which will
throw opaque errors when those arrays are empty; add explicit guards that verify
metricCollectors.length > 0, activeProcessor.processors.length > 0, and
_spanProcessors.length > 0 (or use optional chaining and explicit assertions)
before accessing index 0, and if any are missing fail the test with a clear
message that the expected exporter is absent so assertions run only when
exporters are present.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9089b7fc-b658-49e8-874e-61886386884a

📥 Commits

Reviewing files that changed from the base of the PR and between 0ff599d and 4f344d0.

📒 Files selected for processing (1)

• tests/otel.test.ts

[thomasbonenfant]

Thanks for the review.

I added regression tests for the protocol mapping behavior. They currently assert against the internal structure of the OpenTelemetry SDK objects to verify which exporters setupOtel wires up. If you'd prefer a less white-box approach, I can extract the exporter selection logic from setupOtel and test it directly

[dialupdisaster]

Looks good! Thanks for contributing!