Skip to content

CI: Bump macOS runner to macos-26-intel#12

Merged
Ryan Zhu (underthestars-zhy) merged 9 commits into
mainfrom
feat/vapor-auto-tracing
Mar 10, 2026
Merged

CI: Bump macOS runner to macos-26-intel#12
Ryan Zhu (underthestars-zhy) merged 9 commits into
mainfrom
feat/vapor-auto-tracing

Conversation

@underthestars-zhy
Copy link
Copy Markdown
Member

@underthestars-zhy Ryan Zhu (underthestars-zhy) commented Mar 10, 2026

Summary

  • Bumps the CI macOS runner from macos-15-intel to macos-26-intel to fix build failures caused by swift-configuration requiring the macOS 15 SDK (Xcode 26+) for Data.bytes

Changes

.github/workflows/ci.yml

  • Changed runs-on: macos-15-intelruns-on: macos-26-intel

Test plan

  • CI pipeline passes on the new runner

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Vapor framework integration library with automatic HTTP request tracing
    • Introduced middleware for automatic request span creation with standard HTTP attributes (method, status code, route, etc.)
    • Enhanced W3C trace context propagation for distributed tracing
  • Documentation

    • Updated guides with Vapor integration setup and middleware usage examples
  • Chores

    • Updated CI runner image for macOS builds
    • Updated package dependencies to latest versions

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>
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>
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>
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>
Copilot AI review requested due to automatic review settings March 10, 2026 18:44
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 10, 2026

📝 Walkthrough

Walkthrough

This pull request introduces Vapor HTTP integration for OpenTelemetry tracing in SignozSwift. It adds a new SignozVapor package with automatic request tracing middleware, refactors W3C trace context handling into a reusable utility, updates dependencies to support Vapor 4.121.3, and updates documentation with Vapor-specific usage examples.

Changes

Cohort / File(s) Summary
CI/Infrastructure
.github/workflows/ci.yml
Updated macOS CI runner image from macos-15-intel to macos-26-intel.
Dependency Management
Package.swift, Package.resolved
Added Vapor 4.89.0 as a package dependency; added new SignozVapor library product and target. Updated resolved lock file with Vapor ecosystem dependencies (async-http-client, async-kit, console-kit, multipart-kit, routing-kit, websocket-kit) and updated swift-distributed-tracing to version 1.4.1.
Trace Context Utilities
Sources/SignozSwift/Tracing/W3CTraceContext.swift
New utility enum for W3C Trace Context parsing and serialization, providing standardized traceparent/tracestate handling with public static methods and constants for header keys.
Tracing Bridge Refactoring
Sources/SignozSwift/Tracing/OTelTracingBridge.swift
Refactored W3C trace context propagation to use centralized W3CTraceContext.parse() and serialize() methods, removing manual trace ID/span ID construction and explicit tracestate parsing.
Vapor Middleware
Sources/SignozVapor/SignozTracingMiddleware.swift
New AsyncMiddleware for Vapor that automatically creates OpenTelemetry server spans for HTTP requests, extracting incoming W3C trace context, setting standard HTTP attributes, deriving route patterns, and injecting response headers with trace context.
Documentation
README.md
Updated Vapor integration examples to use new SignozTracingMiddleware instead of manual span creation, documented middleware behavior and automatic attributes (http.method, http.target, http.scheme, http.route, http.status_code), and updated repository references to reflect opentelemetry-swift source.

Sequence Diagram

sequenceDiagram
    actor Client
    participant Middleware as SignozTracingMiddleware
    participant TraceContext as W3CTraceContext
    participant OTel as OpenTelemetry
    participant Responder as Downstream Handler
    participant Response as HTTP Response

    Client->>Middleware: HTTP Request (with optional traceparent/tracestate)
    Middleware->>TraceContext: parse(traceparent, tracestate)
    TraceContext->>TraceContext: Validate and decode W3C format
    TraceContext-->>Middleware: Remote SpanContext (if present)
    Middleware->>OTel: Start server span with context
    OTel-->>Middleware: Active span
    Middleware->>Middleware: Set HTTP attributes (method, target, scheme)
    Middleware->>Responder: Execute with active span context
    Responder-->>Middleware: Response
    Middleware->>OTel: Extract route pattern & status code
    OTel->>OTel: Enrich span (http.route, http.status_code)
    Middleware->>TraceContext: serialize(spanContext)
    TraceContext-->>Middleware: traceparent & tracestate strings
    Middleware->>Response: Inject trace context headers
    Middleware-->>Client: Enhanced Response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • photon-hq/SignozSwift#10: Modifies tracing bridge and W3C trace context propagation with overlapping changes to OTelTracingBridge and trace context handling.
  • photon-hq/SignozSwift#11: Introduces SignozTracingMiddleware for Vapor, W3CTraceContext utilities, and refactors OTelTracingBridge with nearly identical changes.

Suggested labels

release

Poem

🐰 A middleware so fine, with traces so bright,
Vapor requests now sparkle with OpenTel light!
W3C context flows seamlessly through,
Routes and spans dance—tracing made new! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title accurately describes the main CI infrastructure change (bumping macOS runner from macos-15-intel to macos-26-intel), but the changeset contains substantial feature additions (SignozVapor library, SignozTracingMiddleware, W3CTraceContext utility, Vapor integration) that are not reflected in the title. Revise the title to reflect the primary changes in the PR. Consider: 'Add Vapor tracing support with SignozVapor and SignozTracingMiddleware' or 'Add Vapor integration with automatic request tracing' if that's the main feature, or merge this as a separate CI-only PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/vapor-auto-tracing

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR expands the package with a Vapor integration layer and shared W3C Trace Context utilities, and updates CI configuration accordingly.

Changes:

  • Add a new SignozVapor library target/product with SignozTracingMiddleware for automatic Vapor HTTP request spans.
  • Introduce W3CTraceContext to centralize traceparent/tracestate parsing/serialization and refactor OTelTracingBridge to use it.
  • Update documentation and CI runner configuration.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
.github/workflows/ci.yml Updates macOS runner selection for CI.
Sources/SignozVapor/SignozTracingMiddleware.swift New Vapor middleware for server span creation, route naming, and propagation.
Sources/SignozSwift/Tracing/W3CTraceContext.swift New shared W3C Trace Context parse/serialize utility.
Sources/SignozSwift/Tracing/OTelTracingBridge.swift Refactors inject/extract to reuse W3CTraceContext.
Package.swift Adds SignozVapor product/target and the Vapor package dependency.
Package.resolved Locks new Vapor-related transitive dependencies.
README.md Documents Vapor usage via SignozVapor and the new middleware behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +103 to +107
let (traceparent, tracestate) = W3CTraceContext.serialize(spanContext)
response.headers.replaceOrAdd(name: W3CTraceContext.traceparentKey, value: traceparent)
if let tracestate {
response.headers.replaceOrAdd(name: W3CTraceContext.tracestateKey, value: tracestate)
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This middleware adds traceparent/tracestate headers to responses. That can unintentionally expose internal trace IDs to external clients and may not be desirable by default. Consider making response header injection opt-in (config flag), limiting it to cases where the incoming request provided a valid traceparent, or documenting the security/privacy implications clearly.

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/ci.yml
Comment on lines +13 to 14
runs-on: macos-26-intel
steps:
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR metadata says this change is only a CI runner bump, but this PR also adds a new SignozVapor product/target, a new middleware implementation, W3C trace context refactor, and documentation updates. Please update the PR title/description (or split into separate PRs) so reviewers understand the full scope.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +2
@preconcurrency import OpenTelemetryApi

Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file calls trimmingCharacters(in: .whitespaces) (below) but does not import Foundation, which can cause a compile error since that API is provided by Foundation’s overlay. Add import Foundation at the top.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +28
guard parts.count >= 4, parts[0] != "ff" else {
return nil
}

let traceId = TraceId(fromHexString: String(parts[1]))
let spanId = SpanId(fromHexString: String(parts[2]))
guard traceId.isValid, spanId.isValid else {
return nil
}

let sampled = UInt8(String(parts[3]), radix: 16).map { $0 & 0x01 != 0 } ?? false
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

traceparent validation is too permissive for version 00: per W3C Trace Context, version 00 must have exactly 4 --delimited parts (no extra fields), and the version/flags fields must be 2 hex chars. Consider tightening the guards so malformed headers don’t get accepted as valid remote parents.

Suggested change
guard parts.count >= 4, parts[0] != "ff" else {
return nil
}
let traceId = TraceId(fromHexString: String(parts[1]))
let spanId = SpanId(fromHexString: String(parts[2]))
guard traceId.isValid, spanId.isValid else {
return nil
}
let sampled = UInt8(String(parts[3]), radix: 16).map { $0 & 0x01 != 0 } ?? false
// Need at least 4 parts: version, trace-id, span-id, trace-flags.
guard parts.count >= 4 else {
return nil
}
let versionPart = parts[0]
// Version field must be exactly 2 hex chars and not "ff".
guard versionPart.count == 2,
UInt8(versionPart, radix: 16) != nil,
versionPart != "ff"
else {
return nil
}
// For version "00", the header must have exactly 4 parts (no extras).
if versionPart == "00" && parts.count != 4 {
return nil
}
let traceId = TraceId(fromHexString: String(parts[1]))
let spanId = SpanId(fromHexString: String(parts[2]))
guard traceId.isValid, spanId.isValid else {
return nil
}
let flagsPart = parts[3]
// Trace-flags field must be exactly 2 hex chars.
guard flagsPart.count == 2,
let flagsByte = UInt8(flagsPart, radix: 16)
else {
return nil
}
let sampled = (flagsByte & 0x01) != 0

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +16
/// Parse W3C `traceparent` and optional `tracestate` header values into a `SpanContext`.
public static func parse(traceparent: String, tracestate: String? = nil) -> SpanContext? {
// W3C spec: reject only version "ff"; accept unknown future versions
// for forward compatibility (the first 55 chars are guaranteed stable).
// Future versions may append extra `-` delimited fields, so require >= 4 parts.
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W3CTraceContext.parse/serialize introduces new behavior (forward-version support, ff rejection, extra-field handling) that isn’t directly covered by the existing propagation tests. Adding focused unit tests for these edge cases would help prevent regressions.

Copilot uses AI. Check for mistakes.
span.status = .error(description: "\(status)")
} else {
span.setAttribute(key: "http.status_code", value: .int(500))
span.status = .error(description: String(describing: type(of: error)))
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For non-AbortError failures, this records only the error type name in the span status. Elsewhere in the codebase, span status error descriptions use the full error string ("\(error)"), which preserves the message/context. Consider switching to that format here for consistency and better debuggability.

Suggested change
span.status = .error(description: String(describing: type(of: error)))
span.status = .error(description: "\(error)")

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/SignozSwift/Tracing/W3CTraceContext.swift`:
- Around line 13-30: In parse(traceparent:tracestate:) strengthen validation:
require parts.count >= 4 and validate each field's length and hex-ness (parts[0]
must be exactly 2 hex chars and not "ff" case-insensitive; parts[1] must be 32
hex chars for TraceId; parts[2] must be 16 hex chars for SpanId; parts[3] must
be exactly 2 hex chars for flags), and if any of these checks fail return nil;
parse the flags with a failable UInt8 initializer and if it fails reject the
header instead of defaulting to false, and only construct/set traceFlags (using
TraceFlags.setIsSampled) after all validations succeed (use
TraceId(fromHexString:) and SpanId(fromHexString:) results only when their
inputs passed the explicit hex/length checks).

In `@Sources/SignozVapor/SignozTracingMiddleware.swift`:
- Around line 73-79: The AbortError branch currently marks every thrown
AbortError as span.status = .error; change it to mirror the earlier response
handling by setting span.setAttribute(key: "http.status_code", value:
.int(Int(status.code))) and only set span.status = .error(description:
"\(status)") when status.code >= 500, otherwise set span.status to a non-error
state (e.g., .ok or unset) so thrown AbortError spans match returned-response
classification; update the logic in the AbortError handling block around
AbortError, status.code, span.setAttribute and span.status to apply this
conditional.
- Around line 28-35: The current extraction only calls builder.setParent(...)
when W3CTraceContext.parse succeeds; when headers are absent or parse returns
nil you must explicitly break the parent chain by calling builder.setNoParent().
Update the logic in SignozTracingMiddleware.swift around the
W3CTraceContext.parse branch so that if traceparent/tracestate are missing or
parsing fails you call builder.setNoParent() (instead of leaving it unset),
mirroring the pattern used in OTelTracingBridge.swift to avoid inheriting an
ambient span.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a08ea577-8744-4ea4-a061-5889fe65fafe

📥 Commits

Reviewing files that changed from the base of the PR and between be46216 and 19d0137.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml
  • Package.resolved
  • Package.swift
  • README.md
  • Sources/SignozSwift/Tracing/OTelTracingBridge.swift
  • Sources/SignozSwift/Tracing/W3CTraceContext.swift
  • Sources/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/SignozSwift/Tracing/OTelTracingBridge.swift (2)
Sources/SignozSwift/Tracing/W3CTraceContext.swift (2)
  • serialize (54-66)
  • parse (13-51)
Tests/SignozSwiftTests/SignozSwiftTests.swift (3)
  • serialize (723-725)
  • inject (759-761)
  • extract (766-768)
🪛 actionlint (1.7.11)
.github/workflows/ci.yml

[error] 13-13: label "macos-26-intel" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

Comment on lines +13 to +30
public static func parse(traceparent: String, tracestate: String? = nil) -> SpanContext? {
// W3C spec: reject only version "ff"; accept unknown future versions
// for forward compatibility (the first 55 chars are guaranteed stable).
// Future versions may append extra `-` delimited fields, so require >= 4 parts.
let parts = traceparent.split(separator: "-")
guard parts.count >= 4, parts[0] != "ff" else {
return nil
}

let traceId = TraceId(fromHexString: String(parts[1]))
let spanId = SpanId(fromHexString: String(parts[2]))
guard traceId.isValid, spanId.isValid else {
return nil
}

let sampled = UInt8(String(parts[3]), radix: 16).map { $0 & 0x01 != 0 } ?? false
var traceFlags = TraceFlags()
traceFlags.setIsSampled(sampled)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n Sources/SignozSwift/Tracing/W3CTraceContext.swift

Repository: photon-hq/SignozSwift

Length of output: 3244


🌐 Web query:

W3C Trace Context specification traceparent header format validation requirements

💡 Result:

traceparent format (Trace Context v1.0, version 00)

A traceparent header value must be (ABNF):
version "-" trace-id "-" parent-id "-" trace-flags, where all hex is lowercase (0-9a-f). [1]

  • version: exactly 2 lowercase hex chars; ff is forbidden/invalid. [1]
  • trace-id: exactly 32 lowercase hex chars (16 bytes); all-zero value is invalid (000…000). If invalid, vendors MUST ignore the traceparent. [1]
  • parent-id: exactly 16 lowercase hex chars (8 bytes); all-zero value is invalid (000…0000). If invalid, vendors MUST ignore the traceparent. [1]
  • trace-flags: exactly 2 lowercase hex chars (1 byte). For version 00, only the least-significant bit is defined (sampled); all other bits are reserved and vendors MUST set them to 0 when propagating. [1]

What to do when parsing/validation fails

  • If the version prefix can’t be parsed (not 2 hex + -), the implementation should restart the trace (i.e., create a new traceparent) and delete tracestate. [1]
  • If the version is higher than supported, the implementation SHOULD try to parse trace-id, parent-id, and the sampled bit using fixed positions/checks; if that parsing fails, restart the trace and delete tracestate; also set any unknown/unparsed flags to 0 on outgoing requests. [1]
  • If traceparent parsing fails, the vendor MUST NOT attempt to parse tracestate. [1]

Header name casing

  • Senders SHOULD use lowercase traceparent, but receivers MUST accept any case. [1]

Sources
[1] W3C Recommendation: Trace Context (Trace Context HTTP Headers Format / traceparent). (w3.org)

Citations:


🏁 Script executed:

rg -t swift "class TraceId|struct TraceId" --max-count=5 -A 10 -B 2

Repository: photon-hq/SignozSwift

Length of output: 47


🏁 Script executed:

rg "TraceId|SpanId" --max-count=10 -B 2 -A 5 | head -50

Repository: photon-hq/SignozSwift

Length of output: 1640


🏁 Script executed:

rg "import.*TraceId|import.*OpenTelemetry" --max-count=10

Repository: photon-hq/SignozSwift

Length of output: 2068


🏁 Script executed:

# Check if there are any tests for W3CTraceContext.parse to understand current behavior
rg "W3CTraceContext.parse|traceparent" -A 5 -B 2 Tests/

Repository: photon-hq/SignozSwift

Length of output: 2081


🏁 Script executed:

rg "Invalid traceparent|sampledBitmask" -A 10 Tests/SignozSwiftTests/SignozSwiftTests.swift

Repository: photon-hq/SignozSwift

Length of output: 777


🏁 Script executed:

# Check if TraceId and SpanId validate all-zeros case
rg "isValid|all.zero|00000000" Tests/SignozSwiftTests/ | grep -i "trace\|span" | head -20

Repository: photon-hq/SignozSwift

Length of output: 47


🏁 Script executed:

# Check the test for sampledBitmask to see if uppercase hex is tested
rg "traceparent.*03|traceparent.*[A-F]" Tests/

Repository: photon-hq/SignozSwift

Length of output: 196


🏁 Script executed:

# Look for any test cases with invalid flags or malformed traceparent
rg "@Test|func.*traceparent|func.*trace" Tests/SignozSwiftTests/SignozSwiftTests.swift -A 1 | grep -A 1 "@Test"

Repository: photon-hq/SignozSwift

Length of output: 3905


🏁 Script executed:

# Let's verify what happens with invalid hex in the flags field
cat > /tmp/test_parse.swift << 'EOF'
let flagsInvalid = "ZZ"
let result = UInt8(flagsInvalid, radix: 16)
print("Invalid hex 'ZZ': \(result ?? -1)") // Should be nil

let flagsValid = "03"
let result2 = UInt8(flagsValid, radix: 16)
print("Valid hex '03': \(result2 ?? -1)") // Should be 3

// Current implementation pattern
let sampled = UInt8(flagsInvalid, radix: 16).map { $0 & 0x01 != 0 } ?? false
print("Invalid flags default to sampled=false: \(sampled)")
EOF
# Can't run Swift directly, but we can verify the logic is correct
cat /tmp/test_parse.swift

Repository: photon-hq/SignozSwift

Length of output: 487


🏁 Script executed:

# Check if there's any validation of hex character length or format in the tests
rg "parts\[0\]|parts\[3\]|parts\.count" Sources/SignozSwift/Tracing/W3CTraceContext.swift

Repository: photon-hq/SignozSwift

Length of output: 209


Strengthen traceparent validation to comply with W3C Trace Context specification.

Line 18 uses string comparison (!= "ff") instead of hex validation, so it accepts uppercase variants like "FF" and doesn't reject invalid hex. Line 28 silently converts invalid flags to false instead of rejecting the header. The W3C spec requires exact 2-character hex fields with specific constraints, and vendors must ignore malformed traceparent headers entirely. Currently, invalid headers may still produce a SpanContext and propagate as remote parents downstream.

Suggested hardening
-        let parts = traceparent.split(separator: "-")
-        guard parts.count >= 4, parts[0] != "ff" else {
+        let parts = traceparent.split(separator: "-", omittingEmptySubsequences: false)
+        guard parts.count >= 4,
+              parts[0].count == 2,
+              let version = UInt8(String(parts[0]), radix: 16),
+              version != 0xff,
+              parts[3].count == 2,
+              let flags = UInt8(String(parts[3]), radix: 16),
+              !(version == 0x00 && parts.count != 4) else {
             return nil
         }
 
         let traceId = TraceId(fromHexString: String(parts[1]))
         let spanId = SpanId(fromHexString: String(parts[2]))
         guard traceId.isValid, spanId.isValid else {
             return nil
         }
 
-        let sampled = UInt8(String(parts[3]), radix: 16).map { $0 & 0x01 != 0 } ?? false
+        let sampled = flags & 0x01 != 0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/SignozSwift/Tracing/W3CTraceContext.swift` around lines 13 - 30, In
parse(traceparent:tracestate:) strengthen validation: require parts.count >= 4
and validate each field's length and hex-ness (parts[0] must be exactly 2 hex
chars and not "ff" case-insensitive; parts[1] must be 32 hex chars for TraceId;
parts[2] must be 16 hex chars for SpanId; parts[3] must be exactly 2 hex chars
for flags), and if any of these checks fail return nil; parse the flags with a
failable UInt8 initializer and if it fails reject the header instead of
defaulting to false, and only construct/set traceFlags (using
TraceFlags.setIsSampled) after all validations succeed (use
TraceId(fromHexString:) and SpanId(fromHexString:) results only when their
inputs passed the explicit hex/length checks).

Comment on lines +28 to +35
// 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)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the SignozTracingMiddleware file
find . -type f -name "SignozTracingMiddleware.swift" | head -5

Repository: photon-hq/SignozSwift

Length of output: 116


🏁 Script executed:

# Also find the OTelTracingBridge file
find . -type f -name "*OTelTracingBridge*" -o -name "*TracingBridge*" | head -5

Repository: photon-hq/SignozSwift

Length of output: 118


🏁 Script executed:

# Read the SignozTracingMiddleware.swift file, focusing on lines 28-35
cat -n ./Sources/SignozVapor/SignozTracingMiddleware.swift | head -50

Repository: photon-hq/SignozSwift

Length of output: 2320


🏁 Script executed:

# Read the OTelTracingBridge.swift file, focusing on lines 28-34
cat -n ./Sources/SignozSwift/Tracing/OTelTracingBridge.swift | head -50

Repository: photon-hq/SignozSwift

Length of output: 2243


🏁 Script executed:

# Search for setNoParent usage in the codebase to see if this is a known pattern
rg "setNoParent" .

Repository: photon-hq/SignozSwift

Length of output: 152


🏁 Script executed:

# Also check if there's any documentation or tests related to parent context behavior
rg -i "parent.*context\|no.*parent" --type swift | head -30

Repository: photon-hq/SignozSwift

Length of output: 47


🏁 Script executed:

# Get the full context of SignozTracingMiddleware to see what happens after the span is created
cat -n ./Sources/SignozVapor/SignozTracingMiddleware.swift | sed -n '25,70p'

Repository: photon-hq/SignozSwift

Length of output: 2337


Call setNoParent() when extraction fails or headers are absent.

When the request has no valid inbound W3C trace context, the builder should explicitly break the parent chain. OTelTracingBridge.swift (lines 28–34) demonstrates this pattern: it explicitly calls setNoParent() when no parent context is available. Without this call, the span builder may inherit an unrelated ambient span, breaking trace isolation for this entry point.

Suggested fix
         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)
+        } else {
+            builder.setNoParent()
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/SignozVapor/SignozTracingMiddleware.swift` around lines 28 - 35, The
current extraction only calls builder.setParent(...) when W3CTraceContext.parse
succeeds; when headers are absent or parse returns nil you must explicitly break
the parent chain by calling builder.setNoParent(). Update the logic in
SignozTracingMiddleware.swift around the W3CTraceContext.parse branch so that if
traceparent/tracestate are missing or parsing fails you call
builder.setNoParent() (instead of leaving it unset), mirroring the pattern used
in OTelTracingBridge.swift to avoid inheriting an ambient span.

Comment on lines +73 to +79
if let abort = error as? AbortError {
let status = abort.status
span.setAttribute(key: "http.status_code", value: .int(Int(status.code)))
span.status = .error(description: "\(status)")
} else {
span.setAttribute(key: "http.status_code", value: .int(500))
span.status = .error(description: String(describing: type(of: error)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n Sources/SignozVapor/SignozTracingMiddleware.swift

Repository: photon-hq/SignozSwift

Length of output: 5008


Keep thrown AbortError classification consistent with returned responses.

Line 58 only flags >= 500 responses as errors, but line 76 marks every thrown AbortError as .error. A returned 404 and a thrown Abort(.notFound) therefore emit different span status for the same HTTP result. Apply the same status code threshold to AbortError to maintain consistency:

if let abort = error as? AbortError {
    let status = abort.status
    span.setAttribute(key: "http.status_code", value: .int(Int(status.code)))
-   span.status = .error(description: "\(status)")
+   if status.code >= 500 {
+       span.status = .error(description: "\(status)")
+   } else {
+       span.status = .ok
+   }
} else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if let abort = error as? AbortError {
let status = abort.status
span.setAttribute(key: "http.status_code", value: .int(Int(status.code)))
span.status = .error(description: "\(status)")
} else {
span.setAttribute(key: "http.status_code", value: .int(500))
span.status = .error(description: String(describing: type(of: error)))
if let abort = error as? AbortError {
let status = abort.status
span.setAttribute(key: "http.status_code", value: .int(Int(status.code)))
if status.code >= 500 {
span.status = .error(description: "\(status)")
} else {
span.status = .ok
}
} else {
span.setAttribute(key: "http.status_code", value: .int(500))
span.status = .error(description: String(describing: type(of: error)))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/SignozVapor/SignozTracingMiddleware.swift` around lines 73 - 79, The
AbortError branch currently marks every thrown AbortError as span.status =
.error; change it to mirror the earlier response handling by setting
span.setAttribute(key: "http.status_code", value: .int(Int(status.code))) and
only set span.status = .error(description: "\(status)") when status.code >= 500,
otherwise set span.status to a non-error state (e.g., .ok or unset) so thrown
AbortError spans match returned-response classification; update the logic in the
AbortError handling block around AbortError, status.code, span.setAttribute and
span.status to apply this conditional.

@underthestars-zhy Ryan Zhu (underthestars-zhy) merged commit c318035 into main Mar 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants