Add Vapor tracing middleware (SignozVapor)#11
Conversation
Introduce SignozVapor library and SignozTracingMiddleware which creates OpenTelemetry server spans for incoming Vapor requests and extracts/injects W3C traceparent/tracestate. Add Vapor package dependency and update Package.resolved pins
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughAdds Vapor integration (SignozVapor library and middleware) and centralizes W3C trace-context parsing/serialization; updates package dependencies and lockfile to include Vapor and related packages. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Middleware as SignozTracingMiddleware
participant Tracer as Tracer/OTel
participant App as App/NextResponder
participant Response as Response
Client->>Middleware: HTTP Request (headers)
Note over Middleware: Extract W3C traceparent/tracestate
Middleware->>Tracer: Start server span ("<METHOD> <path>") with HTTP attrs
activate Tracer
Middleware->>App: Call next responder with active span context
activate App
App->>Response: Build response
deactivate App
Middleware->>Tracer: Set route pattern, status code, mark ok/error
Middleware->>Response: Inject traceparent/tracestate headers
Tracer->>Tracer: End span
deactivate Tracer
Middleware->>Client: HTTP Response (with trace headers)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a new SignozVapor library target that provides SignozTracingMiddleware, an AsyncMiddleware for Vapor that automatically creates OpenTelemetry .server spans for every incoming HTTP request. The middleware sets standard OTel HTTP semantic convention attributes, handles W3C trace context propagation (both extraction from request headers and injection into response headers), and updates span names with matched route patterns.
Changes:
- New
SignozVaportarget and product added toPackage.swiftwith Vapor as a package-level dependency - New
SignozTracingMiddlewareimplementing VaporAsyncMiddlewarewith OTel span creation, HTTP attribute recording, W3C traceparent/tracestate handling, and route pattern extraction - README updated to document the
SignozVapormiddleware usage instead of manual span boilerplate for Vapor projects
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
Package.swift |
Adds SignozVapor library product/target and vapor package dependency |
Sources/SignozVapor/SignozTracingMiddleware.swift |
New middleware that wraps Vapor requests in OTel server spans with HTTP attributes and W3C trace context propagation |
README.md |
Updates installation and Vapor usage sections to reference SignozVapor and SignozTracingMiddleware |
Package.resolved |
Auto-updated resolved dependencies including Vapor and its transitive dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/SignozVapor/SignozTracingMiddleware.swift (1)
14-145: Please add focused coverage for the new middleware.This lands route-pattern rewriting, W3C propagation, and error-path handling without any dedicated coverage in the PR. A small Vapor test app around incoming
traceparent,http.routerenaming, and thrown-error behavior would make this much safer to evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SignozVapor/SignozTracingMiddleware.swift` around lines 14 - 145, Add focused unit/integration tests exercising SignozTracingMiddleware: create a small Vapor test app that mounts a route with a parameter (to validate routePattern and http.route renaming in SignozTracingMiddleware.respond), send a request containing a valid W3C traceparent (and tracestate) header to assert extractTraceContext propagates into the created span and that injectTraceContext writes traceparent/tracestate into the response, and also add a handler that throws to assert the error-path sets span.status to error and still updates http.route before ending the span; use SignozTracingMiddleware in the app pipeline and assert span attributes (http.method, http.target, http.route, http.status_code) and response headers, referencing SignozTracingMiddleware.respond, SignozTracingMiddleware.extractTraceContext, SignozTracingMiddleware.injectTraceContext, and SignozTracingMiddleware.routePattern when locating code to test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 50-55: The README Vapor example is missing import Vapor which is
required because SignozVapor/@_exported import SignozSwift does not re-export
Vapor; update the snippet that currently shows import SignozVapor and usage of
Application by adding an explicit import Vapor (so Application is in scope)
while keeping import SignozVapor for the tracing middleware.
In `@Sources/SignozVapor/SignozTracingMiddleware.swift`:
- Around line 63-71: In the catch block of SignozTracingMiddleware (where you
currently set span.status and end the span), also set the "http.status_code"
attribute and replace the raw error string with a sanitized description: if the
caught error is a Vapor HTTP error (e.g., conforms to AbortError/has an HTTP
status), extract its numeric status code and the standard HTTP status
reason/description to set span.status = .error(description: reason) and
span.setAttribute(key: "http.status_code", value: .int(status.code)); otherwise
set span.status = .error(description: String(describing: type(of: error))) and
set a generic http.status_code (e.g., 500) or omit it—ensure you do this before
span.end(); keep route naming logic (Self.routePattern(route)) as-is.
---
Nitpick comments:
In `@Sources/SignozVapor/SignozTracingMiddleware.swift`:
- Around line 14-145: Add focused unit/integration tests exercising
SignozTracingMiddleware: create a small Vapor test app that mounts a route with
a parameter (to validate routePattern and http.route renaming in
SignozTracingMiddleware.respond), send a request containing a valid W3C
traceparent (and tracestate) header to assert extractTraceContext propagates
into the created span and that injectTraceContext writes traceparent/tracestate
into the response, and also add a handler that throws to assert the error-path
sets span.status to error and still updates http.route before ending the span;
use SignozTracingMiddleware in the app pipeline and assert span attributes
(http.method, http.target, http.route, http.status_code) and response headers,
referencing SignozTracingMiddleware.respond,
SignozTracingMiddleware.extractTraceContext,
SignozTracingMiddleware.injectTraceContext, and
SignozTracingMiddleware.routePattern when locating code to test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d4663e1-7ca4-44b2-a48d-6a3e10c02f84
📒 Files selected for processing (4)
Package.resolvedPackage.swiftREADME.mdSources/SignozVapor/SignozTracingMiddleware.swift
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: linux
- GitHub Check: macOS
🧰 Additional context used
🧬 Code graph analysis (1)
Sources/SignozVapor/SignozTracingMiddleware.swift (3)
Tests/SignozSwiftTests/SignozSwiftTests.swift (1)
spanKind(263-274)Sources/SignozSwift/Tracing/OTelTracingBridge.swift (1)
startSpan(15-50)Sources/SignozSwift/Tracing/BridgedSpan.swift (1)
end(77-81)
Replace the separate SignozVapor target with a package trait. The middleware now lives in SignozSwift behind #if SIGNOZ_VAPOR, activated via .package(..., traits: ["Vapor"]). This avoids pulling Vapor into consumers that don't need it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ryan Zhu (underthestars-zhy)
changed the title
Move traceparent/tracestate parsing and serialization into W3CTraceContext so OTelTracingBridge and SignozTracingMiddleware share a single implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Per the W3C Trace Context spec, only version "ff" is invalid. Unknown future versions should be accepted since the first 55 characters are guaranteed stable. Changed the guard from == "00" to != "ff" and accept >= 4 dash-delimited parts to allow extra fields from future versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract HTTP status from Vapor's AbortError when available, otherwise default to 500. Use type name instead of raw error string to avoid leaking internal details into span status. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move middleware from inline #if SIGNOZ_VAPOR to a dedicated SignozVapor target. Vapor is fetched as a package dependency but only compiled when a consumer depends on SignozVapor. Remove traits machinery. Make W3CTraceContext public so SignozVapor can reuse it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ryan Zhu (underthestars-zhy)
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let parts = traceparent.split(separator: "-") | ||
| guard parts.count >= 4, parts[0] != "ff" else { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The W3C version check was changed from parts[0] == "00" (strict version-00-only) to parts[0] != "ff" (forward-compatible, per W3C spec). This is a behavioral change that's called out in the PR description, but no tests cover it. There should be tests verifying that: (1) a future version like "01" is accepted, and (2) version "ff" is rejected. The existing test suite covers W3C trace context propagation at line 771 but only tests with version "00".
| let builder = Signoz.tracer.spanBuilder(spanName: "\(method) \(path)") | ||
| builder.setSpanKind(spanKind: .server) | ||
|
|
||
| // Extract W3C traceparent/tracestate from request headers | ||
| if let traceparent = request.headers.first(name: W3CTraceContext.traceparentKey), | ||
| let parentContext = W3CTraceContext.parse( | ||
| traceparent: traceparent, | ||
| tracestate: request.headers.first(name: W3CTraceContext.tracestateKey) | ||
| ) { | ||
| builder.setParent(parentContext) | ||
| } | ||
|
|
||
| let span = builder.startSpan() | ||
| span.setAttribute(key: "http.method", value: .string(method)) | ||
| span.setAttribute(key: "http.target", value: .string(path)) | ||
| span.setAttribute(key: "http.scheme", value: .string(request.url.scheme ?? "http")) | ||
|
|
||
| do { | ||
| let response = try await OpenTelemetry.instance.contextProvider | ||
| .withActiveSpan(span) { | ||
| try await next.respond(to: request) | ||
| } | ||
|
|
||
| // Route is available after the responder chain resolves the route | ||
| if let route = request.route { | ||
| let pattern = Self.routePattern(route) | ||
| span.name = "\(method) \(pattern)" | ||
| span.setAttribute(key: "http.route", value: .string(pattern)) | ||
| } |
There was a problem hiding this comment.
When no route is matched (e.g., 404 responses), the span name retains the concrete request path (e.g., GET /users/12345/foo/bar). This can cause a cardinality explosion in your trace backend since every unique URL path becomes a separate span name. Per OpenTelemetry HTTP semantic conventions, the span name should fall back to just the HTTP method (e.g., GET) when no route pattern is available. Consider updating the span name to just method (or "\(method) \(path)" only as a temporary initial name) and then after the responder chain, setting it to "\(method) \(pattern)" if a route exists, or just method if no route was matched.
Summary
SignozVaporlibrary target withSignozTracingMiddlewarethat automatically creates.serverspans for every incoming Vapor HTTP requestSignozVaporand registering the middleware:app.middleware.use(SignozTracingMiddleware())W3CTraceContextutility, deduplicating code betweenOTelTracingBridgeand the new middlewareff, not require00)Usage
Changes
Package.swiftSignozVaporproduct and target (depends onSignozSwift+Vapor)vaporpackage dependency (from: "4.89.0")Sources/SignozVapor/SignozTracingMiddleware.swift(new)SignozTracingMiddleware: AsyncMiddlewarewraps each request in a.serverspanhttp.method,http.target,http.scheme,http.status_code,http.routeGET /users/:id) when available, falls back to the concrete path.errorfor 5xx responses; extracts status code from Vapor'sAbortErroron throwtraceparent/tracestatefrom request headers to parent spans in distributed tracestraceparent/tracestateinto response headers for downstream propagationSignozSwiftso users only needimport SignozVaporSources/SignozSwift/Tracing/W3CTraceContext.swift(new)parse(traceparent:tracestate:)andserialize(_:)methodsOTelTracingBridge(gRPC) andSignozTracingMiddleware(Vapor)ff)Sources/SignozSwift/Tracing/OTelTracingBridge.swiftinject/extractto use sharedW3CTraceContextutilityREADME.mdSignozVaporproductTest plan
SignozVaportarget builds successfullySignozSwifttarget still builds independentlytraceparentheader, confirm span is parented correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores