Skip to content

Commit 044e9b9

Browse files
lrafeeilrafeei
committed
Reviewer suggestions and syntax
1 parent d95f7a0 commit 044e9b9

File tree

4 files changed

+63
-53
lines changed

4 files changed

+63
-53
lines changed

‎newrelic/api/opentelemetry.py‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
from newrelic.api.transaction import Sentinel, current_transaction
3636
from newrelic.api.web_transaction import WebTransaction, WSGIWebTransaction
3737
from newrelic.core.otlp_utils import create_resource
38-
from newrelic.core.database_utils import _parse_operation, _parse_target
3938

4039
_logger = logging.getLogger(__name__)
4140

@@ -218,16 +217,15 @@ def set_attributes(self, attributes):
218217
self.set_attribute(key, value)
219218

220219
def _set_attributes_in_nr(self, otel_attributes=None):
221-
if not otel_attributes or not getattr(self, "nr_trace"):
220+
if not otel_attributes or not getattr(self, "nr_trace", None):
222221
return
223222

224-
# If these attributes already exist in NR's intrinsic or agent
225-
# attributes, keep the attributes in the OTel span, but do not
226-
# add them to NR's user attributes to avoid sending the same
227-
# data multiple times.
228-
combined_attrs = {**self.nr_trace.agent_attributes, **self.nr_trace.user_attributes}
223+
# If these attributes already exist in NR's agent attributes,
224+
# keep the attributes in the OTel span, but do not add them
225+
# to NR's user attributes to avoid sending the same data
226+
# multiple times.
229227
for key, value in otel_attributes.items():
230-
if key not in combined_attrs:
228+
if key not in self.nr_trace.agent_attributes:
231229
self.nr_trace.add_custom_attribute(key, value)
232230

233231
def add_event(self, name, attributes=None, timestamp=None):
@@ -312,9 +310,10 @@ def _database_attribute_mapping(self):
312310
"port_path_or_id": self.attributes.get("net.peer.port") or self.attributes.get("server.port"),
313311
"product": self.attributes.get("db.system").capitalize(),
314312
}
313+
agent_attrs = {}
315314

316315
db_statement = self.attributes.get("db.statement")
317-
if db_statment:
316+
if db_statement:
318317
if hasattr(db_statement, "string"):
319318
db_statement = db_statement.string
320319
operation, target = get_database_operation_target_from_statement(db_statement)
@@ -324,27 +323,32 @@ def _database_attribute_mapping(self):
324323
"target": target,
325324
})
326325
elif span_obj_attrs["product"] == "Dynamodb":
327-
region = self.attributes.get("cloud.region"),
326+
region = self.attributes.get("cloud.region")
327+
operation = self.attributes.get("db.operation")
328328
target = self.attributes.get("aws.dynamodb.table_names", [None])[-1]
329329
account_id = self.nr_transaction.settings.cloud.aws.account_id
330-
resource_id = generate_dynamodb_arn(region, account_id, target)
331-
agent_attrs = {
330+
resource_id = generate_dynamodb_arn(span_obj_attrs["host"], region, account_id, target)
331+
agent_attrs.update({
332332
"aws.operation": self.attributes.get("db.operation"),
333333
"cloud.resource_id": resource_id,
334334
"cloud.region": region,
335335
"aws.requestId": self.attributes.get("aws.request_id"),
336-
"http.statusCode", self.attributes.get("http.status_code"))
337-
"cloud.account.id", account_id)
338-
}
336+
"http.statusCode": self.attributes.get("http.status_code"),
337+
"cloud.account.id": account_id,
338+
})
339339
span_obj_attrs.update({
340340
"target": target,
341341
"operation": operation,
342342
})
343343

344-
for key, value in span_obj_attrs.items() if value:
345-
setattr(self.nr_trace, key, value)
346-
for key, value in agent_attrs.items() if value:
347-
self.nr_trace._add_agent_attribute(key, value)
344+
# We do not want to override any agent attributes
345+
# with `None` if `value` does not exist.
346+
for key, value in span_obj_attrs.items():
347+
if value:
348+
setattr(self.nr_trace, key, value)
349+
for key, value in agent_attrs.items():
350+
if value:
351+
self.nr_trace._add_agent_attribute(key, value)
348352

349353
def end(self, end_time=None, *args, **kwargs):
350354
# We will ignore the end_time parameter and use NR's end_time

‎newrelic/core/database_utils.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,12 @@ def _parse_operation(sql):
419419
return operation if operation in _operation_table else ""
420420

421421

422+
def _parse_operation_otel(sql):
423+
match = _parse_operation_re.search(sql)
424+
operation = (match and match.group(1).lower()) or ""
425+
return operation or ""
426+
427+
422428
def _parse_target(sql, operation):
423429
sql = sql.rstrip(";")
424430
parse = _operation_table.get(operation, None)
@@ -901,7 +907,7 @@ def sql_statement(sql, dbapi2_module):
901907
return result
902908

903909

904-
def generate_dynamodb_arn(host, region, account_id, target):
910+
def generate_dynamodb_arn(host, region=None, account_id=None, target=None):
905911
# There are 3 different partition options.
906912
# See https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html for details.
907913
partition = "aws"
@@ -911,10 +917,10 @@ def generate_dynamodb_arn(host, region, account_id, target):
911917
partition = "aws-us-gov"
912918

913919
if partition and region and account_id and target:
914-
return f"arn:{partition}:dynamodb:{region}:{account_id:012d}:table/{_target}"
920+
return f"arn:{partition}:dynamodb:{region}:{account_id:012d}:table/{target}"
915921

916922

917923
def get_database_operation_target_from_statement(db_statement):
918-
operation = _parse_operation(db_statement)
924+
operation = _parse_operation_otel(db_statement)
919925
target = _parse_target(db_statement, operation)
920-
return target
926+
return operation, target

‎newrelic/hooks/external_botocore.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,7 @@ def _nr_dynamodb_datastore_trace_wrapper_(wrapped, instance, args, kwargs):
12981298

12991299
_db_host = getattr(getattr(instance, "_endpoint", None), "host", None)
13001300
resource_id = generate_dynamodb_arn(_db_host, region, account_id, _target)
1301-
if resouce_id:
1301+
if resource_id:
13021302
agent_attrs["cloud.resource_id"] = resource_id
13031303

13041304
except Exception:

‎tests/hybridagent_pymongo/test_collection.py‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,13 @@ def _exercise_mongo(db):
9292
@validate_span_events(
9393
count=1,
9494
exact_agents={
95-
"db.operation": "test.insert",
95+
"db.operation": "insert",
9696
"db.collection": MONGODB_COLLECTION,
9797
"db.instance": "test",
9898
"peer.hostname": INSTANCE_METRIC_HOST,
9999
"peer.address": f"{INSTANCE_METRIC_HOST}:{MONGODB_PORT}",
100100
},
101-
exact_intrinsics={"name": f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.insert"},
101+
exact_intrinsics={"name": f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/insert"},
102102
)
103103
@validate_transaction_metrics(
104104
"test_motor_instance_info", rollup_metrics=[(INSTANCE_METRIC_NAME, 1)], background_task=True
@@ -115,21 +115,21 @@ def test_collection_instance_info(loop, tracer_provider):
115115
# Common Metrics for tests that use _exercise_mongo().
116116

117117
_test_pymongo_client_scoped_metrics = [
118-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.insert", 6),
119-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.find", 5),
120-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.aggregate", 3),
121-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.count", 1),
122-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.update", 3),
123-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.delete", 2),
124-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.distinct", 1),
125-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.createIndexes", 2),
126-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.listIndexes", 2),
127-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.dropIndexes", 2),
128-
(f"Datastore/operation/Mongodb/test.getMore", 1),
129-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/test.findAndModify", 3),
130-
(f"Datastore/operation/Mongodb/test.listCollections", 1),
131-
(f"Datastore/statement/Mongodb/test.{MONGODB_COLLECTION}/admin.renameCollection", 1),
132-
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}_renamed/test.drop", 1),
118+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/insert", 6),
119+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/find", 5),
120+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/aggregate", 3),
121+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/count", 1),
122+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/update", 3),
123+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/delete", 2),
124+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/distinct", 1),
125+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/createindexes", 2),
126+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/listindexes", 2),
127+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/dropindexes", 2),
128+
(f"Datastore/operation/Mongodb/getmore", 1),
129+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}/findandmodify", 3),
130+
(f"Datastore/operation/Mongodb/listcollections", 1),
131+
(f"Datastore/statement/Mongodb/test.{MONGODB_COLLECTION}/renamecollection", 1), # admin commands use name given by OTel
132+
(f"Datastore/statement/Mongodb/{MONGODB_COLLECTION}_renamed/drop", 1),
133133
]
134134

135135
_test_pymongo_client_rollup_metrics = [
@@ -138,18 +138,18 @@ def test_collection_instance_info(loop, tracer_provider):
138138
("Datastore/Mongodb/all", 35),
139139
("Datastore/Mongodb/allOther", 35),
140140
(INSTANCE_METRIC_NAME, 35),
141-
("Datastore/operation/Mongodb/test.insert", 6),
142-
("Datastore/operation/Mongodb/test.find", 5),
143-
("Datastore/operation/Mongodb/test.aggregate", 3),
144-
("Datastore/operation/Mongodb/test.count", 1),
145-
("Datastore/operation/Mongodb/test.update", 3),
146-
("Datastore/operation/Mongodb/test.delete", 2),
147-
("Datastore/operation/Mongodb/test.distinct", 1),
148-
("Datastore/operation/Mongodb/test.createIndexes", 2),
149-
("Datastore/operation/Mongodb/test.listIndexes", 2),
150-
("Datastore/operation/Mongodb/test.dropIndexes", 2),
151-
("Datastore/operation/Mongodb/test.findAndModify", 3),
152-
("Datastore/operation/Mongodb/admin.renameCollection", 1),
141+
("Datastore/operation/Mongodb/insert", 6),
142+
("Datastore/operation/Mongodb/find", 5),
143+
("Datastore/operation/Mongodb/aggregate", 3),
144+
("Datastore/operation/Mongodb/count", 1),
145+
("Datastore/operation/Mongodb/update", 3),
146+
("Datastore/operation/Mongodb/delete", 2),
147+
("Datastore/operation/Mongodb/distinct", 1),
148+
("Datastore/operation/Mongodb/createindexes", 2),
149+
("Datastore/operation/Mongodb/listindexes", 2),
150+
("Datastore/operation/Mongodb/dropindexes", 2),
151+
("Datastore/operation/Mongodb/findandmodify", 3),
152+
("Datastore/operation/Mongodb/renamecollection", 1),
153153
]
154154
_test_pymongo_client_rollup_metrics.extend(_test_pymongo_client_scoped_metrics)
155155

0 commit comments

Comments
 (0)