Db semantic conventions update - #48979
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate SQL Server compatibility issues remain unresolved.
Pull request overview
Updates Azure Monitor OpenTelemetry database semantic-convention handling while retaining deprecated attribute compatibility.
Changes:
- Adds stable database attribute support for exporter and QuickPulse telemetry.
- Updates namespace, target, and port resolution.
- Adds regression tests and changelog documentation.
File summaries
| File | Summary |
|---|---|
sdk/monitor/azure-monitor-opentelemetry-exporter/tests/trace/test_trace.py |
Adds stable database span conversion tests. |
sdk/monitor/azure-monitor-opentelemetry-exporter/tests/quickpulse/test_types.py |
Adds stable QuickPulse database tests. |
sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md |
Documents the semantic-convention update. |
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_utils.py |
Supports stable attributes; needs stable SQL Server default-port handling. |
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_exporter.py |
Maps stable database attributes; needs stable SQL Server normalization. |
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_quickpulse/_types.py |
Adds stable database mapping for QuickPulse. |
Review details
Suppressed comments (2)
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_exporter.py:430
- The stable
db.system.namevalue for SQL Server ismicrosoft.sql_server, but this path only treats the deprecatedmssqlvalue as SQL via_is_sql_db. Stable SQL Server spans therefore fall through todata.type = db_systemand are emitted asmicrosoft.sql_server, instead of preserving the existingSQLdependency type produced for legacy SQL Server spans. Normalize the stable alias as well.
db_system = span.attributes.get(DB_SYSTEM_NAME) or span.attributes.get(SpanAttributes.DB_SYSTEM)
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_utils.py:167
- The stable
db.system.namevalue for SQL Server ismicrosoft.sql_server, but_get_default_port_dbonly recognizes the deprecatedmssqlvalue. A CLIENT span with the stable attribute,net.peer.name, and the default port 1433 will therefore be emitted withtarget=host:1433instead of omitting the default port, unlike the legacy path. Add the stable value to the default-port mapping and cover this case.
if port != _get_default_port_http(attributes) and port != _get_default_port_db(str(db_system_for_port)):
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
a91b022 to
95e432a
Compare
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
95e432a to
d5c7046
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Stable SQL Server normalization and server endpoint/default-port handling require fixes and regression coverage.
Review details
Suppressed comments (4)
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_exporter.py:430
- The stable convention uses
db.system.name="microsoft.sql_server", but the existing type mapping and_is_sql_db/DbSystemValues.MSSQLchecks recognize only the legacymssqlvalue. A stable SQL Server span will therefore be exported with dependency typemicrosoft.sql_serverinstead of the existingSQLmapping. Normalize this stable value before the mapping or update the shared SQL-system handling, and add a regression test.
db_system = span.attributes.get(DB_SYSTEM_NAME) or span.attributes.get(SpanAttributes.DB_SYSTEM)
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_utils.py:167
- This new branch changes whether
net.peer.portis included in the target for stable database spans, but the added tests only cover the system/query/namespace/operation fields and never exercise default versus non-default database ports. A stable PostgreSQL span withnet.peer.port=5432must not becomehost:5432, while a non-default port must remain visible; add regression cases for both so this changed target-formatting path is protected.
db_system_for_port = attributes.get(db_attributes.DB_SYSTEM_NAME) or attributes.get(
SpanAttributes.DB_SYSTEM
)
if port != _get_default_port_http(attributes) and port != _get_default_port_db(str(db_system_for_port)):
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_utils.py:167
- Stable database client spans use
server.address/server.portfor the database endpoint, but this helper still derives the target only fromnet.peer.name/net.peer.ipandnet.peer.port. A span withdb.system.nameplusserver.addresstherefore loses the server host (and port) and falls back to onlydb.namespace/the system name in_get_target_for_db_dependency; read the stable server attributes here while retaining the deprecated peer fallback, and cover it in both exporter paths.
# Honor both the new stable `db.system.name` and the deprecated `db.system`.
db_system_for_port = attributes.get(db_attributes.DB_SYSTEM_NAME) or attributes.get(
SpanAttributes.DB_SYSTEM
)
if port != _get_default_port_http(attributes) and port != _get_default_port_db(str(db_system_for_port)):
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/trace/_utils.py:167
- The stable convention uses
db.system.name="microsoft.sql_server", while_get_default_port_dbrecognizes only the legacymssqlvalue. Consequently a stable SQL Server span using the default port 1433 will be rendered with:1433in its target instead of omitting the default port. Extend or normalize the database-port mapping for the stable value and cover this case in a test.
db_system_for_port = attributes.get(db_attributes.DB_SYSTEM_NAME) or attributes.get(
SpanAttributes.DB_SYSTEM
)
if port != _get_default_port_http(attributes) and port != _get_default_port_db(str(db_system_for_port)):
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
Description
Fixes - #48958
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines