Skip to content

Commit 80018e2

Browse files
chore(dbm): use peer.service precursor when enabled [backport 1.19] (#7034)
Backport 466fd2a from #6962 to 1.19. DBM adds comments to database commands for the purposes of context propagation. One of the pieces of information in the comments is the service name of the external database and is used for generating metrics (i.e. # of writes per second to external database). Prior to this change, DBM relied on the service name being set to the value of span.service. In auto-instrumented services, this value typically belongs to the fake span generated with the destination database name, such as "postgres". This caused issues when peer.service is enabled, since the fake span no longer existed. After this change, DBM will use the precursor for peer.service (if available) when peer service defaults are enabled. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Teague Bick <[email protected]>
1 parent 1911437 commit 80018e2

File tree

3 files changed

+52
-1
lines changed

3 files changed

+52
-1
lines changed

ddtrace/propagation/_database_monitoring.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
from typing import Union # noqa
33

44
from ddtrace.internal.logger import get_logger
5+
from ddtrace.settings.peer_service import PeerServiceConfig
56
from ddtrace.vendor.sqlcommenter import generate_sql_comment as _generate_sql_comment
67

8+
from ..internal import compat
79
from ..internal.utils import get_argument_value
810
from ..internal.utils import set_argument_value
911
from ..settings import _config as dd_config
@@ -71,11 +73,17 @@ def _get_dbm_comment(self, db_span):
7173
return None
7274

7375
# set the following tags if DBM injection mode is full or service
76+
peer_service_enabled = PeerServiceConfig().set_defaults_enabled
77+
service_name_key = db_span.service
78+
if peer_service_enabled:
79+
db_name = db_span.get_tags().get("db.name")
80+
service_name_key = compat.ensure_str(db_name) if db_name else db_span.service
81+
7482
dbm_tags = {
7583
DBM_PARENT_SERVICE_NAME_KEY: dd_config.service,
7684
DBM_ENVIRONMENT_KEY: dd_config.env,
7785
DBM_VERSION_KEY: dd_config.version,
78-
DBM_DATABASE_SERVICE_NAME_KEY: db_span.service,
86+
DBM_DATABASE_SERVICE_NAME_KEY: service_name_key,
7987
}
8088

8189
if dbm_config.propagation_mode == "full":
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
dbm: this fixes an issue with attributing metrics to incorrect services when peer.service is enabled

tests/internal/test_database_monitoring.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,45 @@ def test_dbm_propagation_full_mode():
119119
assert dbspan.get_tag(_database_monitoring.DBM_TRACE_INJECTED_TAG) == "true"
120120

121121

122+
@pytest.mark.subprocess(
123+
env=dict(
124+
DD_DBM_PROPAGATION_MODE="full",
125+
DD_SERVICE="orders-app",
126+
DD_ENV="staging",
127+
DD_VERSION="v7343437-d7ac743",
128+
DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED="True",
129+
)
130+
)
131+
def test_dbm_dddbs_peer_service_enabled():
132+
from ddtrace import tracer
133+
from ddtrace.propagation import _database_monitoring
134+
135+
dbspan_no_service = tracer.trace("dbname")
136+
137+
# when dbm propagation mode is full sql comments should be generated with dbm tags and traceparent keys
138+
dbm_popagator = _database_monitoring._DBM_Propagator(0, "procedure")
139+
sqlcomment = dbm_popagator._get_dbm_comment(dbspan_no_service)
140+
# assert tags sqlcomment contains the correct value
141+
assert (
142+
sqlcomment
143+
== "/*dddbs='orders-app',dde='staging',ddps='orders-app',ddpv='v7343437-d7ac743',traceparent='%s'*/ "
144+
% (dbspan_no_service.context._traceparent,)
145+
), sqlcomment
146+
147+
dbspan_with_peer_service = tracer.trace("dbname")
148+
dbspan_with_peer_service.set_tag_str("db.name", "db-name-test")
149+
150+
# when dbm propagation mode is full sql comments should be generated with dbm tags and traceparent keys
151+
dbm_popagator = _database_monitoring._DBM_Propagator(0, "procedure")
152+
sqlcomment = dbm_popagator._get_dbm_comment(dbspan_with_peer_service)
153+
# assert tags sqlcomment contains the correct value
154+
assert (
155+
sqlcomment
156+
== "/*dddbs='db-name-test',dde='staging',ddps='orders-app',ddpv='v7343437-d7ac743',traceparent='%s'*/ "
157+
% (dbspan_with_peer_service.context._traceparent,)
158+
), sqlcomment
159+
160+
122161
def test_default_sql_injector(caplog):
123162
# test sql injection with unicode str
124163
dbm_comment = "/*dddbs='orders-db'*/ "

0 commit comments

Comments
 (0)