Tutorials: OpenTelemetry#3022
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an OpenTelemetry tutorial YAML with three chained APIs and a matching integration test. Expands Javadoc on the OpenTelemetry interceptor and exporter, and adjusts one exporter path check. ChangesOpenTelemetry Tutorial and Documentation
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as Gateway :2000
participant MicroserviceA as Microservice A :2001
participant MicroserviceB as Microservice B :2002
participant Jaeger as Jaeger OTLP :4317
Client->>Gateway: HTTP request
Gateway->>MicroserviceA: forward with traceparent header
MicroserviceA->>MicroserviceB: forward with traceparent header
MicroserviceB-->>MicroserviceA: static 200 response
MicroserviceA-->>Gateway: response
Gateway-->>Client: response
Gateway->>Jaeger: export spans via gRPC
MicroserviceA->>Jaeger: export spans via gRPC
MicroserviceB->>Jaeger: export spans via gRPC
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@core/src/main/java/com/predic8/membrane/core/interceptor/opentelemetry/exporter/OtlpExporter.java`:
- Around line 139-147: The secured flag description in OtlpExporter is
inconsistent with how getEndpointUrl() and buildGrpcExporter(...) actually use
it: when secured is true, the endpoint becomes https even for gRPC. Update the
Javadoc/@description on setSecured(boolean secured) to reflect that secured
affects both HTTP and gRPC endpoint URLs, or change the implementation to limit
secured to HTTP-only if that was the intended behavior.
- Around line 149-155: The HTTP path default handling in OtlpExporter#setPath is
using a reference comparison against an empty string, which can miss explicitly
configured empty values. Update the path default logic in OtlpExporter to check
for emptiness with isEmpty() when deciding whether to apply the /v1/traces
default, so both literal and configured empty strings are handled correctly.
In
`@distribution/src/test/java/com/predic8/membrane/tutorials/operation/OpenTelemetryTutorialTest.java`:
- Around line 59-69: The test traceparentIsPropagatedToBackend only waits for a
single traceparent substring, which can be satisfied by the Gateway alone and
does not prove end-to-end propagation. Update this test to follow the stronger
pattern used in OpenTelemetryExampleTest by capturing the trace ID from the
first hop and asserting matching traceparent logs across multiple downstream
hops (for example, Gateway plus Microservice A and B) using the existing console
event helper. Keep the assertion tied to the same trace ID so the test fails if
propagation breaks anywhere in the chain.
🪄 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: 19f5faed-8db8-4ed8-be0d-97285a64dcc3
📒 Files selected for processing (4)
core/src/main/java/com/predic8/membrane/core/interceptor/opentelemetry/OpenTelemetryInterceptor.javacore/src/main/java/com/predic8/membrane/core/interceptor/opentelemetry/exporter/OtlpExporter.javadistribution/src/test/java/com/predic8/membrane/tutorials/operation/OpenTelemetryTutorialTest.javadistribution/tutorials/operation/40-OpenTelemetry.yaml
|
/ok-to-test |
Summary by CodeRabbit
pathvalue (more reliable HTTP endpoint path resolution).traceparenthop behavior.