feat: enhance OTLP support for metrics, and traces#7
Conversation
WalkthroughThis PR adds metrics and traces test case configurations (correctness and performance variants) alongside a major refactoring of the OTLP generator to support all three signal types (logs, metrics, traces) over multiple transports (gRPC and HTTP). The generator now uses a reusable batch interface and signal-specific implementations. ChangesTest Case Configurations
OTLP Generator Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cases/otlp_metrics_to_otlp_correctness/configs/otel-collector.yaml (1)
14-15: 💤 Low valuegRPC receiver enabled in HTTP-only tests (both metrics and traces). Both correctness cases target only
subject:4318(HTTP), and both vmetric configs explicitly disable gRPC to "isolate the HTTP ingress path." However, both otel-collector configs enable gRPC (4317) and HTTP (4318) receivers. While the gRPC listeners won't receive traffic, this creates an asymmetry between subjects. Consider disabling gRPC in both otel-collector configs to match the test intent and ensure both subjects use the same HTTP-only ingress path.🤖 Prompt for 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. In `@cases/otlp_metrics_to_otlp_correctness/configs/otel-collector.yaml` around lines 14 - 15, The otel-collector YAML currently enables a gRPC receiver (grpc: endpoint: 0.0.0.0:4317) alongside the HTTP receiver, which conflicts with the tests' intent to be HTTP-only; remove or comment out the grpc receiver block (the grpc: endpoint: 0.0.0.0:4317 section) from the otel-collector configs used by cases/otlp_metrics_to_otlp_correctness so the collector only declares the HTTP receiver (endpoint: 0.0.0.0:4318), ensuring symmetry with the vmetric configs that disable gRPC.containers/generator/otel.go (1)
23-52: 💤 Low valueStale package documentation from logs-only version.
Lines 3-22 describe the new multi-signal behavior, but this block (lines 23-52) still references only logs ("LogRecord", "ResourceLogs → ScopeLogs → LogRecords", "body 'OTEL-'"). Consider consolidating or removing this legacy documentation to avoid confusion.
🤖 Prompt for 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. In `@containers/generator/otel.go` around lines 23 - 52, The top comment block contains stale logs-only wording (mentions "LogRecord", "ResourceLogs → ScopeLogs → LogRecords", and "body 'OTEL-<seq>'") that conflicts with the new multi-signal behavior; either remove or update those lines to describe multi-signal support (traces/metrics/logs), remove the hard-coded "OTEL-<seq>" reference, and consolidate the transport/behavior notes into a single coherent paragraph — edit the comment in otel.go to replace the legacy LogRecord-specific sentences with multi-signal descriptions and keep the existing transport details (HTTP/gRPC/protobuf/json, gzip, batching) intact.
🤖 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.
Nitpick comments:
In `@cases/otlp_metrics_to_otlp_correctness/configs/otel-collector.yaml`:
- Around line 14-15: The otel-collector YAML currently enables a gRPC receiver
(grpc: endpoint: 0.0.0.0:4317) alongside the HTTP receiver, which conflicts with
the tests' intent to be HTTP-only; remove or comment out the grpc receiver block
(the grpc: endpoint: 0.0.0.0:4317 section) from the otel-collector configs used
by cases/otlp_metrics_to_otlp_correctness so the collector only declares the
HTTP receiver (endpoint: 0.0.0.0:4318), ensuring symmetry with the vmetric
configs that disable gRPC.
In `@containers/generator/otel.go`:
- Around line 23-52: The top comment block contains stale logs-only wording
(mentions "LogRecord", "ResourceLogs → ScopeLogs → LogRecords", and "body
'OTEL-<seq>'") that conflicts with the new multi-signal behavior; either remove
or update those lines to describe multi-signal support (traces/metrics/logs),
remove the hard-coded "OTEL-<seq>" reference, and consolidate the
transport/behavior notes into a single coherent paragraph — edit the comment in
otel.go to replace the legacy LogRecord-specific sentences with multi-signal
descriptions and keep the existing transport details (HTTP/gRPC/protobuf/json,
gzip, batching) intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d06517f6-5817-4c08-9bef-22c9593473d8
📒 Files selected for processing (14)
cases/otlp_metrics_to_otlp_correctness/case.yamlcases/otlp_metrics_to_otlp_correctness/configs/otel-collector.yamlcases/otlp_metrics_to_otlp_correctness/configs/vmetric.ymlcases/otlp_metrics_to_otlp_performance/case.yamlcases/otlp_metrics_to_otlp_performance/configs/otel-collector.yamlcases/otlp_metrics_to_otlp_performance/configs/vmetric.ymlcases/otlp_traces_to_otlp_correctness/case.yamlcases/otlp_traces_to_otlp_correctness/configs/otel-collector.yamlcases/otlp_traces_to_otlp_correctness/configs/vmetric.ymlcases/otlp_traces_to_otlp_performance/case.yamlcases/otlp_traces_to_otlp_performance/configs/otel-collector.yamlcases/otlp_traces_to_otlp_performance/configs/vmetric.ymlcontainers/generator/main.gocontainers/generator/otel.go
Summary by CodeRabbit