Skip to content

fix: align HTTP and DB spans with OpenTelemetry Semantic Conventions#18

Open
lucas-angeli-gimenes wants to merge 1 commit intomainfrom
span-conventions
Open

fix: align HTTP and DB spans with OpenTelemetry Semantic Conventions#18
lucas-angeli-gimenes wants to merge 1 commit intomainfrom
span-conventions

Conversation

@lucas-angeli-gimenes
Copy link
Collaborator

Description

This PR fixes span naming, attributes, and status code handling across HTTP (server/client) and database (SQL, Redis, MongoDB) instrumentation to conform with the OpenTelemetry Semantic Conventions.


HTTP Spans

TraceMiddleware (server spans)

  • Fixed span name to follow the {method} {target} format (e.g. GET /users/{number}) as required by the spec. Previously, only the sanitized path was used.

GuzzleClientAspect (client spans)

  • Removed http.response.status_code = 0 from metric attributes on rejected requests (network errors, timeouts). Per spec, this attribute is conditionally required: if and only if a response was received. When no response exists,
    the attribute must be omitted.

Database Spans

DbQueryExecutedListener (MySQL / PostgreSQL)

  • Fixed span name: removed the driver prefix (e.g. mysql, postgresql). Span name now follows {operation} {table} (e.g. SELECT users).
  • Added db.query.summary to span attributes (was previously only set on metrics).
  • Removed deprecated legacy attributes: db.system, db.operation, db.sql.table.

RedisAspect

  • Fixed span name: removed the Redis prefix. Span name is now the command only (e.g. GET, HMSET).
  • Removed setStatus(STATUS_OK) on successful operations — per spec, span status SHOULD be left UNSET on success.
  • Removed deprecated legacy attributes: db.system, db.operation.pool, redis.pool.

MongoAspect

  • Fixed span name: removed the mongodb prefix and fixed casing. Span name now uses the method name as-is (e.g. find users, insertOne users), matching MongoDB's camelCase command naming convention.
  • Added db.namespace (database name) via reflection, as it is conditionally required for MongoDB spans.
  • Removed setStatus(STATUS_OK) on successful operations.
  • Removed deprecated legacy attributes: db.system, db.operation, db.collection.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Performance/Refactor
  • CI/CD/Chore

Checklist

  • Title follows Conventional Commits (e.g., feat: add SQS aspect)
  • Tests added/updated
  • composer test passes locally
  • Documentation updated (README/Docs)
  • No breaking changes (or documented if any)

Copilot AI review requested due to automatic review settings February 26, 2026 19:48
Copy link
Contributor

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 pull request aligns OpenTelemetry instrumentation with the official Semantic Conventions by fixing span naming, attribute usage, and status code handling across HTTP and database instrumentation. The changes remove deprecated attributes, adjust span names to follow spec-compliant formats, and correct conditional attribute inclusion.

Changes:

  • HTTP server spans now use {method} {path} format (e.g., 'GET /users/{number}')
  • HTTP client spans omit http.response.status_code attribute when no response is received
  • Database spans remove driver/system prefixes and deprecated legacy attributes while adding DB_QUERY_SUMMARY
  • MongoDB spans use camelCase operation names and conditionally include DB_NAMESPACE
  • Successful spans no longer set explicit STATUS_OK status

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Middleware/TraceMiddleware.php Updated HTTP server span names to include HTTP method prefix
src/Aspect/GuzzleClientAspect.php Removed HTTP_RESPONSE_STATUS_CODE => 0 from metrics for rejected requests
src/Listener/DbQueryExecutedListener.php Removed driver prefix from span names, added DB_QUERY_SUMMARY attribute, removed legacy attributes
src/Aspect/RedisAspect.php Removed 'Redis ' prefix from span names, removed setStatus(STATUS_OK), removed legacy attributes and unused import
src/Aspect/MongoAspect.php Changed to camelCase operation names, added conditional DB_NAMESPACE attribute, removed setStatus(STATUS_OK), removed legacy attributes and unused imports
tests/Unit/Middleware/TraceMiddlewareTest.php Updated test expectations to match new HTTP server span naming format
tests/Unit/Aspect/GuzzleClientAspectTest.php Updated test to verify HTTP_RESPONSE_STATUS_CODE is not included for rejected requests
tests/Unit/Listener/DbQueryExecutedListenerTest.php Updated test expectations for new span names, added DB_QUERY_SUMMARY verification, removed legacy attributes
tests/Unit/Aspect/RedisAspectTest.php Updated test expectations for new span names, removed setStatus verification, removed legacy attributes, removed unused import
tests/Unit/Aspect/MongoAspectTest.php Updated test expectations for camelCase operation names, removed setStatus verification, removed legacy attributes, removed unused import

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

Comment on lines 149 to 153
$this->equalTo([
DbAttributes::DB_SYSTEM_NAME => DbIncubatingAttributes::DB_SYSTEM_NAME_VALUE_MONGODB,
DbAttributes::DB_COLLECTION_NAME => 'users',
DbAttributes::DB_OPERATION_NAME => 'FIND',
'db.system' => DbIncubatingAttributes::DB_SYSTEM_NAME_VALUE_MONGODB,
'db.operation' => 'FIND',
'db.collection' => 'users',
DbAttributes::DB_OPERATION_NAME => 'find',
])
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The new getNamespace() method and the conditional inclusion of DB_NAMESPACE attribute lack test coverage. The mock Collection class in the test setup (lines 62-68) only defines a 'collection' property, not a 'database' property. This means getNamespace() will always return null in the current tests, and the happy path where DB_NAMESPACE is successfully extracted and included in span attributes is not verified.

Consider adding a test case that:

  1. Creates a mock Collection with both 'collection' and 'database' properties
  2. Verifies that DB_NAMESPACE is included in the span attributes when successfully extracted
  3. Ensures the existing test continues to verify the behavior when 'database' property is not available

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants